Skip to content

[CIR] Upstream SelectOp and ShiftOp #133405

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 22, 2025
Merged

Conversation

mmha
Copy link
Contributor

@mmha mmha commented Mar 28, 2025

Since SelectOp will only generated by a future pass that transforms a TernaryOp this only includes the lowering bits.

This patch also improves the testing of the existing binary operators.

@llvmbot llvmbot added clang Clang issues not falling into any other category ClangIR Anything related to the ClangIR project labels Mar 28, 2025
@llvmbot
Copy link
Member

llvmbot commented Mar 28, 2025

@llvm/pr-subscribers-clangir

Author: Morris Hafner (mmha)

Changes

Since SelectOp will only generated by a future pass that transforms a TernaryOp this only includes the lowering bits.

This patch also improves the testing of the existing binary operators.


Patch is 28.28 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/133405.diff

7 Files Affected:

  • (modified) clang/include/clang/CIR/Dialect/IR/CIROps.td (+73)
  • (modified) clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp (+5-4)
  • (modified) clang/lib/CIR/Dialect/IR/CIRDialect.cpp (+43)
  • (modified) clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp (+87)
  • (modified) clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.h (+20)
  • (modified) clang/test/CIR/CodeGen/binop.cpp (+303-14)
  • (added) clang/test/CIR/Lowering/select.cir (+48)
diff --git a/clang/include/clang/CIR/Dialect/IR/CIROps.td b/clang/include/clang/CIR/Dialect/IR/CIROps.td
index 455cc2b8b0277..5c3d0549c1c47 100644
--- a/clang/include/clang/CIR/Dialect/IR/CIROps.td
+++ b/clang/include/clang/CIR/Dialect/IR/CIROps.td
@@ -889,6 +889,79 @@ def BinOp : CIR_Op<"binop", [Pure,
   let hasVerifier = 1;
 }
 
+//===----------------------------------------------------------------------===//
+// ShiftOp
+//===----------------------------------------------------------------------===//
+
+def ShiftOp : CIR_Op<"shift", [Pure]> {
+  let summary = "Shift";
+  let description = [{
+    Shift `left` or `right`, according to the first operand. Second operand is
+    the shift target and the third the amount. Second and the thrid operand are
+    integers.
+
+    ```mlir
+    %7 = cir.shift(left, %1 : !u64i, %4 : !s32i) -> !u64i
+    ```
+  }];
+
+  // TODO(cir): Support vectors. CIR_IntType -> CIR_AnyIntOrVecOfInt. Also
+  // update the description above.
+  let results = (outs CIR_IntType:$result);
+  let arguments = (ins CIR_IntType:$value, CIR_IntType:$amount,
+                       UnitAttr:$isShiftleft);
+
+  let assemblyFormat = [{
+    `(`
+      (`left` $isShiftleft^) : (```right`)?
+      `,` $value `:` type($value)
+      `,` $amount `:` type($amount)
+    `)` `->` type($result) attr-dict
+  }];
+
+  let hasVerifier = 1;
+}
+
+//===----------------------------------------------------------------------===//
+// SelectOp
+//===----------------------------------------------------------------------===//
+
+def SelectOp : CIR_Op<"select", [Pure,
+    AllTypesMatch<["true_value", "false_value", "result"]>]> {
+  let summary = "Yield one of two values based on a boolean value";
+  let description = [{
+    The `cir.select` operation takes three operands. The first operand
+    `condition` is a boolean value of type `!cir.bool`. The second and the third
+    operand can be of any CIR types, but their types must be the same. If the
+    first operand is `true`, the operation yields its second operand. Otherwise,
+    the operation yields its third operand.
+
+    Example:
+
+    ```mlir
+    %0 = cir.const #cir.bool<true> : !cir.bool
+    %1 = cir.const #cir.int<42> : !s32i
+    %2 = cir.const #cir.int<72> : !s32i
+    %3 = cir.select if %0 then %1 else %2 : (!cir.bool, !s32i, !s32i) -> !s32i
+    ```
+  }];
+
+  let arguments = (ins CIR_BoolType:$condition, CIR_AnyType:$true_value,
+                       CIR_AnyType:$false_value);
+  let results = (outs CIR_AnyType:$result);
+
+  let assemblyFormat = [{
+    `if` $condition `then` $true_value `else` $false_value
+    `:` `(`
+      qualified(type($condition)) `,`
+      qualified(type($true_value)) `,`
+      qualified(type($false_value))
+    `)` `->` qualified(type($result)) attr-dict
+  }];
+
+  let hasFolder = 1;
+}
+
 //===----------------------------------------------------------------------===//
 // GlobalOp
 //===----------------------------------------------------------------------===//
diff --git a/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp b/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp
index 52bd3b2933744..3a904fa656104 100644
--- a/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp
@@ -1138,8 +1138,9 @@ mlir::Value ScalarExprEmitter::emitShl(const BinOpInfo &ops) {
            mlir::isa<cir::IntType>(ops.lhs.getType()))
     cgf.cgm.errorNYI("sanitizers");
 
-  cgf.cgm.errorNYI("shift ops");
-  return {};
+  return builder.create<cir::ShiftOp>(cgf.getLoc(ops.loc),
+                                      cgf.convertType(ops.fullType), ops.lhs,
+                                      ops.rhs, cgf.getBuilder().getUnitAttr());
 }
 
 mlir::Value ScalarExprEmitter::emitShr(const BinOpInfo &ops) {
@@ -1163,8 +1164,8 @@ mlir::Value ScalarExprEmitter::emitShr(const BinOpInfo &ops) {
 
   // Note that we don't need to distinguish unsigned treatment at this
   // point since it will be handled later by LLVM lowering.
-  cgf.cgm.errorNYI("shift ops");
-  return {};
+  return builder.create<cir::ShiftOp>(
+      cgf.getLoc(ops.loc), cgf.convertType(ops.fullType), ops.lhs, ops.rhs);
 }
 
 mlir::Value ScalarExprEmitter::emitAnd(const BinOpInfo &ops) {
diff --git a/clang/lib/CIR/Dialect/IR/CIRDialect.cpp b/clang/lib/CIR/Dialect/IR/CIRDialect.cpp
index cdcfa77b66379..09806cfa1a5f7 100644
--- a/clang/lib/CIR/Dialect/IR/CIRDialect.cpp
+++ b/clang/lib/CIR/Dialect/IR/CIRDialect.cpp
@@ -728,6 +728,9 @@ void cir::FuncOp::print(OpAsmPrinter &p) {
 // been implemented yet.
 mlir::LogicalResult cir::FuncOp::verify() { return success(); }
 
+//===----------------------------------------------------------------------===//
+// BinOp
+//===----------------------------------------------------------------------===//
 LogicalResult cir::BinOp::verify() {
   bool noWrap = getNoUnsignedWrap() || getNoSignedWrap();
   bool saturated = getSaturated();
@@ -759,6 +762,46 @@ LogicalResult cir::BinOp::verify() {
   return mlir::success();
 }
 
+//===----------------------------------------------------------------------===//
+// ShiftOp
+//===----------------------------------------------------------------------===//
+LogicalResult cir::ShiftOp::verify() {
+  mlir::Operation *op = getOperation();
+  mlir::Type resType = getResult().getType();
+  assert(!cir::MissingFeatures::vectorType());
+  bool isOp0Vec = false;
+  bool isOp1Vec = false;
+  if (isOp0Vec != isOp1Vec)
+    return emitOpError() << "input types cannot be one vector and one scalar";
+  if (isOp1Vec && op->getOperand(1).getType() != resType) {
+    return emitOpError() << "shift amount must have the type of the result "
+                         << "if it is vector shift";
+  }
+  return mlir::success();
+}
+
+//===----------------------------------------------------------------------===//
+// SelectOp
+//===----------------------------------------------------------------------===//
+
+OpFoldResult cir::SelectOp::fold(FoldAdaptor adaptor) {
+  mlir::Attribute condition = adaptor.getCondition();
+  if (condition) {
+    bool conditionValue = mlir::cast<cir::BoolAttr>(condition).getValue();
+    return conditionValue ? getTrueValue() : getFalseValue();
+  }
+
+  // cir.select if %0 then x else x -> x
+  mlir::Attribute trueValue = adaptor.getTrueValue();
+  mlir::Attribute falseValue = adaptor.getFalseValue();
+  if (trueValue && trueValue == falseValue)
+    return trueValue;
+  if (getTrueValue() == getFalseValue())
+    return getTrueValue();
+
+  return {};
+}
+
 //===----------------------------------------------------------------------===//
 // UnaryOp
 //===----------------------------------------------------------------------===//
diff --git a/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp b/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
index 1c2b9ad05a132..56da5a96c5c2f 100644
--- a/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
+++ b/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
@@ -1117,6 +1117,91 @@ mlir::LogicalResult CIRToLLVMBinOpLowering::matchAndRewrite(
   return mlir::LogicalResult::success();
 }
 
+mlir::LogicalResult CIRToLLVMShiftOpLowering::matchAndRewrite(
+    cir::ShiftOp op, OpAdaptor adaptor,
+    mlir::ConversionPatternRewriter &rewriter) const {
+  auto cirAmtTy = mlir::dyn_cast<cir::IntType>(op.getAmount().getType());
+  auto cirValTy = mlir::dyn_cast<cir::IntType>(op.getValue().getType());
+
+  // Operands could also be vector type
+  assert(!cir::MissingFeatures::vectorType());
+  mlir::Type llvmTy = getTypeConverter()->convertType(op.getType());
+  mlir::Value amt = adaptor.getAmount();
+  mlir::Value val = adaptor.getValue();
+
+  // TODO(cir): Assert for vector types
+  assert((cirValTy && cirAmtTy) &&
+         "shift input type must be integer or vector type, otherwise NYI");
+
+  assert((cirValTy == op.getType()) && "inconsistent operands' types NYI");
+
+  // Ensure shift amount is the same type as the value. Some undefined
+  // behavior might occur in the casts below as per [C99 6.5.7.3].
+  // Vector type shift amount needs no cast as type consistency is expected to
+  // be already be enforced at CIRGen.
+  if (cirAmtTy)
+    amt = getLLVMIntCast(rewriter, amt, mlir::cast<mlir::IntegerType>(llvmTy),
+                         !cirAmtTy.isSigned(), cirAmtTy.getWidth(),
+                         cirValTy.getWidth());
+
+  // Lower to the proper LLVM shift operation.
+  if (op.getIsShiftleft()) {
+    rewriter.replaceOpWithNewOp<mlir::LLVM::ShlOp>(op, llvmTy, val, amt);
+  } else {
+    assert(!cir::MissingFeatures::vectorType());
+    bool isUnsigned = !cirValTy.isSigned();
+    if (isUnsigned)
+      rewriter.replaceOpWithNewOp<mlir::LLVM::LShrOp>(op, llvmTy, val, amt);
+    else
+      rewriter.replaceOpWithNewOp<mlir::LLVM::AShrOp>(op, llvmTy, val, amt);
+  }
+
+  return mlir::success();
+}
+
+mlir::LogicalResult CIRToLLVMSelectOpLowering::matchAndRewrite(
+    cir::SelectOp op, OpAdaptor adaptor,
+    mlir::ConversionPatternRewriter &rewriter) const {
+  auto getConstantBool = [](mlir::Value value) -> std::optional<bool> {
+    auto definingOp =
+        mlir::dyn_cast_if_present<cir::ConstantOp>(value.getDefiningOp());
+    if (!definingOp)
+      return std::nullopt;
+
+    auto constValue = mlir::dyn_cast<cir::BoolAttr>(definingOp.getValue());
+    if (!constValue)
+      return std::nullopt;
+
+    return constValue.getValue();
+  };
+
+  // Two special cases in the LLVMIR codegen of select op:
+  // - select %0, %1, false => and %0, %1
+  // - select %0, true, %1 => or %0, %1
+  if (mlir::isa<cir::BoolType>(op.getTrueValue().getType())) {
+    std::optional<bool> trueValue = getConstantBool(op.getTrueValue());
+    std::optional<bool> falseValue = getConstantBool(op.getFalseValue());
+    if (falseValue.has_value() && !*falseValue) {
+      // select %0, %1, false => and %0, %1
+      rewriter.replaceOpWithNewOp<mlir::LLVM::AndOp>(op, adaptor.getCondition(),
+                                                     adaptor.getTrueValue());
+      return mlir::success();
+    }
+    if (trueValue.has_value() && *trueValue) {
+      // select %0, true, %1 => or %0, %1
+      rewriter.replaceOpWithNewOp<mlir::LLVM::OrOp>(op, adaptor.getCondition(),
+                                                    adaptor.getFalseValue());
+      return mlir::success();
+    }
+  }
+
+  mlir::Value llvmCondition = adaptor.getCondition();
+  rewriter.replaceOpWithNewOp<mlir::LLVM::SelectOp>(
+      op, llvmCondition, adaptor.getTrueValue(), adaptor.getFalseValue());
+
+  return mlir::success();
+}
+
 static void prepareTypeConverter(mlir::LLVMTypeConverter &converter,
                                  mlir::DataLayout &dataLayout) {
   converter.addConversion([&](cir::PointerType type) -> mlir::Type {
@@ -1259,6 +1344,8 @@ void ConvertCIRToLLVMPass::runOnOperation() {
                CIRToLLVMBrCondOpLowering,
                CIRToLLVMBrOpLowering,
                CIRToLLVMFuncOpLowering,
+               CIRToLLVMSelectOpLowering,
+               CIRToLLVMShiftOpLowering,
                CIRToLLVMTrapOpLowering,
                CIRToLLVMUnaryOpLowering
       // clang-format on
diff --git a/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.h b/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.h
index ef0bb2deaccdf..fbfb29dad8706 100644
--- a/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.h
+++ b/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.h
@@ -189,6 +189,26 @@ class CIRToLLVMBinOpLowering : public mlir::OpConversionPattern<cir::BinOp> {
                   mlir::ConversionPatternRewriter &) const override;
 };
 
+class CIRToLLVMShiftOpLowering
+    : public mlir::OpConversionPattern<cir::ShiftOp> {
+public:
+  using mlir::OpConversionPattern<cir::ShiftOp>::OpConversionPattern;
+
+  mlir::LogicalResult
+  matchAndRewrite(cir::ShiftOp op, OpAdaptor,
+                  mlir::ConversionPatternRewriter &) const override;
+};
+
+class CIRToLLVMSelectOpLowering
+    : public mlir::OpConversionPattern<cir::SelectOp> {
+public:
+  using mlir::OpConversionPattern<cir::SelectOp>::OpConversionPattern;
+
+  mlir::LogicalResult
+  matchAndRewrite(cir::SelectOp op, OpAdaptor,
+                  mlir::ConversionPatternRewriter &) const override;
+};
+
 class CIRToLLVMBrOpLowering : public mlir::OpConversionPattern<cir::BrOp> {
 public:
   using mlir::OpConversionPattern<cir::BrOp>::OpConversionPattern;
diff --git a/clang/test/CIR/CodeGen/binop.cpp b/clang/test/CIR/CodeGen/binop.cpp
index 4c20f79600fac..22ee5d1cd1148 100644
--- a/clang/test/CIR/CodeGen/binop.cpp
+++ b/clang/test/CIR/CodeGen/binop.cpp
@@ -1,5 +1,9 @@
-// RUN: %clang_cc1 -std=c++20 -triple x86_64-unknown-linux-gnu -O1 -Wno-unused-value -fclangir -emit-cir %s -o %t.cir
-// RUN: FileCheck --input-file=%t.cir %s
+// RUN: %clang_cc1 -std=c++11 -triple x86_64-unknown-linux-gnu -Wno-unused-value -fclangir -emit-cir %s -o %t.cir
+// RUN: FileCheck --input-file=%t.cir %s -check-prefix=CIR
+// RUN: %clang_cc1 -std=c++11 -triple x86_64-unknown-linux-gnu -Wno-unused-value -fclangir -emit-llvm %s -o %t-cir.ll
+// RUN: FileCheck --input-file=%t-cir.ll %s -check-prefix=LLVM
+// RUN: %clang_cc1 -std=c++11 -triple x86_64-unknown-linux-gnu -Wno-unused-value -emit-llvm %s -o %t.ll
+// RUN: FileCheck --input-file=%t.ll %s -check-prefix=OGCG
 
 void b0(int a, int b) {
   int x = a * b;
@@ -12,22 +16,307 @@ void b0(int a, int b) {
   x = x | b;
 }
 
-// CHECK: %{{.+}} = cir.binop(mul, %{{.+}}, %{{.+}}) nsw : !s32i
-// CHECK: %{{.+}} = cir.binop(div, %{{.+}}, %{{.+}}) : !s32i
-// CHECK: %{{.+}} = cir.binop(rem, %{{.+}}, %{{.+}}) : !s32i
-// CHECK: %{{.+}} = cir.binop(add, %{{.+}}, %{{.+}}) nsw : !s32i
-// CHECK: %{{.+}} = cir.binop(sub, %{{.+}}, %{{.+}}) nsw : !s32i
-// CHECK: %{{.+}} = cir.binop(and, %{{.+}}, %{{.+}}) : !s32i
-// CHECK: %{{.+}} = cir.binop(xor, %{{.+}}, %{{.+}}) : !s32i
-// CHECK: %{{.+}} = cir.binop(or, %{{.+}}, %{{.+}}) : !s32i
+// CIR-LABEL: cir.func @b0(
+// CIR: %{{.+}} = cir.binop(mul, %{{.+}}, %{{.+}}) nsw : !s32i
+// CIR: %{{.+}} = cir.binop(div, %{{.+}}, %{{.+}}) : !s32i
+// CIR: %{{.+}} = cir.binop(rem, %{{.+}}, %{{.+}}) : !s32i
+// CIR: %{{.+}} = cir.binop(add, %{{.+}}, %{{.+}}) nsw : !s32i
+// CIR: %{{.+}} = cir.binop(sub, %{{.+}}, %{{.+}}) nsw : !s32i
+// CIR: %{{.+}} = cir.binop(and, %{{.+}}, %{{.+}}) : !s32i
+// CIR: %{{.+}} = cir.binop(xor, %{{.+}}, %{{.+}}) : !s32i
+// CIR: %{{.+}} = cir.binop(or, %{{.+}}, %{{.+}}) : !s32i
+// CIR: cir.return
+
+// LLVM-LABEL: define void @b0(
+// LLVM-SAME: i32 %[[A:.*]], i32 %[[B:.*]])
+// LLVM:         %[[A_ADDR:.*]] = alloca i32
+// LLVM:         %[[B_ADDR:.*]] = alloca i32
+// LLVM:         %[[X:.*]] = alloca i32
+// LLVM:         store i32 %[[A]], ptr %[[A_ADDR]]
+// LLVM:         store i32 %[[B]], ptr %[[B_ADDR]]
+
+// LLVM:         %[[A:.*]] = load i32, ptr %[[A_ADDR]]
+// LLVM:         %[[B:.*]] = load i32, ptr %[[B_ADDR]]
+// LLVM:         %[[MUL:.*]] = mul nsw i32 %[[A]], %[[B]]
+// LLVM:         store i32 %[[MUL]], ptr %[[X]]
+
+// LLVM:         %[[X1:.*]] = load i32, ptr %[[X]]
+// LLVM:         %[[B1:.*]] = load i32, ptr %[[B_ADDR]]
+// LLVM:         %[[DIV:.*]] = sdiv i32 %[[X1]], %[[B1]]
+// LLVM:         store i32 %[[DIV]], ptr %[[X]]
+
+// LLVM:         %[[X2:.*]] = load i32, ptr %[[X]]
+// LLVM:         %[[B2:.*]] = load i32, ptr %[[B_ADDR]]
+// LLVM:         %[[REM:.*]] = srem i32 %[[X2]], %[[B2]]
+// LLVM:         store i32 %[[REM]], ptr %[[X]]
+
+// LLVM:         %[[X3:.*]] = load i32, ptr %[[X]]
+// LLVM:         %[[B3:.*]] = load i32, ptr %[[B_ADDR]]
+// LLVM:         %[[ADD:.*]] = add nsw i32 %[[X3]], %[[B3]]
+// LLVM:         store i32 %[[ADD]], ptr %[[X]]
+
+// LLVM:         %[[X4:.*]] = load i32, ptr %[[X]]
+// LLVM:         %[[B4:.*]] = load i32, ptr %[[B_ADDR]]
+// LLVM:         %[[SUB:.*]] = sub nsw i32 %[[X4]], %[[B4]]
+// LLVM:         store i32 %[[SUB]], ptr %[[X]]
+
+// LLVM:         %[[X5:.*]] = load i32, ptr %[[X]]
+// LLVM:         %[[B5:.*]] = load i32, ptr %[[B_ADDR]]
+// LLVM:         %[[AND:.*]] = and i32 %[[X5]], %[[B5]]
+// LLVM:         store i32 %[[AND]], ptr %[[X]]
+
+// LLVM:         %[[X6:.*]] = load i32, ptr %[[X]]
+// LLVM:         %[[B6:.*]] = load i32, ptr %[[B_ADDR]]
+// LLVM:         %[[XOR:.*]] = xor i32 %[[X6]], %[[B6]]
+// LLVM:         store i32 %[[XOR]], ptr %[[X]]
+
+// LLVM:         %[[X7:.*]] = load i32, ptr %[[X]]
+// LLVM:         %[[B7:.*]] = load i32, ptr %[[B_ADDR]]
+// LLVM:         %[[OR:.*]] = or i32 %[[X7]], %[[B7]]
+// LLVM:         store i32 %[[OR]], ptr %[[X]]
+
+// LLVM:         ret void
+
+// OGCG-LABEL: define dso_local void @_Z2b0ii(i32 {{.*}} %a, i32 {{.*}} %b) {{.*}} { 
+// OGCG:         %[[A_ADDR:.*]] = alloca i32
+// OGCG:         %[[B_ADDR:.*]] = alloca i32
+// OGCG:         %[[X:.*]] = alloca i32
+// OGCG:         store i32 %a, ptr %[[A_ADDR]]
+// OGCG:         store i32 %b, ptr %[[B_ADDR]]
+
+// OGCG:         %[[A:.*]] = load i32, ptr %[[A_ADDR]]
+// OGCG:         %[[B:.*]] = load i32, ptr %[[B_ADDR]]
+// OGCG:         %[[MUL:.*]] = mul nsw i32 %[[A]], %[[B]]
+// OGCG:         store i32 %[[MUL]], ptr %[[X]]
+
+// OGCG:         %[[X1:.*]] = load i32, ptr %[[X]]
+// OGCG:         %[[B1:.*]] = load i32, ptr %[[B_ADDR]]
+// OGCG:         %[[DIV:.*]] = sdiv i32 %[[X1]], %[[B1]]
+// OGCG:         store i32 %[[DIV]], ptr %[[X]]
+
+// OGCG:         %[[X2:.*]] = load i32, ptr %[[X]]
+// OGCG:         %[[B2:.*]] = load i32, ptr %[[B_ADDR]]
+// OGCG:         %[[REM:.*]] = srem i32 %[[X2]], %[[B2]]
+// OGCG:         store i32 %[[REM]], ptr %[[X]]
+
+// OGCG:         %[[X3:.*]] = load i32, ptr %[[X]]
+// OGCG:         %[[B3:.*]] = load i32, ptr %[[B_ADDR]]
+// OGCG:         %[[ADD:.*]] = add nsw i32 %[[X3]], %[[B3]]
+// OGCG:         store i32 %[[ADD]], ptr %[[X]]
+
+// OGCG:         %[[X4:.*]] = load i32, ptr %[[X]]
+// OGCG:         %[[B4:.*]] = load i32, ptr %[[B_ADDR]]
+// OGCG:         %[[SUB:.*]] = sub nsw i32 %[[X4]], %[[B4]]
+// OGCG:         store i32 %[[SUB]], ptr %[[X]]
+
+// OGCG:         %[[X5:.*]] = load i32, ptr %[[X]]
+// OGCG:         %[[B5:.*]] = load i32, ptr %[[B_ADDR]]
+// OGCG:         %[[AND:.*]] = and i32 %[[X5]], %[[B5]]
+// OGCG:         store i32 %[[AND]], ptr %[[X]]
+
+// OGCG:         %[[X6:.*]] = load i32, ptr %[[X]]
+// OGCG:         %[[B6:.*]] = load i32, ptr %[[B_ADDR]]
+// OGCG:         %[[XOR:.*]] = xor i32 %[[X6]], %[[B6]]
+// OGCG:         store i32 %[[XOR]], ptr %[[X]]
+
+// OGCG:         %[[X7:.*]] = load i32, ptr %[[X]]
+// OGCG:         %[[B7:.*]] = load i32, ptr %[[B_ADDR]]
+// OGCG:         %[[OR:.*]] = or i32 %[[X7]], %[[B7]]
+// OGCG:         store i32 %[[OR]], ptr %[[X]]
+
+// OGCG:         ret void
 
 void testFloatingPointBinOps(float a, float b) {
   a * b;
-  // CHECK: cir.binop(mul, %{{.+}}, %{{.+}}) : !cir.float
   a / b;
-  // CHECK: cir.binop(div, %{{.+}}, %{{.+}}) : !cir.float
   a + b;
-  // CHECK: cir.binop(add, %{{.+}}, %{{.+}}) : !cir.float
   a - b;
-  // CHECK: cir.binop(sub, %{{.+}}, %{{.+}}) : !cir.float
 }
+
+// CIR-LABEL: cir.func @testFloatingPointBinOps(
+// CIR: cir.binop(mul, %{{.+}}, %{{.+}}) : !cir.float
+// CIR: cir.binop(div, %{{.+}}, %{{.+}}) : !cir.float
+// CIR: cir.binop(add, %{{.+}}, %{{.+}}) : !cir.float
+// CIR: cir.binop(sub, %{{.+}}, %{{.+}}) : !cir.float
+// CIR: cir.return
+
+// LLVM-LABEL: define void @testFloatingPointBinOps(
+// LLVM-SAME: float %[[A:.*]], float %[[B:.*]])
+// LLVM:         %[[A_ADDR:.*]] = alloca float, i64 1
+// LLVM:         %[[B_ADDR:.*]] = alloca float, i64 1
+// LLVM:         store float %[[A]], ptr %[[A_ADDR]]
+// LLVM:         store float %[[B]], ptr %[[B_ADDR]]
+
+// LLVM:         %[[A1:.*]] = load float, ptr %[[A_ADDR]]
+// LLVM:         %[[B1:.*]] = load float, ptr %[[B_ADDR]]
+// LLVM:         fmul float %[[A1]], %[[B1]]
+
+// LLVM:         %[[A2:.*]] = load float, ptr %[[A_ADDR]]
+// LLVM:         %[[B2:.*]] = load float, ptr %[[B_ADDR]]
+// LLVM:         fdiv float %[[A2]], %[[B2]]
+
+// LLVM:         %[[A3:.*]] = load float, ptr %[[A_ADDR]]
+// LLVM:         %[[B3:.*]] = load float, ptr %[[B_ADDR]]
+// LLVM:         fadd float %[[A3]], %[[B3]]
+
+// LLVM:         %[[A4:.*]] = load float, ptr %[[A_ADDR]]
+// LLVM:         %[[B4:.*]] = load float, ptr %[[B_ADDR]]
+// LLVM:         fsub float %[[A4]], %[[B4]]
+
+// LLVM:         ret void
+
+// OGCG-LABEL: define dso_local void @_Z23testFloatingPointBinOpsff(float {{.*}} %a, float {{.*}} %b)
+// OGCG:         %a.addr = alloca float
+// OGCG:         %b.addr = alloca flo...
[truncated]

@llvmbot
Copy link
Member

llvmbot commented Mar 28, 2025

@llvm/pr-subscribers-clang

Author: Morris Hafner (mmha)

Changes

Since SelectOp will only generated by a future pass that transforms a TernaryOp this only includes the lowering bits.

This patch also improves the testing of the existing binary operators.


Patch is 28.28 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/133405.diff

7 Files Affected:

  • (modified) clang/include/clang/CIR/Dialect/IR/CIROps.td (+73)
  • (modified) clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp (+5-4)
  • (modified) clang/lib/CIR/Dialect/IR/CIRDialect.cpp (+43)
  • (modified) clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp (+87)
  • (modified) clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.h (+20)
  • (modified) clang/test/CIR/CodeGen/binop.cpp (+303-14)
  • (added) clang/test/CIR/Lowering/select.cir (+48)
diff --git a/clang/include/clang/CIR/Dialect/IR/CIROps.td b/clang/include/clang/CIR/Dialect/IR/CIROps.td
index 455cc2b8b0277..5c3d0549c1c47 100644
--- a/clang/include/clang/CIR/Dialect/IR/CIROps.td
+++ b/clang/include/clang/CIR/Dialect/IR/CIROps.td
@@ -889,6 +889,79 @@ def BinOp : CIR_Op<"binop", [Pure,
   let hasVerifier = 1;
 }
 
+//===----------------------------------------------------------------------===//
+// ShiftOp
+//===----------------------------------------------------------------------===//
+
+def ShiftOp : CIR_Op<"shift", [Pure]> {
+  let summary = "Shift";
+  let description = [{
+    Shift `left` or `right`, according to the first operand. Second operand is
+    the shift target and the third the amount. Second and the thrid operand are
+    integers.
+
+    ```mlir
+    %7 = cir.shift(left, %1 : !u64i, %4 : !s32i) -> !u64i
+    ```
+  }];
+
+  // TODO(cir): Support vectors. CIR_IntType -> CIR_AnyIntOrVecOfInt. Also
+  // update the description above.
+  let results = (outs CIR_IntType:$result);
+  let arguments = (ins CIR_IntType:$value, CIR_IntType:$amount,
+                       UnitAttr:$isShiftleft);
+
+  let assemblyFormat = [{
+    `(`
+      (`left` $isShiftleft^) : (```right`)?
+      `,` $value `:` type($value)
+      `,` $amount `:` type($amount)
+    `)` `->` type($result) attr-dict
+  }];
+
+  let hasVerifier = 1;
+}
+
+//===----------------------------------------------------------------------===//
+// SelectOp
+//===----------------------------------------------------------------------===//
+
+def SelectOp : CIR_Op<"select", [Pure,
+    AllTypesMatch<["true_value", "false_value", "result"]>]> {
+  let summary = "Yield one of two values based on a boolean value";
+  let description = [{
+    The `cir.select` operation takes three operands. The first operand
+    `condition` is a boolean value of type `!cir.bool`. The second and the third
+    operand can be of any CIR types, but their types must be the same. If the
+    first operand is `true`, the operation yields its second operand. Otherwise,
+    the operation yields its third operand.
+
+    Example:
+
+    ```mlir
+    %0 = cir.const #cir.bool<true> : !cir.bool
+    %1 = cir.const #cir.int<42> : !s32i
+    %2 = cir.const #cir.int<72> : !s32i
+    %3 = cir.select if %0 then %1 else %2 : (!cir.bool, !s32i, !s32i) -> !s32i
+    ```
+  }];
+
+  let arguments = (ins CIR_BoolType:$condition, CIR_AnyType:$true_value,
+                       CIR_AnyType:$false_value);
+  let results = (outs CIR_AnyType:$result);
+
+  let assemblyFormat = [{
+    `if` $condition `then` $true_value `else` $false_value
+    `:` `(`
+      qualified(type($condition)) `,`
+      qualified(type($true_value)) `,`
+      qualified(type($false_value))
+    `)` `->` qualified(type($result)) attr-dict
+  }];
+
+  let hasFolder = 1;
+}
+
 //===----------------------------------------------------------------------===//
 // GlobalOp
 //===----------------------------------------------------------------------===//
diff --git a/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp b/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp
index 52bd3b2933744..3a904fa656104 100644
--- a/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp
@@ -1138,8 +1138,9 @@ mlir::Value ScalarExprEmitter::emitShl(const BinOpInfo &ops) {
            mlir::isa<cir::IntType>(ops.lhs.getType()))
     cgf.cgm.errorNYI("sanitizers");
 
-  cgf.cgm.errorNYI("shift ops");
-  return {};
+  return builder.create<cir::ShiftOp>(cgf.getLoc(ops.loc),
+                                      cgf.convertType(ops.fullType), ops.lhs,
+                                      ops.rhs, cgf.getBuilder().getUnitAttr());
 }
 
 mlir::Value ScalarExprEmitter::emitShr(const BinOpInfo &ops) {
@@ -1163,8 +1164,8 @@ mlir::Value ScalarExprEmitter::emitShr(const BinOpInfo &ops) {
 
   // Note that we don't need to distinguish unsigned treatment at this
   // point since it will be handled later by LLVM lowering.
-  cgf.cgm.errorNYI("shift ops");
-  return {};
+  return builder.create<cir::ShiftOp>(
+      cgf.getLoc(ops.loc), cgf.convertType(ops.fullType), ops.lhs, ops.rhs);
 }
 
 mlir::Value ScalarExprEmitter::emitAnd(const BinOpInfo &ops) {
diff --git a/clang/lib/CIR/Dialect/IR/CIRDialect.cpp b/clang/lib/CIR/Dialect/IR/CIRDialect.cpp
index cdcfa77b66379..09806cfa1a5f7 100644
--- a/clang/lib/CIR/Dialect/IR/CIRDialect.cpp
+++ b/clang/lib/CIR/Dialect/IR/CIRDialect.cpp
@@ -728,6 +728,9 @@ void cir::FuncOp::print(OpAsmPrinter &p) {
 // been implemented yet.
 mlir::LogicalResult cir::FuncOp::verify() { return success(); }
 
+//===----------------------------------------------------------------------===//
+// BinOp
+//===----------------------------------------------------------------------===//
 LogicalResult cir::BinOp::verify() {
   bool noWrap = getNoUnsignedWrap() || getNoSignedWrap();
   bool saturated = getSaturated();
@@ -759,6 +762,46 @@ LogicalResult cir::BinOp::verify() {
   return mlir::success();
 }
 
+//===----------------------------------------------------------------------===//
+// ShiftOp
+//===----------------------------------------------------------------------===//
+LogicalResult cir::ShiftOp::verify() {
+  mlir::Operation *op = getOperation();
+  mlir::Type resType = getResult().getType();
+  assert(!cir::MissingFeatures::vectorType());
+  bool isOp0Vec = false;
+  bool isOp1Vec = false;
+  if (isOp0Vec != isOp1Vec)
+    return emitOpError() << "input types cannot be one vector and one scalar";
+  if (isOp1Vec && op->getOperand(1).getType() != resType) {
+    return emitOpError() << "shift amount must have the type of the result "
+                         << "if it is vector shift";
+  }
+  return mlir::success();
+}
+
+//===----------------------------------------------------------------------===//
+// SelectOp
+//===----------------------------------------------------------------------===//
+
+OpFoldResult cir::SelectOp::fold(FoldAdaptor adaptor) {
+  mlir::Attribute condition = adaptor.getCondition();
+  if (condition) {
+    bool conditionValue = mlir::cast<cir::BoolAttr>(condition).getValue();
+    return conditionValue ? getTrueValue() : getFalseValue();
+  }
+
+  // cir.select if %0 then x else x -> x
+  mlir::Attribute trueValue = adaptor.getTrueValue();
+  mlir::Attribute falseValue = adaptor.getFalseValue();
+  if (trueValue && trueValue == falseValue)
+    return trueValue;
+  if (getTrueValue() == getFalseValue())
+    return getTrueValue();
+
+  return {};
+}
+
 //===----------------------------------------------------------------------===//
 // UnaryOp
 //===----------------------------------------------------------------------===//
diff --git a/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp b/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
index 1c2b9ad05a132..56da5a96c5c2f 100644
--- a/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
+++ b/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
@@ -1117,6 +1117,91 @@ mlir::LogicalResult CIRToLLVMBinOpLowering::matchAndRewrite(
   return mlir::LogicalResult::success();
 }
 
+mlir::LogicalResult CIRToLLVMShiftOpLowering::matchAndRewrite(
+    cir::ShiftOp op, OpAdaptor adaptor,
+    mlir::ConversionPatternRewriter &rewriter) const {
+  auto cirAmtTy = mlir::dyn_cast<cir::IntType>(op.getAmount().getType());
+  auto cirValTy = mlir::dyn_cast<cir::IntType>(op.getValue().getType());
+
+  // Operands could also be vector type
+  assert(!cir::MissingFeatures::vectorType());
+  mlir::Type llvmTy = getTypeConverter()->convertType(op.getType());
+  mlir::Value amt = adaptor.getAmount();
+  mlir::Value val = adaptor.getValue();
+
+  // TODO(cir): Assert for vector types
+  assert((cirValTy && cirAmtTy) &&
+         "shift input type must be integer or vector type, otherwise NYI");
+
+  assert((cirValTy == op.getType()) && "inconsistent operands' types NYI");
+
+  // Ensure shift amount is the same type as the value. Some undefined
+  // behavior might occur in the casts below as per [C99 6.5.7.3].
+  // Vector type shift amount needs no cast as type consistency is expected to
+  // be already be enforced at CIRGen.
+  if (cirAmtTy)
+    amt = getLLVMIntCast(rewriter, amt, mlir::cast<mlir::IntegerType>(llvmTy),
+                         !cirAmtTy.isSigned(), cirAmtTy.getWidth(),
+                         cirValTy.getWidth());
+
+  // Lower to the proper LLVM shift operation.
+  if (op.getIsShiftleft()) {
+    rewriter.replaceOpWithNewOp<mlir::LLVM::ShlOp>(op, llvmTy, val, amt);
+  } else {
+    assert(!cir::MissingFeatures::vectorType());
+    bool isUnsigned = !cirValTy.isSigned();
+    if (isUnsigned)
+      rewriter.replaceOpWithNewOp<mlir::LLVM::LShrOp>(op, llvmTy, val, amt);
+    else
+      rewriter.replaceOpWithNewOp<mlir::LLVM::AShrOp>(op, llvmTy, val, amt);
+  }
+
+  return mlir::success();
+}
+
+mlir::LogicalResult CIRToLLVMSelectOpLowering::matchAndRewrite(
+    cir::SelectOp op, OpAdaptor adaptor,
+    mlir::ConversionPatternRewriter &rewriter) const {
+  auto getConstantBool = [](mlir::Value value) -> std::optional<bool> {
+    auto definingOp =
+        mlir::dyn_cast_if_present<cir::ConstantOp>(value.getDefiningOp());
+    if (!definingOp)
+      return std::nullopt;
+
+    auto constValue = mlir::dyn_cast<cir::BoolAttr>(definingOp.getValue());
+    if (!constValue)
+      return std::nullopt;
+
+    return constValue.getValue();
+  };
+
+  // Two special cases in the LLVMIR codegen of select op:
+  // - select %0, %1, false => and %0, %1
+  // - select %0, true, %1 => or %0, %1
+  if (mlir::isa<cir::BoolType>(op.getTrueValue().getType())) {
+    std::optional<bool> trueValue = getConstantBool(op.getTrueValue());
+    std::optional<bool> falseValue = getConstantBool(op.getFalseValue());
+    if (falseValue.has_value() && !*falseValue) {
+      // select %0, %1, false => and %0, %1
+      rewriter.replaceOpWithNewOp<mlir::LLVM::AndOp>(op, adaptor.getCondition(),
+                                                     adaptor.getTrueValue());
+      return mlir::success();
+    }
+    if (trueValue.has_value() && *trueValue) {
+      // select %0, true, %1 => or %0, %1
+      rewriter.replaceOpWithNewOp<mlir::LLVM::OrOp>(op, adaptor.getCondition(),
+                                                    adaptor.getFalseValue());
+      return mlir::success();
+    }
+  }
+
+  mlir::Value llvmCondition = adaptor.getCondition();
+  rewriter.replaceOpWithNewOp<mlir::LLVM::SelectOp>(
+      op, llvmCondition, adaptor.getTrueValue(), adaptor.getFalseValue());
+
+  return mlir::success();
+}
+
 static void prepareTypeConverter(mlir::LLVMTypeConverter &converter,
                                  mlir::DataLayout &dataLayout) {
   converter.addConversion([&](cir::PointerType type) -> mlir::Type {
@@ -1259,6 +1344,8 @@ void ConvertCIRToLLVMPass::runOnOperation() {
                CIRToLLVMBrCondOpLowering,
                CIRToLLVMBrOpLowering,
                CIRToLLVMFuncOpLowering,
+               CIRToLLVMSelectOpLowering,
+               CIRToLLVMShiftOpLowering,
                CIRToLLVMTrapOpLowering,
                CIRToLLVMUnaryOpLowering
       // clang-format on
diff --git a/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.h b/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.h
index ef0bb2deaccdf..fbfb29dad8706 100644
--- a/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.h
+++ b/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.h
@@ -189,6 +189,26 @@ class CIRToLLVMBinOpLowering : public mlir::OpConversionPattern<cir::BinOp> {
                   mlir::ConversionPatternRewriter &) const override;
 };
 
+class CIRToLLVMShiftOpLowering
+    : public mlir::OpConversionPattern<cir::ShiftOp> {
+public:
+  using mlir::OpConversionPattern<cir::ShiftOp>::OpConversionPattern;
+
+  mlir::LogicalResult
+  matchAndRewrite(cir::ShiftOp op, OpAdaptor,
+                  mlir::ConversionPatternRewriter &) const override;
+};
+
+class CIRToLLVMSelectOpLowering
+    : public mlir::OpConversionPattern<cir::SelectOp> {
+public:
+  using mlir::OpConversionPattern<cir::SelectOp>::OpConversionPattern;
+
+  mlir::LogicalResult
+  matchAndRewrite(cir::SelectOp op, OpAdaptor,
+                  mlir::ConversionPatternRewriter &) const override;
+};
+
 class CIRToLLVMBrOpLowering : public mlir::OpConversionPattern<cir::BrOp> {
 public:
   using mlir::OpConversionPattern<cir::BrOp>::OpConversionPattern;
diff --git a/clang/test/CIR/CodeGen/binop.cpp b/clang/test/CIR/CodeGen/binop.cpp
index 4c20f79600fac..22ee5d1cd1148 100644
--- a/clang/test/CIR/CodeGen/binop.cpp
+++ b/clang/test/CIR/CodeGen/binop.cpp
@@ -1,5 +1,9 @@
-// RUN: %clang_cc1 -std=c++20 -triple x86_64-unknown-linux-gnu -O1 -Wno-unused-value -fclangir -emit-cir %s -o %t.cir
-// RUN: FileCheck --input-file=%t.cir %s
+// RUN: %clang_cc1 -std=c++11 -triple x86_64-unknown-linux-gnu -Wno-unused-value -fclangir -emit-cir %s -o %t.cir
+// RUN: FileCheck --input-file=%t.cir %s -check-prefix=CIR
+// RUN: %clang_cc1 -std=c++11 -triple x86_64-unknown-linux-gnu -Wno-unused-value -fclangir -emit-llvm %s -o %t-cir.ll
+// RUN: FileCheck --input-file=%t-cir.ll %s -check-prefix=LLVM
+// RUN: %clang_cc1 -std=c++11 -triple x86_64-unknown-linux-gnu -Wno-unused-value -emit-llvm %s -o %t.ll
+// RUN: FileCheck --input-file=%t.ll %s -check-prefix=OGCG
 
 void b0(int a, int b) {
   int x = a * b;
@@ -12,22 +16,307 @@ void b0(int a, int b) {
   x = x | b;
 }
 
-// CHECK: %{{.+}} = cir.binop(mul, %{{.+}}, %{{.+}}) nsw : !s32i
-// CHECK: %{{.+}} = cir.binop(div, %{{.+}}, %{{.+}}) : !s32i
-// CHECK: %{{.+}} = cir.binop(rem, %{{.+}}, %{{.+}}) : !s32i
-// CHECK: %{{.+}} = cir.binop(add, %{{.+}}, %{{.+}}) nsw : !s32i
-// CHECK: %{{.+}} = cir.binop(sub, %{{.+}}, %{{.+}}) nsw : !s32i
-// CHECK: %{{.+}} = cir.binop(and, %{{.+}}, %{{.+}}) : !s32i
-// CHECK: %{{.+}} = cir.binop(xor, %{{.+}}, %{{.+}}) : !s32i
-// CHECK: %{{.+}} = cir.binop(or, %{{.+}}, %{{.+}}) : !s32i
+// CIR-LABEL: cir.func @b0(
+// CIR: %{{.+}} = cir.binop(mul, %{{.+}}, %{{.+}}) nsw : !s32i
+// CIR: %{{.+}} = cir.binop(div, %{{.+}}, %{{.+}}) : !s32i
+// CIR: %{{.+}} = cir.binop(rem, %{{.+}}, %{{.+}}) : !s32i
+// CIR: %{{.+}} = cir.binop(add, %{{.+}}, %{{.+}}) nsw : !s32i
+// CIR: %{{.+}} = cir.binop(sub, %{{.+}}, %{{.+}}) nsw : !s32i
+// CIR: %{{.+}} = cir.binop(and, %{{.+}}, %{{.+}}) : !s32i
+// CIR: %{{.+}} = cir.binop(xor, %{{.+}}, %{{.+}}) : !s32i
+// CIR: %{{.+}} = cir.binop(or, %{{.+}}, %{{.+}}) : !s32i
+// CIR: cir.return
+
+// LLVM-LABEL: define void @b0(
+// LLVM-SAME: i32 %[[A:.*]], i32 %[[B:.*]])
+// LLVM:         %[[A_ADDR:.*]] = alloca i32
+// LLVM:         %[[B_ADDR:.*]] = alloca i32
+// LLVM:         %[[X:.*]] = alloca i32
+// LLVM:         store i32 %[[A]], ptr %[[A_ADDR]]
+// LLVM:         store i32 %[[B]], ptr %[[B_ADDR]]
+
+// LLVM:         %[[A:.*]] = load i32, ptr %[[A_ADDR]]
+// LLVM:         %[[B:.*]] = load i32, ptr %[[B_ADDR]]
+// LLVM:         %[[MUL:.*]] = mul nsw i32 %[[A]], %[[B]]
+// LLVM:         store i32 %[[MUL]], ptr %[[X]]
+
+// LLVM:         %[[X1:.*]] = load i32, ptr %[[X]]
+// LLVM:         %[[B1:.*]] = load i32, ptr %[[B_ADDR]]
+// LLVM:         %[[DIV:.*]] = sdiv i32 %[[X1]], %[[B1]]
+// LLVM:         store i32 %[[DIV]], ptr %[[X]]
+
+// LLVM:         %[[X2:.*]] = load i32, ptr %[[X]]
+// LLVM:         %[[B2:.*]] = load i32, ptr %[[B_ADDR]]
+// LLVM:         %[[REM:.*]] = srem i32 %[[X2]], %[[B2]]
+// LLVM:         store i32 %[[REM]], ptr %[[X]]
+
+// LLVM:         %[[X3:.*]] = load i32, ptr %[[X]]
+// LLVM:         %[[B3:.*]] = load i32, ptr %[[B_ADDR]]
+// LLVM:         %[[ADD:.*]] = add nsw i32 %[[X3]], %[[B3]]
+// LLVM:         store i32 %[[ADD]], ptr %[[X]]
+
+// LLVM:         %[[X4:.*]] = load i32, ptr %[[X]]
+// LLVM:         %[[B4:.*]] = load i32, ptr %[[B_ADDR]]
+// LLVM:         %[[SUB:.*]] = sub nsw i32 %[[X4]], %[[B4]]
+// LLVM:         store i32 %[[SUB]], ptr %[[X]]
+
+// LLVM:         %[[X5:.*]] = load i32, ptr %[[X]]
+// LLVM:         %[[B5:.*]] = load i32, ptr %[[B_ADDR]]
+// LLVM:         %[[AND:.*]] = and i32 %[[X5]], %[[B5]]
+// LLVM:         store i32 %[[AND]], ptr %[[X]]
+
+// LLVM:         %[[X6:.*]] = load i32, ptr %[[X]]
+// LLVM:         %[[B6:.*]] = load i32, ptr %[[B_ADDR]]
+// LLVM:         %[[XOR:.*]] = xor i32 %[[X6]], %[[B6]]
+// LLVM:         store i32 %[[XOR]], ptr %[[X]]
+
+// LLVM:         %[[X7:.*]] = load i32, ptr %[[X]]
+// LLVM:         %[[B7:.*]] = load i32, ptr %[[B_ADDR]]
+// LLVM:         %[[OR:.*]] = or i32 %[[X7]], %[[B7]]
+// LLVM:         store i32 %[[OR]], ptr %[[X]]
+
+// LLVM:         ret void
+
+// OGCG-LABEL: define dso_local void @_Z2b0ii(i32 {{.*}} %a, i32 {{.*}} %b) {{.*}} { 
+// OGCG:         %[[A_ADDR:.*]] = alloca i32
+// OGCG:         %[[B_ADDR:.*]] = alloca i32
+// OGCG:         %[[X:.*]] = alloca i32
+// OGCG:         store i32 %a, ptr %[[A_ADDR]]
+// OGCG:         store i32 %b, ptr %[[B_ADDR]]
+
+// OGCG:         %[[A:.*]] = load i32, ptr %[[A_ADDR]]
+// OGCG:         %[[B:.*]] = load i32, ptr %[[B_ADDR]]
+// OGCG:         %[[MUL:.*]] = mul nsw i32 %[[A]], %[[B]]
+// OGCG:         store i32 %[[MUL]], ptr %[[X]]
+
+// OGCG:         %[[X1:.*]] = load i32, ptr %[[X]]
+// OGCG:         %[[B1:.*]] = load i32, ptr %[[B_ADDR]]
+// OGCG:         %[[DIV:.*]] = sdiv i32 %[[X1]], %[[B1]]
+// OGCG:         store i32 %[[DIV]], ptr %[[X]]
+
+// OGCG:         %[[X2:.*]] = load i32, ptr %[[X]]
+// OGCG:         %[[B2:.*]] = load i32, ptr %[[B_ADDR]]
+// OGCG:         %[[REM:.*]] = srem i32 %[[X2]], %[[B2]]
+// OGCG:         store i32 %[[REM]], ptr %[[X]]
+
+// OGCG:         %[[X3:.*]] = load i32, ptr %[[X]]
+// OGCG:         %[[B3:.*]] = load i32, ptr %[[B_ADDR]]
+// OGCG:         %[[ADD:.*]] = add nsw i32 %[[X3]], %[[B3]]
+// OGCG:         store i32 %[[ADD]], ptr %[[X]]
+
+// OGCG:         %[[X4:.*]] = load i32, ptr %[[X]]
+// OGCG:         %[[B4:.*]] = load i32, ptr %[[B_ADDR]]
+// OGCG:         %[[SUB:.*]] = sub nsw i32 %[[X4]], %[[B4]]
+// OGCG:         store i32 %[[SUB]], ptr %[[X]]
+
+// OGCG:         %[[X5:.*]] = load i32, ptr %[[X]]
+// OGCG:         %[[B5:.*]] = load i32, ptr %[[B_ADDR]]
+// OGCG:         %[[AND:.*]] = and i32 %[[X5]], %[[B5]]
+// OGCG:         store i32 %[[AND]], ptr %[[X]]
+
+// OGCG:         %[[X6:.*]] = load i32, ptr %[[X]]
+// OGCG:         %[[B6:.*]] = load i32, ptr %[[B_ADDR]]
+// OGCG:         %[[XOR:.*]] = xor i32 %[[X6]], %[[B6]]
+// OGCG:         store i32 %[[XOR]], ptr %[[X]]
+
+// OGCG:         %[[X7:.*]] = load i32, ptr %[[X]]
+// OGCG:         %[[B7:.*]] = load i32, ptr %[[B_ADDR]]
+// OGCG:         %[[OR:.*]] = or i32 %[[X7]], %[[B7]]
+// OGCG:         store i32 %[[OR]], ptr %[[X]]
+
+// OGCG:         ret void
 
 void testFloatingPointBinOps(float a, float b) {
   a * b;
-  // CHECK: cir.binop(mul, %{{.+}}, %{{.+}}) : !cir.float
   a / b;
-  // CHECK: cir.binop(div, %{{.+}}, %{{.+}}) : !cir.float
   a + b;
-  // CHECK: cir.binop(add, %{{.+}}, %{{.+}}) : !cir.float
   a - b;
-  // CHECK: cir.binop(sub, %{{.+}}, %{{.+}}) : !cir.float
 }
+
+// CIR-LABEL: cir.func @testFloatingPointBinOps(
+// CIR: cir.binop(mul, %{{.+}}, %{{.+}}) : !cir.float
+// CIR: cir.binop(div, %{{.+}}, %{{.+}}) : !cir.float
+// CIR: cir.binop(add, %{{.+}}, %{{.+}}) : !cir.float
+// CIR: cir.binop(sub, %{{.+}}, %{{.+}}) : !cir.float
+// CIR: cir.return
+
+// LLVM-LABEL: define void @testFloatingPointBinOps(
+// LLVM-SAME: float %[[A:.*]], float %[[B:.*]])
+// LLVM:         %[[A_ADDR:.*]] = alloca float, i64 1
+// LLVM:         %[[B_ADDR:.*]] = alloca float, i64 1
+// LLVM:         store float %[[A]], ptr %[[A_ADDR]]
+// LLVM:         store float %[[B]], ptr %[[B_ADDR]]
+
+// LLVM:         %[[A1:.*]] = load float, ptr %[[A_ADDR]]
+// LLVM:         %[[B1:.*]] = load float, ptr %[[B_ADDR]]
+// LLVM:         fmul float %[[A1]], %[[B1]]
+
+// LLVM:         %[[A2:.*]] = load float, ptr %[[A_ADDR]]
+// LLVM:         %[[B2:.*]] = load float, ptr %[[B_ADDR]]
+// LLVM:         fdiv float %[[A2]], %[[B2]]
+
+// LLVM:         %[[A3:.*]] = load float, ptr %[[A_ADDR]]
+// LLVM:         %[[B3:.*]] = load float, ptr %[[B_ADDR]]
+// LLVM:         fadd float %[[A3]], %[[B3]]
+
+// LLVM:         %[[A4:.*]] = load float, ptr %[[A_ADDR]]
+// LLVM:         %[[B4:.*]] = load float, ptr %[[B_ADDR]]
+// LLVM:         fsub float %[[A4]], %[[B4]]
+
+// LLVM:         ret void
+
+// OGCG-LABEL: define dso_local void @_Z23testFloatingPointBinOpsff(float {{.*}} %a, float {{.*}} %b)
+// OGCG:         %a.addr = alloca float
+// OGCG:         %b.addr = alloca flo...
[truncated]

@mmha
Copy link
Contributor Author

mmha commented Mar 28, 2025

Copy link
Member

@bcardosolopes bcardosolopes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks reasonable to add vector support later for shifts. Because SelectOp is independent of ShiftOp, one possibility would have been to split it in two PRs to land them faster, but wouldn't bother changing for this round.

Copy link
Collaborator

@erichkeane erichkeane left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Andy/Bruno have a good hold on this, so I'll let them review it.

mmha added 2 commits April 14, 2025 15:48
Since SelectOp will only generated by a future pass that transforms a TernaryOp this only includes the lowering bits.

This patchs also improves the testing of the existing binary operators.
- add createShift functions to CIRBuilder
- Rephrase ShiftOpn description comment
- Remove folding for now
- Always zero-extend instead of potentially sign-extend
- Tests with diffferently sized integral types
@mmha mmha force-pushed the cir-logical-binops branch from fc54913 to 8199367 Compare April 14, 2025 14:16
bcardosolopes pushed a commit to llvm/clangir that referenced this pull request Apr 17, 2025
Negative shift amounts are undefined behavior in C and C++. Because of
that we can always zero-extend the shift amount which is slightly faster
on certain architectures (e. g. x86). This also matches the behavior of
the original clang Codegen.

Backported from llvm/llvm-project#133405

Co-authored-by: Morris Hafner <[email protected]>
@erichkeane erichkeane merged commit b53db89 into llvm:main Apr 22, 2025
10 of 11 checks passed
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
Since SelectOp will only generated by a future pass that transforms a
TernaryOp this only includes the lowering bits.

This patch also improves the testing of the existing binary operators.

---------

Co-authored-by: Morris Hafner <[email protected]>
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
Since SelectOp will only generated by a future pass that transforms a
TernaryOp this only includes the lowering bits.

This patch also improves the testing of the existing binary operators.

---------

Co-authored-by: Morris Hafner <[email protected]>
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
Since SelectOp will only generated by a future pass that transforms a
TernaryOp this only includes the lowering bits.

This patch also improves the testing of the existing binary operators.

---------

Co-authored-by: Morris Hafner <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang Clang issues not falling into any other category ClangIR Anything related to the ClangIR project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants