[CIR] Upstream CmpOp #133159

mmha · 2025-03-26T20:53:18Z

This patch adds support for comparison operators with ClangIR, both integral and floating point.

llvmbot · 2025-03-26T20:53:51Z

@llvm/pr-subscribers-clangir

@llvm/pr-subscribers-clang

Author: Morris Hafner (mmha)

Changes

This patch adds support for comparison operators with ClangIR, both integral and floating point.

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

7 Files Affected:

(modified) clang/include/clang/CIR/Dialect/Builder/CIRBaseBuilder.h (+5)
(modified) clang/include/clang/CIR/Dialect/IR/CIROps.td (+44)
(modified) clang/include/clang/CIR/MissingFeatures.h (-1)
(modified) clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp (+83)
(modified) clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp (+92-2)
(modified) clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.h (+14)
(added) clang/test/CIR/CodeGen/cmp.cpp (+233)

diff --git a/clang/include/clang/CIR/Dialect/Builder/CIRBaseBuilder.h b/clang/include/clang/CIR/Dialect/Builder/CIRBaseBuilder.h
index ac7658276ec37..b94e9f8490be5 100644
--- a/clang/include/clang/CIR/Dialect/Builder/CIRBaseBuilder.h
+++ b/clang/include/clang/CIR/Dialect/Builder/CIRBaseBuilder.h
@@ -265,6 +265,11 @@ class CIRBaseBuilderTy : public mlir::OpBuilder {
     return createAdd(loc, lhs, rhs, OverflowBehavior::NoUnsignedWrap);
   }
 
+  cir::CmpOp createCompare(mlir::Location loc, cir::CmpOpKind kind,
+                           mlir::Value lhs, mlir::Value rhs) {
+    return create<cir::CmpOp>(loc, getBoolTy(), kind, lhs, rhs);
+  }
+
   //
   // Block handling helpers
   // ----------------------
diff --git a/clang/include/clang/CIR/Dialect/IR/CIROps.td b/clang/include/clang/CIR/Dialect/IR/CIROps.td
index 455cc2b8b0277..3fba7566e9f1b 100644
--- a/clang/include/clang/CIR/Dialect/IR/CIROps.td
+++ b/clang/include/clang/CIR/Dialect/IR/CIROps.td
@@ -826,6 +826,50 @@ def ForOp : CIR_Op<"for", [LoopOpInterface, NoRegionArguments]> {
   }];
 }
 
+//===----------------------------------------------------------------------===//
+// CmpOp
+//===----------------------------------------------------------------------===//
+
+def CmpOpKind_LT : I32EnumAttrCase<"lt", 1>;
+def CmpOpKind_LE : I32EnumAttrCase<"le", 2>;
+def CmpOpKind_GT : I32EnumAttrCase<"gt", 3>;
+def CmpOpKind_GE : I32EnumAttrCase<"ge", 4>;
+def CmpOpKind_EQ : I32EnumAttrCase<"eq", 5>;
+def CmpOpKind_NE : I32EnumAttrCase<"ne", 6>;
+
+def CmpOpKind : I32EnumAttr<
+    "CmpOpKind",
+    "compare operation kind",
+    [CmpOpKind_LT, CmpOpKind_LE, CmpOpKind_GT,
+     CmpOpKind_GE, CmpOpKind_EQ, CmpOpKind_NE]> {
+  let cppNamespace = "::cir";
+}
+
+def CmpOp : CIR_Op<"cmp", [Pure, SameTypeOperands]> {
+
+  let summary = "Compare values two values and produce a boolean result";
+  let description = [{
+    `cir.cmp` compares two input operands of the same type and produces a
+    `cir.bool` result. The kinds of comparison available are:
+    [lt,gt,ge,eq,ne]
+
+    ```mlir
+    %7 = cir.cmp(gt, %1, %2) : i32, !cir.bool
+    ```
+  }];
+
+  let results = (outs CIR_BoolType:$result);
+  let arguments = (ins Arg<CmpOpKind, "cmp kind">:$kind,
+                       CIR_AnyType:$lhs, CIR_AnyType:$rhs);
+
+  let assemblyFormat = [{
+    `(` $kind `,` $lhs `,` $rhs  `)` `:` type($lhs) `,` type($result) attr-dict
+  }];
+
+  // Already covered by the traits
+  let hasVerifier = 0;
+}
+
 //===----------------------------------------------------------------------===//
 // BinOp
 //===----------------------------------------------------------------------===//
diff --git a/clang/include/clang/CIR/MissingFeatures.h b/clang/include/clang/CIR/MissingFeatures.h
index 3a102d90aba8f..6e93f50ac83e6 100644
--- a/clang/include/clang/CIR/MissingFeatures.h
+++ b/clang/include/clang/CIR/MissingFeatures.h
@@ -89,7 +89,6 @@ struct MissingFeatures {
   static bool opGlobalViewAttr() { return false; }
   static bool lowerModeOptLevel() { return false; }
   static bool opTBAA() { return false; }
-  static bool opCmp() { return false; }
   static bool objCLifetime() { return false; }
   static bool emitNullabilityCheck() { return false; }
   static bool astVarDeclInterface() { return false; }
diff --git a/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp b/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp
index 52bd3b2933744..992c92635369d 100644
--- a/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp
@@ -710,6 +710,89 @@ class ScalarExprEmitter : public StmtVisitor<ScalarExprEmitter, mlir::Value> {
   HANDLEBINOP(Xor)
   HANDLEBINOP(Or)
 #undef HANDLEBINOP
+
+  mlir::Value emitCmp(const BinaryOperator *e) {
+    mlir::Value result;
+    QualType lhsTy = e->getLHS()->getType();
+    QualType rhsTy = e->getRHS()->getType();
+
+    auto clangCmpToCIRCmp =
+        [](clang::BinaryOperatorKind clangCmp) -> cir::CmpOpKind {
+      switch (clangCmp) {
+      case BO_LT:
+        return cir::CmpOpKind::lt;
+      case BO_GT:
+        return cir::CmpOpKind::gt;
+      case BO_LE:
+        return cir::CmpOpKind::le;
+      case BO_GE:
+        return cir::CmpOpKind::ge;
+      case BO_EQ:
+        return cir::CmpOpKind::eq;
+      case BO_NE:
+        return cir::CmpOpKind::ne;
+      default:
+        llvm_unreachable("unsupported comparison kind");
+      }
+    };
+
+    if (lhsTy->getAs<MemberPointerType>()) {
+      assert(e->getOpcode() == BO_EQ || e->getOpcode() == BO_NE);
+      mlir::Value lhs = cgf.emitScalarExpr(e->getLHS());
+      mlir::Value rhs = cgf.emitScalarExpr(e->getRHS());
+      cir::CmpOpKind kind = clangCmpToCIRCmp(e->getOpcode());
+      result =
+          builder.createCompare(cgf.getLoc(e->getExprLoc()), kind, lhs, rhs);
+    } else if (!lhsTy->isAnyComplexType() && !rhsTy->isAnyComplexType()) {
+      BinOpInfo boInfo = emitBinOps(e);
+      mlir::Value lhs = boInfo.lhs;
+      mlir::Value rhs = boInfo.rhs;
+
+      if (lhsTy->isVectorType()) {
+        assert(!cir::MissingFeatures::vectorType());
+        cgf.cgm.errorNYI(boInfo.loc, "vector comparisons");
+        result = builder.getBool(false, cgf.getLoc(boInfo.loc));
+      } else if (boInfo.isFixedPointOp()) {
+        assert(!cir::MissingFeatures::fixedPointType());
+        cgf.cgm.errorNYI(boInfo.loc, "fixed point comparisons");
+        result = builder.getBool(false, cgf.getLoc(boInfo.loc));
+      } else if (lhsTy->hasSignedIntegerRepresentation()) {
+        cir::CmpOpKind kind = clangCmpToCIRCmp(e->getOpcode());
+        result = builder.createCompare(cgf.getLoc(boInfo.loc), kind, lhs, rhs);
+      } else {
+        // Unsigned integers and pointers.
+        if (cgf.cgm.getCodeGenOpts().StrictVTablePointers &&
+            mlir::isa<cir::PointerType>(lhs.getType()) &&
+            mlir::isa<cir::PointerType>(rhs.getType())) {
+          cgf.cgm.errorNYI(boInfo.loc, "strict vtable pointer comparisons");
+          result = builder.getBool(false, cgf.getLoc(boInfo.loc));
+        }
+
+        cir::CmpOpKind kind = clangCmpToCIRCmp(e->getOpcode());
+        result = builder.createCompare(cgf.getLoc(boInfo.loc), kind, lhs, rhs);
+      }
+    } else {
+      // Complex Comparison: can only be an equality comparison.
+      assert(!cir::MissingFeatures::complexType());
+      const mlir::Location loc = cgf.getLoc(e->getSourceRange());
+      cgf.cgm.errorNYI(loc, "complex comparison");
+      result = builder.getBool(false, loc);
+    }
+
+    return emitScalarConversion(result, cgf.getContext().BoolTy, e->getType(),
+                                e->getExprLoc());
+  }
+
+// Comparisons.
+#define VISITCOMP(CODE)                                                        \
+  mlir::Value VisitBin##CODE(const BinaryOperator *E) { return emitCmp(E); }
+  VISITCOMP(LT)
+  VISITCOMP(GT)
+  VISITCOMP(LE)
+  VISITCOMP(GE)
+  VISITCOMP(EQ)
+  VISITCOMP(NE)
+#undef VISITCOMP
 };
 
 LValue ScalarExprEmitter::emitCompoundAssignLValue(
diff --git a/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp b/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
index 1c2b9ad05a132..f58e1cb21bb49 100644
--- a/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
+++ b/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
@@ -20,6 +20,7 @@
 #include "mlir/Dialect/LLVMIR/LLVMDialect.h"
 #include "mlir/IR/BuiltinDialect.h"
 #include "mlir/IR/BuiltinOps.h"
+#include "mlir/IR/Types.h"
 #include "mlir/Pass/Pass.h"
 #include "mlir/Pass/PassManager.h"
 #include "mlir/Target/LLVMIR/Dialect/Builtin/BuiltinToLLVMIRTranslation.h"
@@ -514,9 +515,17 @@ mlir::LogicalResult CIRToLLVMCastOpLowering::matchAndRewrite(
     assert(!MissingFeatures::cxxABI());
     assert(!MissingFeatures::dataMemberType());
     break;
-  case cir::CastKind::ptr_to_bool:
-    assert(!cir::MissingFeatures::opCmp());
+  case cir::CastKind::ptr_to_bool: {
+    auto zero =
+        mlir::IntegerAttr::get(mlir::IntegerType::get(getContext(), 64), 0);
+    auto null = rewriter.create<cir::ConstantOp>(
+        castOp.getLoc(), castOp.getSrc().getType(),
+        cir::ConstPtrAttr::get(getContext(), castOp.getSrc().getType(), zero));
+    rewriter.replaceOpWithNewOp<cir::CmpOp>(
+        castOp, cir::BoolType::get(getContext()), cir::CmpOpKind::ne,
+        castOp.getSrc(), null);
     break;
+  }
   case cir::CastKind::address_space: {
     mlir::Type dstTy = castOp.getType();
     mlir::Value llvmSrcVal = adaptor.getOperands().front();
@@ -1117,6 +1126,86 @@ mlir::LogicalResult CIRToLLVMBinOpLowering::matchAndRewrite(
   return mlir::LogicalResult::success();
 }
 
+/// Convert from a CIR comparison kind to an LLVM IR integral comparison kind.
+static mlir::LLVM::ICmpPredicate
+convertCmpKindToICmpPredicate(cir::CmpOpKind kind, bool isSigned) {
+  using CIR = cir::CmpOpKind;
+  using LLVMICmp = mlir::LLVM::ICmpPredicate;
+  switch (kind) {
+  case CIR::eq:
+    return LLVMICmp::eq;
+  case CIR::ne:
+    return LLVMICmp::ne;
+  case CIR::lt:
+    return (isSigned ? LLVMICmp::slt : LLVMICmp::ult);
+  case CIR::le:
+    return (isSigned ? LLVMICmp::sle : LLVMICmp::ule);
+  case CIR::gt:
+    return (isSigned ? LLVMICmp::sgt : LLVMICmp::ugt);
+  case CIR::ge:
+    return (isSigned ? LLVMICmp::sge : LLVMICmp::uge);
+  }
+  llvm_unreachable("Unknown CmpOpKind");
+}
+
+/// Convert from a CIR comparison kind to an LLVM IR floating-point comparison
+/// kind.
+static mlir::LLVM::FCmpPredicate
+convertCmpKindToFCmpPredicate(cir::CmpOpKind kind) {
+  using CIR = cir::CmpOpKind;
+  using LLVMFCmp = mlir::LLVM::FCmpPredicate;
+  switch (kind) {
+  case CIR::eq:
+    return LLVMFCmp::oeq;
+  case CIR::ne:
+    return LLVMFCmp::une;
+  case CIR::lt:
+    return LLVMFCmp::olt;
+  case CIR::le:
+    return LLVMFCmp::ole;
+  case CIR::gt:
+    return LLVMFCmp::ogt;
+  case CIR::ge:
+    return LLVMFCmp::oge;
+  }
+  llvm_unreachable("Unknown CmpOpKind");
+}
+
+mlir::LogicalResult CIRToLLVMCmpOpLowering::matchAndRewrite(
+    cir::CmpOp cmpOp, OpAdaptor adaptor,
+    mlir::ConversionPatternRewriter &rewriter) const {
+  mlir::Type type = cmpOp.getLhs().getType();
+
+  assert(!cir::MissingFeatures::dataMemberType());
+  assert(!cir::MissingFeatures::methodType());
+
+  // Lower to LLVM comparison op.
+  if (mlir::isa<cir::IntType, mlir::IntegerType>(type)) {
+    bool isSigned = mlir::isa<cir::IntType>(type)
+                        ? mlir::cast<cir::IntType>(type).isSigned()
+                        : mlir::cast<mlir::IntegerType>(type).isSigned();
+    mlir::LLVM::ICmpPredicate kind =
+        convertCmpKindToICmpPredicate(cmpOp.getKind(), isSigned);
+    rewriter.replaceOpWithNewOp<mlir::LLVM::ICmpOp>(
+        cmpOp, kind, adaptor.getLhs(), adaptor.getRhs());
+  } else if (auto ptrTy = mlir::dyn_cast<cir::PointerType>(type)) {
+    mlir::LLVM::ICmpPredicate kind =
+        convertCmpKindToICmpPredicate(cmpOp.getKind(),
+                                      /* isSigned=*/false);
+    rewriter.replaceOpWithNewOp<mlir::LLVM::ICmpOp>(
+        cmpOp, kind, adaptor.getLhs(), adaptor.getRhs());
+  } else if (mlir::isa<cir::CIRFPTypeInterface>(type)) {
+    mlir::LLVM::FCmpPredicate kind =
+        convertCmpKindToFCmpPredicate(cmpOp.getKind());
+    rewriter.replaceOpWithNewOp<mlir::LLVM::FCmpOp>(
+        cmpOp, kind, adaptor.getLhs(), adaptor.getRhs());
+  } else {
+    return cmpOp.emitError() << "unsupported type for CmpOp: " << type;
+  }
+
+  return mlir::success();
+}
+
 static void prepareTypeConverter(mlir::LLVMTypeConverter &converter,
                                  mlir::DataLayout &dataLayout) {
   converter.addConversion([&](cir::PointerType type) -> mlir::Type {
@@ -1258,6 +1347,7 @@ void ConvertCIRToLLVMPass::runOnOperation() {
                CIRToLLVMBinOpLowering,
                CIRToLLVMBrCondOpLowering,
                CIRToLLVMBrOpLowering,
+               CIRToLLVMCmpOpLowering,
                CIRToLLVMFuncOpLowering,
                CIRToLLVMTrapOpLowering,
                CIRToLLVMUnaryOpLowering
diff --git a/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.h b/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.h
index ef0bb2deaccdf..c57f86f57ecde 100644
--- a/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.h
+++ b/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.h
@@ -189,6 +189,20 @@ class CIRToLLVMBinOpLowering : public mlir::OpConversionPattern<cir::BinOp> {
                   mlir::ConversionPatternRewriter &) const override;
 };
 
+class CIRToLLVMCmpOpLowering : public mlir::OpConversionPattern<cir::CmpOp> {
+public:
+  CIRToLLVMCmpOpLowering(const mlir::TypeConverter &typeConverter,
+                         mlir::MLIRContext *context)
+      : OpConversionPattern(typeConverter, context) {
+    setHasBoundedRewriteRecursion();
+  }
+
+  mlir::LogicalResult
+  matchAndRewrite(cir::CmpOp 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/cmp.cpp b/clang/test/CIR/CodeGen/cmp.cpp
new file mode 100644
index 0000000000000..57d6b5b411f98
--- /dev/null
+++ b/clang/test/CIR/CodeGen/cmp.cpp
@@ -0,0 +1,233 @@
+// RUN: %clang_cc1 -std=c++17 -triple x86_64-unknown-linux-gnu -fclangir -emit-cir -DCIR_ONLY %s -o %t.cir
+// RUN: FileCheck --input-file=%t.cir %s -check-prefix=CIR
+// RUN: %clang_cc1 -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
+
+void c0(int a, int b) {
+  bool x = a > b;
+  x = a < b;
+  x = a <= b;
+  x = a >= b;
+  x = a != b;
+  x = a == b;
+}
+
+// CIR: cir.func @c0(
+
+// CIR: %[[A_PTR:.*]] = cir.alloca !s32i, !cir.ptr<!s32i>, ["a", init]
+// CIR: %[[B_PTR:.*]] = cir.alloca !s32i, !cir.ptr<!s32i>, ["b", init]
+// CIR: %[[X_PTR:.*]] = cir.alloca !cir.bool, !cir.ptr<!cir.bool>, ["x", init]
+
+// CIR: %[[A1:.*]] = cir.load %[[A_PTR]]
+// CIR: %[[B1:.*]] = cir.load %[[B_PTR]]
+// CIR: %{{.*}} = cir.cmp(gt, %[[A1]], %[[B1]]) : !s32i, !cir.bool
+// CIR: cir.store {{.*}}, %[[X_PTR]]
+
+// CIR: %[[A2:.*]] = cir.load %[[A_PTR]]
+// CIR: %[[B2:.*]] = cir.load %[[B_PTR]]
+// CIR: %{{.*}} = cir.cmp(lt, %[[A2]], %[[B2]]) : !s32i, !cir.bool
+
+// CIR: %[[A3:.*]] = cir.load %[[A_PTR]]
+// CIR: %[[B3:.*]] = cir.load %[[B_PTR]]
+// CIR: %{{.*}} = cir.cmp(le, %[[A3]], %[[B3]]) : !s32i, !cir.bool
+
+// CIR: %[[A4:.*]] = cir.load %[[A_PTR]]
+// CIR: %[[B4:.*]] = cir.load %[[B_PTR]]
+// CIR: %{{.*}} = cir.cmp(ge, %[[A4]], %[[B4]]) : !s32i, !cir.bool
+
+// CIR: %[[A5:.*]] = cir.load %[[A_PTR]]
+// CIR: %[[B5:.*]] = cir.load %[[B_PTR]]
+// CIR: %{{.*}} = cir.cmp(ne, %[[A5]], %[[B5]]) : !s32i, !cir.bool
+
+// CIR: %[[A6:.*]] = cir.load %[[A_PTR]]
+// CIR: %[[B6:.*]] = cir.load %[[B_PTR]]
+// CIR: %{{.*}} = cir.cmp(eq, %[[A6]], %[[B6]]) : !s32i, !cir.bool
+
+// LLVM: define void @c0(i32 %0, i32 %1) {
+// LLVM: %[[PTR1:.*]] = alloca i32, i64 1
+// LLVM: %[[PTR2:.*]] = alloca i32, i64 1
+// LLVM: %[[BOOL_PTR:.*]] = alloca i8, i64 1
+// LLVM: store i32 %0, ptr %[[PTR1]]
+// LLVM: store i32 %1, ptr %[[PTR2]]
+
+// LLVM: %[[A1:.*]] = load i32, ptr %[[PTR1]]
+// LLVM: %[[B1:.*]] = load i32, ptr %[[PTR2]]
+// LLVM: %[[CMP1:.*]] = icmp sgt i32 %[[A1]], %[[B1]]
+// LLVM: %[[ZEXT1:.*]] = zext i1 %[[CMP1]] to i8
+// LLVM: store i8 %[[ZEXT1]], ptr %[[BOOL_PTR]]
+
+// LLVM: %[[A2:.*]] = load i32, ptr %[[PTR1]]
+// LLVM: %[[B2:.*]] = load i32, ptr %[[PTR2]]
+// LLVM: %[[CMP2:.*]] = icmp slt i32 %[[A2]], %[[B2]]
+// LLVM: %[[ZEXT2:.*]] = zext i1 %[[CMP2]] to i8
+// LLVM: store i8 %[[ZEXT2]], ptr %[[BOOL_PTR]]
+
+// LLVM: %[[A3:.*]] = load i32, ptr %[[PTR1]]
+// LLVM: %[[B3:.*]] = load i32, ptr %[[PTR2]]
+// LLVM: %[[CMP3:.*]] = icmp sle i32 %[[A3]], %[[B3]]
+// LLVM: %[[ZEXT3:.*]] = zext i1 %[[CMP3]] to i8
+// LLVM: store i8 %[[ZEXT3]], ptr %[[BOOL_PTR]]
+
+// LLVM: %[[A4:.*]] = load i32, ptr %[[PTR1]]
+// LLVM: %[[B4:.*]] = load i32, ptr %[[PTR2]]
+// LLVM: %[[CMP4:.*]] = icmp sge i32 %[[A4]], %[[B4]]
+// LLVM: %[[ZEXT4:.*]] = zext i1 %[[CMP4]] to i8
+// LLVM: store i8 %[[ZEXT4]], ptr %[[BOOL_PTR]]
+
+// LLVM: %[[A5:.*]] = load i32, ptr %[[PTR1]]
+// LLVM: %[[B5:.*]] = load i32, ptr %[[PTR2]]
+// LLVM: %[[CMP5:.*]] = icmp ne i32 %[[A5]], %[[B5]]
+// LLVM: %[[ZEXT5:.*]] = zext i1 %[[CMP5]] to i8
+// LLVM: store i8 %[[ZEXT5]], ptr %[[BOOL_PTR]]
+
+// LLVM: %[[A6:.*]] = load i32, ptr %[[PTR1]]
+// LLVM: %[[B6:.*]] = load i32, ptr %[[PTR2]]
+// LLVM: %[[CMP6:.*]] = icmp eq i32 %[[A6]], %[[B6]]
+// LLVM: %[[ZEXT6:.*]] = zext i1 %[[CMP6]] to i8
+// LLVM: store i8 %[[ZEXT6]], ptr %[[BOOL_PTR]]
+
+void c0_unsigned(unsigned int a, unsigned int b) {
+  bool x = a > b;
+  x = a < b;
+  x = a <= b;
+  x = a >= b;
+  x = a != b;
+  x = a == b;
+}
+
+// CIR: cir.func @c0_unsigned(
+
+// CIR: %[[U_A_PTR:.*]] = cir.alloca !u32i, !cir.ptr<!u32i>, ["a", init]
+// CIR: %[[U_B_PTR:.*]] = cir.alloca !u32i, !cir.ptr<!u32i>, ["b", init]
+// CIR: %[[U_X_PTR:.*]] = cir.alloca !cir.bool, !cir.ptr<!cir.bool>, ["x", init]
+
+// CIR: %[[UA1:.*]] = cir.load %[[U_A_PTR]]
+// CIR: %[[UB1:.*]] = cir.load %[[U_B_PTR]]
+// CIR: %{{.*}} = cir.cmp(gt, %[[UA1]], %[[UB1]]) : !u32i, !cir.bool
+
+// CIR: %[[UA2:.*]] = cir.load %[[U_A_PTR]]
+// CIR: %[[UB2:.*]] = cir.load %[[U_B_PTR]]
+// CIR: %{{.*}} = cir.cmp(lt, %[[UA2]], %[[UB2]]) : !u32i, !cir.bool
+
+// CIR: %[[UA3:.*]] = cir.load %[[U_A_PTR]]
+// CIR: %[[UB3:.*]] = cir.load %[[U_B_PTR]]
+// CIR: %{{.*}} = cir.cmp(le, %[[UA3]], %[[UB3]]) : !u32i, !cir.bool
+
+// CIR: %[[UA4:.*]] = cir.load %[[U_A_PTR]]
+// CIR: %[[UB4:.*]] = cir.load %[[U_B_PTR]]
+// CIR: %{{.*}} = cir.cmp(ge, %[[UA4]], %[[UB4]]) : !u32i, !cir.bool
+
+// CIR: %[[UA5:.*]] = cir.load %[[U_A_PTR]]
+// CIR: %[[UB5:.*]] = cir.load %[[U_B_PTR]]
+// CIR: %{{.*}} = cir.cmp(ne, %[[UA5]], %[[UB5]]) : !u32i, !cir.bool
+
+// CIR: %[[UA6:.*]] = cir.load %[[U_A_PTR]]
+// CIR: %[[UB6:.*]] = cir.load %[[U_B_PTR]]
+// CIR: %{{.*}} = cir.cmp(eq, %[[UA6]], %[[UB6]]) : !u32i, !cir.bool
+
+// LLVM: define void @c0_unsigned(i32 %0, i32 %1) {
+// LLVM: %[[U_PTR1:.*]] = alloca i32, i64 1
+// LLVM: %[[U_PTR2:.*]] = alloca i32, i64 1
+// LLVM: %[[U_BOOL_PTR:.*]] = alloca i8, i64 1
+// LLVM: store i32 %0, ptr %[[U_PTR1]]
+// LLVM: store i32 %1, ptr %[[U_PTR2]]
+
+// LLVM: %[[UA1:.*]] = load i32, ptr %[[U_PTR1]]
+// LLVM: %[[UB1:.*]] = load i32, ptr %[[U_PTR2]]
+// LLVM: %[[UCMP1:.*]] = icmp ugt i32 %[[UA1]], %[[UB1]]
+// LLVM: %[[UZEXT1:.*]] = zext i1 %[[UCMP1]] to i8
+// LLVM: store i8 %[[UZEXT1]], ptr %[[U_BOOL_PTR]]
+
+// LLVM: %[[UA2:.*]] = load i32, ptr %[[U_PTR1]]
+// LLVM: %[[UB2:.*]] = load i32, ptr %[[U_PTR2]]
+// LLVM: %[[UCMP2:.*]] = icmp ult i32 %[[UA2]], %[[UB2]]
+// LLVM: %[[UZEXT2:.*]] = zext i1 %[[UCMP2]] to i8
+// LLVM: store i8 %[[UZEXT2]], ptr %[[U_BOOL_PTR]]
+
+// LLVM: %[[UA3:.*]] = load i32, ptr %[[U_PTR1]]
+// LLVM: %[[UB3:.*]] = load i32, ptr %[[U_PTR2]]
+// LLVM: %[[UCMP3:.*]] = icmp ule i32 %[[UA3]], %[[UB3]]
+// LLVM: %[[UZEXT3:.*]] = zext i1 %[[UCMP3]] to i8
+// LLVM: store i8 %[[UZEXT3]], ptr %[[U_BOOL_PTR]]
+
+// LLVM: %[[UA4:.*]] = load i32, ptr %[[U_PTR1]]
+// LLVM: %[[UB4:.*]] = load i32, ptr %[[U_PTR2]]
+// LLVM: %[[UCMP4:.*]] = icmp uge i32 %[[UA4]], %[[UB4]]
+// LLVM: %[[UZEXT4:.*]] = zext i1 %[[UCMP4]] to i8
+// LLVM: store i8 %[[UZEXT4]], ptr %[[U_BOOL_PTR]]
+
+// LLVM: %[[UA5:.*]] = load i32, ptr %[[U_PTR1]]
+// LLVM: %[[UB5:.*]] = load i32, ptr %[[U_PTR2]]
+// LLVM: %[[UCMP5:.*]] = icmp ne i32 %[[UA5]], %[[UB5]]
+// LLVM: %[[UZEXT5:.*]] = zext i1 %[[UCMP5]] to i8
+// LLVM: store i8 %[[UZEXT5]], ptr %[[U_BOOL_PTR]]
+
+// LLVM: %[[UA6:.*]] = load i32, ptr %[[U_PTR1]]
+// LLVM: %[[UB6:.*]] = load i32, ptr %[[U_PTR2]]
+// LLVM: %[[UCMP6:.*]] = icmp eq i32 %[[UA6]], %[[UB6]]
+// LLVM: %[[UZEXT6:.*]] = zext i1 %[[UCMP6]] to i8
+// LLVM: store i8 %[[UZEXT6]], ptr %[[U_BOOL_PTR]]
+
+void c0_float(float a, float b) {
+  bool x = a > b;
+  x = a < b;
+  x = a <= b;
+  x = a >= b;
+  x = a != b;
+  x = a == b;
+}
+
+// CIR: cir.func @c0_float(%arg0: !cir.float{{.*}}, %arg1: !cir.float{{.*}}) {
+// CIR: %[[A_PTR:.*]] = cir.alloca !cir.float, !cir.ptr<!cir.float>, ["a", init]
+// CIR: %[[B_PTR:.*]] = cir.alloca !cir.float, !cir.ptr<!cir.float>, ["b", init]
+// CIR: %[[X_PTR:.*]] = cir.alloca !cir.bool, !cir.ptr<!cir.bool>, ["x", init]
+
+// CIR: cir.store %arg0, %[[A_PTR]] : !cir.float, !cir.ptr<!cir.float>
+// CIR: cir.store ...
[truncated]

mmha · 2025-03-26T20:54:06Z

cc @andykaylor @erichkeane @dkolsen-pgi

github-actions · 2025-03-26T20:56:45Z

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

andykaylor

This looks really good. I have just a few minor comments and a request for more tests.

clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp

andykaylor · 2025-03-26T22:39:07Z

clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp

+        assert(!cir::MissingFeatures::fixedPointType());
+        cgf.cgm.errorNYI(boInfo.loc, "fixed point comparisons");
+        result = builder.getBool(false, cgf.getLoc(boInfo.loc));
+      } else if (lhsTy->hasSignedIntegerRepresentation()) {


Suggested change

} else if (lhsTy->hasSignedIntegerRepresentation()) {

} else if (lhsTy->hasIntegerRepresentation()) {

We don't handle signed and unsigned compare differently, unlike classic codegen.

I think it's better to remove that branch altogether and let the else deal with it.

clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp

andykaylor · 2025-03-26T22:42:21Z

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

@@ -514,9 +515,17 @@ mlir::LogicalResult CIRToLLVMCastOpLowering::matchAndRewrite(
    assert(!MissingFeatures::cxxABI());
    assert(!MissingFeatures::dataMemberType());
    break;
-  case cir::CastKind::ptr_to_bool:
-    assert(!cir::MissingFeatures::opCmp());
+  case cir::CastKind::ptr_to_bool: {


Can this lower directly to LLVM comparison like I just did for int_to_bool? Is anything added by going through the cir comparison?

I wanted to ask you the same question so I guess the answer is no :D

As far as I know, there is no additional benefit to going through the cir ops.

@bcardosolopes Can you comment on this?

Sometimes we'd go for the CIR ops because they already have a canonical way to lower and if something changes there, we get the benefits elsewhere (there are also cases where it might be confusing to look at the adaptor versus the CIR operation in other to take a decision, in which case fallbacking to known CIR lowering also helps). This change doesn't seem related to those cases and LGTM.

clang/test/CIR/CodeGen/cmp.cpp

clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp

bcardosolopes

I don't have anything to add on top of Andy's review. LGTM once those comments are addressed.

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

xlauko · 2025-03-27T09:22:00Z

clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp

+
+    return emitScalarConversion(result, cgf.getContext().BoolTy, e->getType(),
+                                e->getExprLoc());
+  }


There is a bit of a mess how locations are emitted here:

cgf.getLoc(e->getExprLoc()) vs. cgf.getLoc(boInfo.loc) vs. cgf.getLoc(e->getSourceRange())

tbf I am not sure what is correct way, or whether clangir even has some rule of thumb how AST locations are translated to mlir locations. Just the inconsistency caught my eye. @bcardosolopes @andykaylor ?

FWIW classic codegen uses getExprLoc and doesnt emit a range.

In any case I get the Location at the beginning of the function now and store it in a variable.

whether clangir even has some rule of thumb how AST locations are translated to mlir locations

we try to capture what makes more sense w.r.t source code and good diagnostic experience, but I wouldn't claim we have done a diligent process, so I can't attest for the quality. I fixed many bad source locations when working on the lifetime checker, so it's somewhat reliable, but I haven't tested much away from that.

I agree that we need to look at context to decide which source location/range makes sense. I've seen problems where we were getting source locations from operands rather than from the expression where the operand is used, which was wrong in that case.

andykaylor · 2025-03-31T17:01:45Z

clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp

+        cgf.cgm.errorNYI(loc, "fixed point comparisons");
+        result = builder.getBool(false, loc);
+      } else {
+        // Unsigned integers and pointers.


Suggested change

// Unsigned integers and pointers.

// integers and pointers

andykaylor · 2025-03-31T17:35:33Z

clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp

+
+    return emitScalarConversion(result, cgf.getContext().BoolTy, e->getType(),
+                                e->getExprLoc());
+  }


I agree that we need to look at context to decide which source location/range makes sense. I've seen problems where we were getting source locations from operands rather than from the expression where the operand is used, which was wrong in that case.

andykaylor · 2025-03-31T17:37:17Z

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

@@ -514,9 +515,17 @@ mlir::LogicalResult CIRToLLVMCastOpLowering::matchAndRewrite(
    assert(!MissingFeatures::cxxABI());
    assert(!MissingFeatures::dataMemberType());
    break;
-  case cir::CastKind::ptr_to_bool:
-    assert(!cir::MissingFeatures::opCmp());
+  case cir::CastKind::ptr_to_bool: {


As far as I know, there is no additional benefit to going through the cir ops.

@bcardosolopes Can you comment on this?

erichkeane

No comments on my run through, so Andy-et-al + Bruno is enough for me.

bcardosolopes

LGTM after you add a roundtrip test for cir::CmpOp (in a .cir file).

This patch adds support for comparison operators with ClangIR, both integral and floating point.

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

- Expand testing

- Fix comment - Add roundtrip test

andykaylor

lgtm

mmha requested review from lanza and bcardosolopes as code owners March 26, 2025 20:53

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

andykaylor reviewed Mar 26, 2025

View reviewed changes

bcardosolopes approved these changes Mar 26, 2025

View reviewed changes

xlauko reviewed Mar 27, 2025

View reviewed changes

andykaylor reviewed Mar 31, 2025

View reviewed changes

andykaylor mentioned this pull request Apr 1, 2025

[CIR] Implement lowering of int-to-bool casts #132996

Merged

erichkeane reviewed Apr 1, 2025

View reviewed changes

bcardosolopes approved these changes Apr 1, 2025

View reviewed changes

mmha and others added 5 commits April 8, 2025 15:02

[CIR] Upstream CmpOp

57b1d4e

This patch adds support for comparison operators with ClangIR, both integral and floating point.

clang-format

eecc234

Apply suggestions from code review

6a3216e

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

Address review feedback

7753dc3

- Expand testing

Review feedback

f28235f

- Fix comment - Add roundtrip test

mmha force-pushed the cir-comparisons branch from c38442e to f28235f Compare April 8, 2025 13:38

andykaylor approved these changes Apr 8, 2025

View reviewed changes

andykaylor merged commit 441f879 into llvm:main Apr 8, 2025
11 checks passed

	} else if (lhsTy->hasSignedIntegerRepresentation()) {
	} else if (lhsTy->hasIntegerRepresentation()) {

[CIR] Upstream CmpOp #133159

[CIR] Upstream CmpOp #133159

Uh oh!

Conversation

mmha commented Mar 26, 2025

Uh oh!

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

Uh oh!

Uh oh!

mmha commented Mar 26, 2025

Uh oh!

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

Uh oh!

Uh oh!

andykaylor left a comment

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!

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!

Uh oh!

Uh oh!

bcardosolopes left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mmha Mar 27, 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!

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!

erichkeane left a comment

Choose a reason for hiding this comment

Uh oh!

bcardosolopes left a comment

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!

llvmbot commented Mar 26, 2025 •

edited

Loading

github-actions bot commented Mar 26, 2025 •

edited

Loading

mmha Mar 27, 2025 •

edited

Loading