[CIR] Add binary operators #132420

mmha · 2025-03-21T16:55:41Z

This patch adds upstreams support for BinOp including lvalue assignments. Note that this does not include ternary ops, BinOpOverflowOp, pointer arithmetic, ShiftOp and SelectOp which are required for logical binary operators.

mmha · 2025-03-21T16:56:10Z

@erichkeane @dkolsen-pgi @andykaylor

llvmbot · 2025-03-21T16:56:15Z

@llvm/pr-subscribers-clang-codegen
@llvm/pr-subscribers-clangir

@llvm/pr-subscribers-clang

Author: Morris Hafner (mmha)

Changes

This patch adds upstreams support for BinOp and BinOverflowOp including lvalue assignments and rudimentary support for pointer arithmetic. Note that this does not include ternary ops, ShiftOp and SelectOp which are required for logical binary operators.

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

19 Files Affected:

(modified) clang/include/clang/CIR/Dialect/Builder/CIRBaseBuilder.h (+113-1)
(modified) clang/include/clang/CIR/Dialect/IR/CIROps.td (+123)
(modified) clang/include/clang/CIR/Dialect/IR/CIRTypes.h (+1)
(modified) clang/include/clang/CIR/MissingFeatures.h (+12-1)
(modified) clang/lib/CIR/CodeGen/CIRGenBuilder.h (+111-5)
(modified) clang/lib/CIR/CodeGen/CIRGenExpr.cpp (+59-5)
(modified) clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp (+721-9)
(modified) clang/lib/CIR/CodeGen/CIRGenFunction.cpp (+2)
(modified) clang/lib/CIR/CodeGen/CIRGenFunction.h (+20-10)
(modified) clang/lib/CIR/Dialect/IR/CIRDialect.cpp (+36-5)
(modified) clang/lib/CIR/Dialect/IR/CIRTypes.cpp (+10)
(modified) clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp (+238-5)
(modified) clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.h (+33)
(added) clang/test/CIR/CodeGen/binop.cpp (+33)
(added) clang/test/CIR/Lowering/binop-bool.cir (+18)
(added) clang/test/CIR/Lowering/binop-fp.cir (+68)
(added) clang/test/CIR/Lowering/binop-overflow.cir (+63)
(added) clang/test/CIR/Lowering/binop-signed-int.cir (+60)
(added) clang/test/CIR/Lowering/binop-unsigned-int.cir (+73)

diff --git a/clang/include/clang/CIR/Dialect/Builder/CIRBaseBuilder.h b/clang/include/clang/CIR/Dialect/Builder/CIRBaseBuilder.h
index c6aea10d46b63..9fe80cde261a9 100644
--- a/clang/include/clang/CIR/Dialect/Builder/CIRBaseBuilder.h
+++ b/clang/include/clang/CIR/Dialect/Builder/CIRBaseBuilder.h
@@ -10,7 +10,6 @@
 #define LLVM_CLANG_CIR_DIALECT_BUILDER_CIRBASEBUILDER_H
 
 #include "clang/AST/CharUnits.h"
-#include "clang/AST/Type.h"
 #include "clang/CIR/Dialect/IR/CIRAttrs.h"
 #include "clang/CIR/Dialect/IR/CIRDialect.h"
 #include "clang/CIR/Dialect/IR/CIRTypes.h"
@@ -28,6 +27,11 @@ class CIRBaseBuilderTy : public mlir::OpBuilder {
   CIRBaseBuilderTy(mlir::MLIRContext &mlirContext)
       : mlir::OpBuilder(&mlirContext) {}
 
+  mlir::Value getConstAPInt(mlir::Location loc, mlir::Type typ,
+                            const llvm::APInt &val) {
+    return create<cir::ConstantOp>(loc, typ, getAttr<cir::IntAttr>(typ, val));
+  }
+
   cir::ConstantOp getConstant(mlir::Location loc, mlir::TypedAttr attr) {
     return create<cir::ConstantOp>(loc, attr.getType(), attr);
   }
@@ -143,6 +147,114 @@ class CIRBaseBuilderTy : public mlir::OpBuilder {
     return createCast(loc, cir::CastKind::bitcast, src, newTy);
   }
 
+  mlir::Value createBinop(mlir::Value lhs, cir::BinOpKind kind,
+                          const llvm::APInt &rhs) {
+    return create<cir::BinOp>(lhs.getLoc(), lhs.getType(), kind, lhs,
+                              getConstAPInt(lhs.getLoc(), lhs.getType(), rhs));
+  }
+
+  mlir::Value createBinop(mlir::Value lhs, cir::BinOpKind kind,
+                          mlir::Value rhs) {
+    return create<cir::BinOp>(lhs.getLoc(), lhs.getType(), kind, lhs, rhs);
+  }
+
+  mlir::Value createBinop(mlir::Location loc, mlir::Value lhs,
+                          cir::BinOpKind kind, mlir::Value rhs) {
+    return create<cir::BinOp>(loc, lhs.getType(), kind, lhs, rhs);
+  }
+
+  mlir::Value createLowBitsSet(mlir::Location loc, unsigned size,
+                               unsigned bits) {
+    llvm::APInt val = llvm::APInt::getLowBitsSet(size, bits);
+    auto type = cir::IntType::get(getContext(), size, false);
+    return getConstAPInt(loc, type, val);
+  }
+
+  mlir::Value createAnd(mlir::Value lhs, const llvm::APInt &rhs) {
+    mlir::Value val = getConstAPInt(lhs.getLoc(), lhs.getType(), rhs);
+    return createBinop(lhs, cir::BinOpKind::And, val);
+  }
+
+  mlir::Value createAnd(mlir::Value lhs, mlir::Value rhs) {
+    return createBinop(lhs, cir::BinOpKind::And, rhs);
+  }
+
+  mlir::Value createAnd(mlir::Location loc, mlir::Value lhs, mlir::Value rhs) {
+    return createBinop(loc, lhs, cir::BinOpKind::And, rhs);
+  }
+
+  mlir::Value createOr(mlir::Value lhs, const llvm::APInt &rhs) {
+    mlir::Value val = getConstAPInt(lhs.getLoc(), lhs.getType(), rhs);
+    return createBinop(lhs, cir::BinOpKind::Or, val);
+  }
+
+  mlir::Value createOr(mlir::Value lhs, mlir::Value rhs) {
+    return createBinop(lhs, cir::BinOpKind::Or, rhs);
+  }
+
+  mlir::Value createMul(mlir::Value lhs, mlir::Value rhs, bool hasNUW = false,
+                        bool hasNSW = false) {
+    auto op = create<cir::BinOp>(lhs.getLoc(), lhs.getType(),
+                                 cir::BinOpKind::Mul, lhs, rhs);
+    if (hasNUW)
+      op.setNoUnsignedWrap(true);
+    if (hasNSW)
+      op.setNoSignedWrap(true);
+    return op;
+  }
+  mlir::Value createNSWMul(mlir::Value lhs, mlir::Value rhs) {
+    return createMul(lhs, rhs, false, true);
+  }
+  mlir::Value createNUWAMul(mlir::Value lhs, mlir::Value rhs) {
+    return createMul(lhs, rhs, true, false);
+  }
+
+  mlir::Value createMul(mlir::Value lhs, const llvm::APInt &rhs) {
+    mlir::Value val = getConstAPInt(lhs.getLoc(), lhs.getType(), rhs);
+    return createBinop(lhs, cir::BinOpKind::Mul, val);
+  }
+
+  mlir::Value createSub(mlir::Value lhs, mlir::Value rhs, bool hasNUW = false,
+                        bool hasNSW = false, bool saturated = false) {
+    auto op = create<cir::BinOp>(lhs.getLoc(), lhs.getType(),
+                                 cir::BinOpKind::Sub, lhs, rhs);
+    if (hasNUW)
+      op.setNoUnsignedWrap(true);
+    if (hasNSW)
+      op.setNoSignedWrap(true);
+    if (saturated)
+      op.setSaturated(true);
+    return op;
+  }
+
+  mlir::Value createNSWSub(mlir::Value lhs, mlir::Value rhs) {
+    return createSub(lhs, rhs, false, true);
+  }
+
+  mlir::Value createNUWSub(mlir::Value lhs, mlir::Value rhs) {
+    return createSub(lhs, rhs, true, false);
+  }
+
+  mlir::Value createAdd(mlir::Value lhs, mlir::Value rhs, bool hasNUW = false,
+                        bool hasNSW = false, bool saturated = false) {
+    auto op = create<cir::BinOp>(lhs.getLoc(), lhs.getType(),
+                                 cir::BinOpKind::Add, lhs, rhs);
+    if (hasNUW)
+      op.setNoUnsignedWrap(true);
+    if (hasNSW)
+      op.setNoSignedWrap(true);
+    if (saturated)
+      op.setSaturated(true);
+    return op;
+  }
+
+  mlir::Value createNSWAdd(mlir::Value lhs, mlir::Value rhs) {
+    return createAdd(lhs, rhs, false, true);
+  }
+  mlir::Value createNUWAdd(mlir::Value lhs, mlir::Value rhs) {
+    return createAdd(lhs, rhs, true, false);
+  }
+
   //
   // Block handling helpers
   // ----------------------
diff --git a/clang/include/clang/CIR/Dialect/IR/CIROps.td b/clang/include/clang/CIR/Dialect/IR/CIROps.td
index d7d63e040a2ba..dca17e6cd2d2d 100644
--- a/clang/include/clang/CIR/Dialect/IR/CIROps.td
+++ b/clang/include/clang/CIR/Dialect/IR/CIROps.td
@@ -826,6 +826,129 @@ def ForOp : CIR_Op<"for", [LoopOpInterface, NoRegionArguments]> {
   }];
 }
 
+//===----------------------------------------------------------------------===//
+// BinOp
+//===----------------------------------------------------------------------===//
+
+// FIXME: represent Commutative, Idempotent traits for appropriate binops
+def BinOpKind_Mul : I32EnumAttrCase<"Mul", 1, "mul">;
+def BinOpKind_Div : I32EnumAttrCase<"Div", 2, "div">;
+def BinOpKind_Rem : I32EnumAttrCase<"Rem", 3, "rem">;
+def BinOpKind_Add : I32EnumAttrCase<"Add", 4, "add">;
+def BinOpKind_Sub : I32EnumAttrCase<"Sub", 5, "sub">;
+def BinOpKind_And : I32EnumAttrCase<"And", 8, "and">;
+def BinOpKind_Xor : I32EnumAttrCase<"Xor", 9, "xor">;
+def BinOpKind_Or  : I32EnumAttrCase<"Or", 10, "or">;
+// TODO(cir): Do we need a min binop?
+def BinOpKind_Max : I32EnumAttrCase<"Max", 11, "max">;
+
+def BinOpKind : I32EnumAttr<
+    "BinOpKind",
+    "binary operation (arith and logic) kind",
+    [BinOpKind_Mul, BinOpKind_Div, BinOpKind_Rem,
+     BinOpKind_Add, BinOpKind_Sub,
+     BinOpKind_And, BinOpKind_Xor,
+     BinOpKind_Or, BinOpKind_Max]> {
+  let cppNamespace = "::cir";
+}
+
+// FIXME: Pure won't work when we add overloading.
+def BinOp : CIR_Op<"binop", [Pure,
+  SameTypeOperands, SameOperandsAndResultType]> {
+
+  let summary = "Binary operations (arith and logic)";
+  let description = [{
+    cir.binop performs the binary operation according to
+    the specified opcode kind: [mul, div, rem, add, sub,
+    and, xor, or, max].
+
+    It requires two input operands and has one result, all types
+    should be the same.
+
+    ```mlir
+    %7 = cir.binop(add, %1, %2) : !s32i
+    %7 = cir.binop(mul, %1, %2) : !u8i
+    ```
+  }];
+
+  // TODO: get more accurate than CIR_AnyType
+  let results = (outs CIR_AnyType:$result);
+  let arguments = (ins Arg<BinOpKind, "binop kind">:$kind,
+                       CIR_AnyType:$lhs, CIR_AnyType:$rhs,
+                       UnitAttr:$no_unsigned_wrap,
+                       UnitAttr:$no_signed_wrap,
+                       UnitAttr:$saturated);
+
+  let assemblyFormat = [{
+    `(` $kind `,` $lhs `,` $rhs  `)`
+    (`nsw` $no_signed_wrap^)?
+    (`nuw` $no_unsigned_wrap^)?
+    (`sat` $saturated^)?
+    `:` type($lhs) attr-dict
+  }];
+
+  let hasVerifier = 1;
+}
+
+
+//===----------------------------------------------------------------------===//
+// BinOpOverflowOp
+//===----------------------------------------------------------------------===//
+
+def BinOpOverflowKind : I32EnumAttr<
+    "BinOpOverflowKind",
+    "checked binary arithmetic operation kind",
+    [BinOpKind_Add, BinOpKind_Sub, BinOpKind_Mul]> {
+  let cppNamespace = "::cir";
+}
+
+def BinOpOverflowOp : CIR_Op<"binop.overflow", [Pure, SameTypeOperands]> {
+  let summary = "Perform binary integral arithmetic with overflow checking";
+  let description = [{
+    `cir.binop.overflow` performs binary arithmetic operations with overflow
+    checking on integral operands.
+
+    The `kind` argument specifies the kind of arithmetic operation to perform.
+    It can be either `add`, `sub`, or `mul`. The `lhs` and `rhs` arguments
+    specify the input operands of the arithmetic operation. The types of `lhs`
+    and `rhs` must be the same.
+
+    `cir.binop.overflow` produces two SSA values. `result` is the result of the
+    arithmetic operation truncated to its specified type. `overflow` is a
+    boolean value indicating whether overflow happens during the operation.
+
+    The exact semantic of this operation is as follows:
+
+      - `lhs` and `rhs` are promoted to an imaginary integral type that has
+        infinite precision.
+      - The arithmetic operation is performed on the promoted operands.
+      - The infinite-precision result is truncated to the type of `result`. The
+        truncated result is assigned to `result`.
+      - If the truncated result is equal to the un-truncated result, `overflow`
+        is assigned to false. Otherwise, `overflow` is assigned to true.
+  }];
+
+  let arguments = (ins Arg<BinOpOverflowKind, "arithmetic kind">:$kind,
+                       CIR_IntType:$lhs, CIR_IntType:$rhs);
+  let results = (outs CIR_IntType:$result, CIR_BoolType:$overflow);
+
+  let assemblyFormat = [{
+    `(` $kind `,` $lhs `,` $rhs `)` `:` type($lhs) `,`
+    `(` type($result) `,` type($overflow) `)`
+    attr-dict
+  }];
+
+  let builders = [
+    OpBuilder<(ins "cir::IntType":$resultTy,
+                   "cir::BinOpOverflowKind":$kind,
+                   "mlir::Value":$lhs,
+                   "mlir::Value":$rhs), [{
+      auto overflowTy = cir::BoolType::get($_builder.getContext());
+      build($_builder, $_state, resultTy, overflowTy, kind, lhs, rhs);
+    }]>
+  ];
+}
+
 //===----------------------------------------------------------------------===//
 // GlobalOp
 //===----------------------------------------------------------------------===//
diff --git a/clang/include/clang/CIR/Dialect/IR/CIRTypes.h b/clang/include/clang/CIR/Dialect/IR/CIRTypes.h
index 5d1eb17e146d0..7b0fcbc7cc98f 100644
--- a/clang/include/clang/CIR/Dialect/IR/CIRTypes.h
+++ b/clang/include/clang/CIR/Dialect/IR/CIRTypes.h
@@ -21,6 +21,7 @@
 namespace cir {
 
 bool isAnyFloatingPointType(mlir::Type t);
+bool isFPOrFPVectorTy(mlir::Type);
 
 } // namespace cir
 
diff --git a/clang/include/clang/CIR/MissingFeatures.h b/clang/include/clang/CIR/MissingFeatures.h
index 3e33e5dc60194..3654038a51fbd 100644
--- a/clang/include/clang/CIR/MissingFeatures.h
+++ b/clang/include/clang/CIR/MissingFeatures.h
@@ -79,6 +79,9 @@ struct MissingFeatures {
   static bool opUnarySignedOverflow() { return false; }
   static bool opUnaryPromotionType() { return false; }
 
+  // Clang early optimizations or things defered to LLVM lowering.
+  static bool mayHaveIntegerOverflow() { return false; }
+
   // Misc
   static bool cxxABI() { return false; }
   static bool tryEmitAsConstant() { return false; }
@@ -93,16 +96,20 @@ struct MissingFeatures {
   static bool stackSaveOp() { return false; }
   static bool aggValueSlot() { return false; }
   static bool generateDebugInfo() { return false; }
+  static bool getFPFeaturesInEffect() { return false; }
+  static bool pointerOverflowSanitizer() { return false; }
   static bool fpConstraints() { return false; }
   static bool sanitizers() { return false; }
   static bool addHeapAllocSiteMetadata() { return false; }
   static bool targetCodeGenInfoGetNullPointer() { return false; }
-  static bool CGFPOptionsRAII() { return false; }
   static bool loopInfoStack() { return false; }
   static bool requiresCleanups() { return false; }
   static bool createProfileWeightsForLoop() { return false; }
   static bool emitCondLikelihoodViaExpectIntrinsic() { return false; }
   static bool pgoUse() { return false; }
+  static bool cgFPOptionsRAII() { return false; }
+  static bool metaDataNode() { return false; }
+  static bool foldBinOpFMF() { return false; }
 
   // Missing types
   static bool dataMemberType() { return false; }
@@ -111,6 +118,8 @@ struct MissingFeatures {
   static bool scalableVectors() { return false; }
   static bool unsizedTypes() { return false; }
   static bool vectorType() { return false; }
+  static bool complexType() { return false; }
+  static bool fixedPointType() { return false; }
 
   // Future CIR operations
   static bool awaitOp() { return false; }
@@ -127,6 +136,8 @@ struct MissingFeatures {
   static bool ternaryOp() { return false; }
   static bool tryOp() { return false; }
   static bool zextOp() { return false; }
+  static bool opPtrStride() { return false; }
+  static bool opPtrDiff() { return false; }
 };
 
 } // namespace cir
diff --git a/clang/lib/CIR/CodeGen/CIRGenBuilder.h b/clang/lib/CIR/CodeGen/CIRGenBuilder.h
index fef290612149a..dfffc2639b0d2 100644
--- a/clang/lib/CIR/CodeGen/CIRGenBuilder.h
+++ b/clang/lib/CIR/CodeGen/CIRGenBuilder.h
@@ -14,11 +14,13 @@
 
 #include "clang/CIR/Dialect/Builder/CIRBaseBuilder.h"
 #include "clang/CIR/MissingFeatures.h"
+#include "llvm/ADT/STLExtras.h"
 
 namespace clang::CIRGen {
 
 class CIRGenBuilderTy : public cir::CIRBaseBuilderTy {
   const CIRGenTypeCache &typeCache;
+  bool isFPConstrained = false;
 
 public:
   CIRGenBuilderTy(mlir::MLIRContext &mlirContext, const CIRGenTypeCache &tc)
@@ -72,15 +74,72 @@ class CIRGenBuilderTy : public cir::CIRBaseBuilderTy {
     if (const auto arrayVal = mlir::dyn_cast<cir::ConstArrayAttr>(attr)) {
       if (mlir::isa<mlir::StringAttr>(arrayVal.getElts()))
         return false;
-      for (const auto elt : mlir::cast<mlir::ArrayAttr>(arrayVal.getElts())) {
-        if (!isNullValue(elt))
-          return false;
-      }
-      return true;
+
+      return llvm::all_of(
+          mlir::cast<mlir::ArrayAttr>(arrayVal.getElts()),
+          [&](const mlir::Attribute &elt) { return isNullValue(elt); });
     }
     return false;
   }
 
+  //
+  // Type helpers
+  // ------------
+  //
+  cir::IntType getUIntNTy(int n) {
+    switch (n) {
+    case 8:
+      return getUInt8Ty();
+    case 16:
+      return getUInt16Ty();
+    case 32:
+      return getUInt32Ty();
+    case 64:
+      return getUInt64Ty();
+    default:
+      return cir::IntType::get(getContext(), n, false);
+    }
+  }
+
+  cir::IntType getSIntNTy(int n) {
+    switch (n) {
+    case 8:
+      return getSInt8Ty();
+    case 16:
+      return getSInt16Ty();
+    case 32:
+      return getSInt32Ty();
+    case 64:
+      return getSInt64Ty();
+    default:
+      return cir::IntType::get(getContext(), n, true);
+    }
+  }
+
+  cir::VoidType getVoidTy() { return typeCache.VoidTy; }
+
+  cir::IntType getSInt8Ty() { return typeCache.SInt8Ty; }
+  cir::IntType getSInt16Ty() { return typeCache.SInt16Ty; }
+  cir::IntType getSInt32Ty() { return typeCache.SInt32Ty; }
+  cir::IntType getSInt64Ty() { return typeCache.SInt64Ty; }
+
+  cir::IntType getUInt8Ty() { return typeCache.UInt8Ty; }
+  cir::IntType getUInt16Ty() { return typeCache.UInt16Ty; }
+  cir::IntType getUInt32Ty() { return typeCache.UInt32Ty; }
+  cir::IntType getUInt64Ty() { return typeCache.UInt64Ty; }
+
+  bool isInt8Ty(mlir::Type i) {
+    return i == typeCache.UInt8Ty || i == typeCache.SInt8Ty;
+  }
+  bool isInt16Ty(mlir::Type i) {
+    return i == typeCache.UInt16Ty || i == typeCache.SInt16Ty;
+  }
+  bool isInt32Ty(mlir::Type i) {
+    return i == typeCache.UInt32Ty || i == typeCache.SInt32Ty;
+  }
+  bool isInt64Ty(mlir::Type i) {
+    return i == typeCache.UInt64Ty || i == typeCache.SInt64Ty;
+  }
   bool isInt(mlir::Type i) { return mlir::isa<cir::IntType>(i); }
 
   // Creates constant nullptr for pointer type ty.
@@ -88,6 +147,53 @@ class CIRGenBuilderTy : public cir::CIRBaseBuilderTy {
     assert(!cir::MissingFeatures::targetCodeGenInfoGetNullPointer());
     return create<cir::ConstantOp>(loc, ty, getConstPtrAttr(ty, 0));
   }
+
+  mlir::Value createNeg(mlir::Value value) {
+
+    if (auto intTy = mlir::dyn_cast<cir::IntType>(value.getType())) {
+      // Source is a unsigned integer: first cast it to signed.
+      if (intTy.isUnsigned())
+        value = createIntCast(value, getSIntNTy(intTy.getWidth()));
+      return create<cir::UnaryOp>(value.getLoc(), value.getType(),
+                                  cir::UnaryOpKind::Minus, value);
+    }
+
+    llvm_unreachable("negation for the given type is NYI");
+  }
+
+  mlir::Value createFSub(mlir::Value lhs, mlir::Value rhs) {
+    assert(!cir::MissingFeatures::metaDataNode());
+    if (isFPConstrained)
+      llvm_unreachable("Constrained FP NYI");
+
+    assert(!cir::MissingFeatures::foldBinOpFMF());
+    return create<cir::BinOp>(lhs.getLoc(), cir::BinOpKind::Sub, lhs, rhs);
+  }
+
+  mlir::Value createFAdd(mlir::Value lhs, mlir::Value rhs) {
+    assert(!cir::MissingFeatures::metaDataNode());
+    if (isFPConstrained)
+      llvm_unreachable("Constrained FP NYI");
+
+    assert(!cir::MissingFeatures::foldBinOpFMF());
+    return create<cir::BinOp>(lhs.getLoc(), cir::BinOpKind::Add, lhs, rhs);
+  }
+  mlir::Value createFMul(mlir::Value lhs, mlir::Value rhs) {
+    assert(!cir::MissingFeatures::metaDataNode());
+    if (isFPConstrained)
+      llvm_unreachable("Constrained FP NYI");
+
+    assert(!cir::MissingFeatures::foldBinOpFMF());
+    return create<cir::BinOp>(lhs.getLoc(), cir::BinOpKind::Mul, lhs, rhs);
+  }
+  mlir::Value createFDiv(mlir::Value lhs, mlir::Value rhs) {
+    assert(!cir::MissingFeatures::metaDataNode());
+    if (isFPConstrained)
+      llvm_unreachable("Constrained FP NYI");
+
+    assert(!cir::MissingFeatures::foldBinOpFMF());
+    return create<cir::BinOp>(lhs.getLoc(), cir::BinOpKind::Div, lhs, rhs);
+  }
 };
 
 } // namespace clang::CIRGen
diff --git a/clang/lib/CIR/CodeGen/CIRGenExpr.cpp b/clang/lib/CIR/CodeGen/CIRGenExpr.cpp
index 306130b80d457..d3365cbcbbeed 100644
--- a/clang/lib/CIR/CodeGen/CIRGenExpr.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenExpr.cpp
@@ -149,8 +149,8 @@ LValue CIRGenFunction::emitDeclRefLValue(const DeclRefExpr *e) {
     Address addr = Address::invalid();
 
     // The variable should generally be present in the local decl map.
-    auto iter = LocalDeclMap.find(vd);
-    if (iter != LocalDeclMap.end()) {
+    auto iter = localDeclMap.find(vd);
+    if (iter != localDeclMap.end()) {
       addr = iter->second;
     } else {
       // Otherwise, it might be static local we haven't emitted yet for some
@@ -176,7 +176,7 @@ mlir::Value CIRGenFunction::evaluateExprAsBool(const Expr *e) {
     return createDummyValue(getLoc(loc), boolTy);
   }
 
-  assert(!cir::MissingFeatures::CGFPOptionsRAII());
+  assert(!cir::MissingFeatures::cgFPOptionsRAII());
   if (!e->getType()->isAnyComplexType())
     return emitScalarConversion(emitScalarExpr(e), e->getType(), boolTy, loc);
 
@@ -211,9 +211,8 @@ LValue CIRGenFunction::emitUnaryOpLValue(const UnaryOperator *e) {
     if (e->getType()->isAnyComplexType()) {
       cgm.errorNYI(e->getSourceRange(), "UnaryOp complex inc/dec");
       return LValue();
-    } else {
-      emitScalarPrePostIncDec(e, lv, isInc, /*isPre=*/true);
     }
+    emitScalarPrePostIncDec(e, lv, isInc, /*isPre=*/true);
 
     return lv;
   }
@@ -232,6 +231,61 @@ LValue CIRGenFunction::emitUnaryOpLValue(const UnaryOperator *e) {
   llvm_unreachable("Unknown unary operator kind!");
 }
 
+LValue CIRGenFunction::emitBinaryOperatorLValue(const BinaryOperator *e) {
+  // Comma expressions just emit their LHS then their RHS as an l-value.
+  if (e->getOpcode() == BO_Comma) {
+    emitIgnoredExpr(e->getLHS());
+    return emitLValue(e->getRHS());
+  }
+
+  if (e->getOpcode() == BO_PtrMemD || e->getOpcode() == BO_PtrMemI) {
+    cgm.errorNYI(e->getSourceRange(), "member pointers");
+    return {};
+  }
+
+  assert(e->getOpcode() == BO_Assign && "unexpected binary l-value");
+
+  // Note that in all of these cases, __block variables need the RHS
+  // evaluated first just in case the variable gets moved by the RHS.
+
+  switch (CIR...
[truncated]

erichkeane · 2025-03-21T17:03:37Z

clang/include/clang/CIR/Dialect/Builder/CIRBaseBuilder.h

@@ -143,6 +147,114 @@ class CIRBaseBuilderTy : public mlir::OpBuilder {
    return createCast(loc, cir::CastKind::bitcast, src, newTy);
  }

+  mlir::Value createBinop(mlir::Value lhs, cir::BinOpKind kind,
+                          const llvm::APInt &rhs) {
+    return create<cir::BinOp>(lhs.getLoc(), lhs.getType(), kind, lhs,


Its a shame for readability BinOp's create isnt LHS/Kind/RHS.

erichkeane · 2025-03-21T17:09:51Z

clang/include/clang/CIR/Dialect/Builder/CIRBaseBuilder.h

+    return createBinop(lhs, cir::BinOpKind::Mul, val);
+  }
+
+  mlir::Value createSub(mlir::Value lhs, mlir::Value rhs, bool hasNUW = false,


Would it make sense to have NUW/NSW (maybe saturated?) be a bitfield-esque enum? Bool parameters general are frowned upon as they lack significant context.

Alternatively, we could not have these parameters at all here and set the attributes in createNSWSub, createNUWSub, etc.

I think I prefer Erich's suggestion here as seting these attributes in the create* functions would lead to more code duplication (if I understand you correctly).

For creating that bitfield enum I'd like to reuse CV_DEFINE_ENUM_CLASS_FLAGS_OPERATORS from llvm/include/llvm/DebugInfo/CodeView/CodeView.h. What would be a good place to move this macro to? clang::CodeGen::FnInfoOpts could use that as well.

+1 on Erich's point. I'd suggest we just have our CIR specific enum CIRBaseBuilder.h (should be small and simple) and not depend on LLVM for it, we can incrementally add more things when they make sense, but I bet 3 states will hold for long enough. Alternatively, you could add enum kinds in CIRAttr and just refer to those directly, but doing that will require you to redesign BinOp a bit to also use them.

Does LLVM_MARK_AS_BITMASK_ENUM do what you want for the bitfield enum?

It would if we were in the llvm:: namespace :)

I'll handroll the operator overloads for the time being.

clang/include/clang/CIR/Dialect/Builder/CIRBaseBuilder.h

erichkeane · 2025-03-21T17:11:42Z

clang/include/clang/CIR/Dialect/IR/CIROps.td

+def BinOpKind_Xor : I32EnumAttrCase<"Xor", 9, "xor">;
+def BinOpKind_Or  : I32EnumAttrCase<"Or", 10, "or">;
+// TODO(cir): Do we need a min binop?
+def BinOpKind_Max : I32EnumAttrCase<"Max", 11, "max">;


What does max represent in the language? Or is this something we transform to?

This is to support __builtin_elementwise_max and at least one corresponding SIMD intrinsic function (see llvm/clangir#1201).

I can't find that TODO in the git history of the incubator for some reason. But since there is a __builtin_elementwise_min I assume there needs to be a min as well.

Both builtins are still NYI though so I could remove them for the time being.

clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp

andykaylor

This is an awful lot to digest in one review. Can you split out the BinOpOverflowOp and pointer arithmetic?

clang/include/clang/CIR/Dialect/Builder/CIRBaseBuilder.h

andykaylor · 2025-03-21T20:34:52Z

clang/include/clang/CIR/Dialect/Builder/CIRBaseBuilder.h

+
+  mlir::Value createBinop(mlir::Value lhs, cir::BinOpKind kind,
+                          mlir::Value rhs) {
+    return create<cir::BinOp>(lhs.getLoc(), lhs.getType(), kind, lhs, rhs);


This seems likely to be the wrong location for the binop. Are we going to regret this when we start generating debug info?

Very nice catch. The source locations of all binops except for division were not passed through. I changed the signature of the builder functions to require an mlir::Location. If there's a future need for an overload without a location we can add those later.

clang/include/clang/CIR/Dialect/Builder/CIRBaseBuilder.h

andykaylor · 2025-03-21T20:39:18Z

clang/include/clang/CIR/Dialect/Builder/CIRBaseBuilder.h

+    return createBinop(lhs, cir::BinOpKind::Mul, val);
+  }
+
+  mlir::Value createSub(mlir::Value lhs, mlir::Value rhs, bool hasNUW = false,


Alternatively, we could not have these parameters at all here and set the attributes in createNSWSub, createNUWSub, etc.

clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp

github-actions · 2025-03-24T12:38:03Z

✅ With the latest revision this PR passed the C/C++ code formatter.

mmha · 2025-03-24T17:39:31Z

As @andykaylor suggested, I'm going to split off BinOpOverflowOp and the pointer arithmetic bits into a separate patch next.

andykaylor · 2025-03-24T20:07:57Z

clang/include/clang/AST/ASTContext.h

@@ -200,7 +200,7 @@ class ASTContext : public RefCountedBase<ASTContext> {
  mutable llvm::ContextualFoldingSet<ConstantArrayType, ASTContext &>
      ConstantArrayTypes;
  mutable llvm::FoldingSet<IncompleteArrayType> IncompleteArrayTypes;
-  mutable std::vector<VariableArrayType*> VariableArrayTypes;
+  mutable std::vector<VariableArrayType *> VariableArrayTypes;


Did you run clang-format on this entire file? LLVM's coding standards don't allow that. You'll need to undo the formatting changes that aren't related to your patch.

That was git clang-format in action, yes.

(Why is ASTContext.h/cpp not clang-format clean?)

This patch adds upstreams support for BinOp and BinOverflowOp including lvalue assignments and rudimentary support for pointer arithmetic. Note that this does not include ternary ops, ShiftOp and SelectOp which are required for logical binary operators.

Co-authored-by: Andy Kaylor <[email protected]>

Also revert ASTContext reformat

andykaylor

This looks good to me except for a few nits and the need to defer the ASTContext refactoring change.

andykaylor · 2025-03-25T18:25:24Z

clang/include/clang/CIR/MissingFeatures.h

@@ -127,6 +135,8 @@ struct MissingFeatures {
  static bool ternaryOp() { return false; }
  static bool tryOp() { return false; }
  static bool zextOp() { return false; }
+  static bool ptrStrideOp() { return false; }


@AmrDeveloper Be sure to look for the places where this is used when you add support for cir.ptr_stride.

andykaylor · 2025-03-25T18:34:38Z

clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp

+
+  mlir::Value emitPromotedValue(mlir::Value result, QualType promotionType) {
+    cgf.cgm.errorNYI(result.getLoc(), "floating cast for promoted value");
+    return nullptr;


Suggested change

return nullptr;

return {};

andykaylor · 2025-03-25T18:34:49Z

clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp

+
+  mlir::Value emitUnPromotedValue(mlir::Value result, QualType exprType) {
+    cgf.cgm.errorNYI(result.getLoc(), "floating cast for unpromoted value");
+    return nullptr;


Suggested change

return nullptr;

return {};

andykaylor · 2025-03-25T18:42:22Z

clang/lib/CodeGen/CGExprScalar.cpp

 /// Check if \p E is a widened promoted integer.
 static bool IsWidenedIntegerOp(const ASTContext &Ctx, const Expr *E) {
-  return getUnwidenedIntegerType(Ctx, E).has_value();
+  return Ctx.getUnwidenedIntegerType(E).has_value();


I love this change, but this should probably be split out as a separate patch. As we discussed earlier, let's defer this refactoring for a separate patch.

andykaylor · 2025-03-25T19:00:41Z

clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp

+      }
+      rewriter.replaceOpWithNewOp<mlir::LLVM::AddOp>(op, llvmTy, lhs, rhs,
+                                                     getIntOverflowFlag(op));
+    } else


The LLVM Coding Standard requires braces on the else when they are needed for the if. Likewise below.

andykaylor · 2025-03-25T19:03:56Z

clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp

@@ -1133,6 +1252,7 @@ void ConvertCIRToLLVMPass::runOnOperation() {
  patterns.add<
      // clang-format off
               CIRToLLVMBrCondOpLowering,
+               CIRToLLVMBinOpLowering,


This list is alphabetical (for consistency in rebasing the incubator), so this belongs one line up.

- return nullptr -> {} - Add braces after else

andykaylor

lgtm

mmha requested review from lanza and bcardosolopes as code owners March 21, 2025 16:55

llvmbot added clang Clang issues not falling into any other category ClangIR Anything related to the ClangIR project labels Mar 21, 2025

erichkeane reviewed Mar 21, 2025

View reviewed changes

andykaylor reviewed Mar 21, 2025

View reviewed changes

andykaylor mentioned this pull request Mar 21, 2025

Upstreaming ClangIR #126834

Open

llvmbot added clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:codegen IR generation bugs: mangling, exceptions, etc. labels Mar 24, 2025

andykaylor reviewed Mar 24, 2025

View reviewed changes

mmha and others added 10 commits March 25, 2025 15:15

[CIR] Add binary operators

706c944

This patch adds upstreams support for BinOp and BinOverflowOp including lvalue assignments and rudimentary support for pointer arithmetic. Note that this does not include ternary ops, ShiftOp and SelectOp which are required for logical binary operators.

Update clang/include/clang/CIR/Dialect/Builder/CIRBaseBuilder.h

e2e0fa4

Co-authored-by: Andy Kaylor <[email protected]>

Update clang/include/clang/CIR/MissingFeatures.h

a77053b

Co-authored-by: Andy Kaylor <[email protected]>

Update clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp

ad4b4c2

Co-authored-by: Andy Kaylor <[email protected]>

Update clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp

a3c8c79

Co-authored-by: Andy Kaylor <[email protected]>

Update clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp

c66dac4

Co-authored-by: Andy Kaylor <[email protected]>

Address review comments

be5af08

More review feedback addressed

77aa79b

Also revert ASTContext reformat

Remove BinOpOverflowOp and pointer arithmetic

56fbcba

foldBinOpFMF -> fastMathFlags

455edf2

andykaylor reviewed Mar 25, 2025

View reviewed changes

- Revert ASTContecxt refactor

4428f8b

- return nullptr -> {} - Add braces after else

mmha force-pushed the cir-binops branch from 6e18bb4 to 4428f8b Compare March 25, 2025 19:20

andykaylor approved these changes Mar 25, 2025

View reviewed changes

andykaylor merged commit 2f3c937 into llvm:main Mar 25, 2025
11 checks passed

mmha mentioned this pull request Mar 26, 2025

[CIR] Add BinOpOverflowOp and basic pointer arithmetic support #133118

Open

[CIR] Add binary operators #132420

[CIR] Add binary operators #132420

Uh oh!

Conversation

mmha commented Mar 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mmha commented Mar 21, 2025

Uh oh!

llvmbot commented Mar 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bcardosolopes Mar 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

andykaylor left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Mar 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mmha commented Mar 24, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andykaylor left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andykaylor left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mmha commented Mar 21, 2025 •

edited

Loading

llvmbot commented Mar 21, 2025 •

edited

Loading

bcardosolopes Mar 24, 2025 •

edited

Loading

github-actions bot commented Mar 24, 2025 •

edited

Loading