[CIR] Upstream CastOp and scalar conversions #130690

mmha · 2025-03-11T00:13:12Z

This patch upstreams ClangIR's CastOp with the following exceptions:

No Fixed/FP conversions
No casts between value categories
No complex casts
No array_to_ptrdecay
No address_space
No casts involving record types (member pointers, base/derived casts)
No casts specific to ObjC or OpenCL

llvmbot · 2025-03-11T00:13:43Z

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clangir

Author: Morris Hafner (mmha)

Changes

This patch upstreams ClangIR's CastOp with the following exceptions:

No Fixed/FP conversions
No casts between value categories
No complex casts
No array_to_ptrdecay
No address_space
No casts involving record types (member pointers, base/derived casts)
No casts specific to ObjC or OpenCL

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

13 Files Affected:

(modified) clang/include/clang/CIR/Dialect/Builder/CIRBaseBuilder.h (+62)
(modified) clang/include/clang/CIR/Dialect/IR/CIROps.td (+105)
(modified) clang/include/clang/CIR/MissingFeatures.h (+13-1)
(modified) clang/lib/CIR/CodeGen/CIRGenBuilder.h (+9)
(modified) clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp (+389-11)
(modified) clang/lib/CIR/CodeGen/CIRGenFunction.h (+3)
(modified) clang/lib/CIR/Dialect/IR/CIRDialect.cpp (+170)
(modified) clang/lib/CIR/Dialect/IR/CIRMemorySlot.cpp (+27)
(modified) clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp (+207)
(modified) clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.h (+16)
(added) clang/test/CIR/CodeGen/cast.cpp (+58)
(added) clang/test/CIR/IR/cast.cir (+23)
(added) clang/test/CIR/Lowering/cast.cir (+92)

diff --git a/clang/include/clang/CIR/Dialect/Builder/CIRBaseBuilder.h b/clang/include/clang/CIR/Dialect/Builder/CIRBaseBuilder.h
index 017ae0c53a984..e5e8132e9f527 100644
--- a/clang/include/clang/CIR/Dialect/Builder/CIRBaseBuilder.h
+++ b/clang/include/clang/CIR/Dialect/Builder/CIRBaseBuilder.h
@@ -13,6 +13,7 @@
 #include "clang/CIR/Dialect/IR/CIRAttrs.h"
 #include "clang/CIR/Dialect/IR/CIRDialect.h"
 #include "clang/CIR/Dialect/IR/CIRTypes.h"
+#include "llvm/Support/ErrorHandling.h"
 
 #include "mlir/IR/Builders.h"
 #include "mlir/IR/BuiltinTypes.h"
@@ -78,6 +79,67 @@ class CIRBaseBuilderTy : public mlir::OpBuilder {
     return create<cir::StoreOp>(loc, val, dst);
   }
 
+  //===--------------------------------------------------------------------===//
+  // Cast/Conversion Operators
+  //===--------------------------------------------------------------------===//
+
+  mlir::Value createCast(mlir::Location loc, cir::CastKind kind,
+                         mlir::Value src, mlir::Type newTy) {
+    if (newTy == src.getType())
+      return src;
+    return create<cir::CastOp>(loc, newTy, kind, src);
+  }
+
+  mlir::Value createCast(cir::CastKind kind, mlir::Value src,
+                         mlir::Type newTy) {
+    if (newTy == src.getType())
+      return src;
+    return createCast(src.getLoc(), kind, src, newTy);
+  }
+
+  mlir::Value createIntCast(mlir::Value src, mlir::Type newTy) {
+    return createCast(cir::CastKind::integral, src, newTy);
+  }
+
+  mlir::Value createIntToPtr(mlir::Value src, mlir::Type newTy) {
+    return createCast(cir::CastKind::int_to_ptr, src, newTy);
+  }
+
+  mlir::Value createPtrToInt(mlir::Value src, mlir::Type newTy) {
+    return createCast(cir::CastKind::ptr_to_int, src, newTy);
+  }
+
+  mlir::Value createPtrToBoolCast(mlir::Value v) {
+    return createCast(cir::CastKind::ptr_to_bool, v, getBoolTy());
+  }
+
+  mlir::Value createBoolToInt(mlir::Value src, mlir::Type newTy) {
+    return createCast(cir::CastKind::bool_to_int, src, newTy);
+  }
+
+  mlir::Value createBitcast(mlir::Value src, mlir::Type newTy) {
+    return createCast(cir::CastKind::bitcast, src, newTy);
+  }
+
+  mlir::Value createBitcast(mlir::Location loc, mlir::Value src,
+                            mlir::Type newTy) {
+    return createCast(loc, cir::CastKind::bitcast, src, newTy);
+  }
+
+  mlir::Value createPtrBitcast(mlir::Value src, mlir::Type newPointeeTy) {
+    assert(mlir::isa<cir::PointerType>(src.getType()) && "expected ptr src");
+    return createBitcast(src, getPointerTo(newPointeeTy));
+  }
+
+  mlir::Value createAddrSpaceCast(mlir::Location loc, mlir::Value src,
+                                  mlir::Type newTy) {
+    return createCast(loc, cir::CastKind::address_space, src, newTy);
+  }
+
+  mlir::Value createAddrSpaceCast(mlir::Value src, mlir::Type newTy) {
+    return createAddrSpaceCast(src.getLoc(), src, newTy);
+  }
+
   //
   // Block handling helpers
   // ----------------------
diff --git a/clang/include/clang/CIR/Dialect/IR/CIROps.td b/clang/include/clang/CIR/Dialect/IR/CIROps.td
index e2ab50c78ec2d..caef0947d0b16 100644
--- a/clang/include/clang/CIR/Dialect/IR/CIROps.td
+++ b/clang/include/clang/CIR/Dialect/IR/CIROps.td
@@ -78,6 +78,111 @@ class LLVMLoweringInfo {
 class CIR_Op<string mnemonic, list<Trait> traits = []> :
     Op<CIR_Dialect, mnemonic, traits>, LLVMLoweringInfo;
 
+//===----------------------------------------------------------------------===//
+// CastOp
+//===----------------------------------------------------------------------===//
+
+// The enumaration value isn't in sync with clang.
+def CK_IntegralToBoolean : I32EnumAttrCase<"int_to_bool", 1>;
+def CK_ArrayToPointerDecay : I32EnumAttrCase<"array_to_ptrdecay", 2>;
+def CK_IntegralCast : I32EnumAttrCase<"integral", 3>;
+def CK_BitCast : I32EnumAttrCase<"bitcast", 4>;
+def CK_FloatingCast : I32EnumAttrCase<"floating", 5>;
+def CK_PtrToBoolean : I32EnumAttrCase<"ptr_to_bool", 6>;
+def CK_FloatToIntegral : I32EnumAttrCase<"float_to_int", 7>;
+def CK_IntegralToPointer : I32EnumAttrCase<"int_to_ptr", 8>;
+def CK_PointerToIntegral : I32EnumAttrCase<"ptr_to_int", 9>;
+def CK_FloatToBoolean : I32EnumAttrCase<"float_to_bool", 10>;
+def CK_BooleanToIntegral : I32EnumAttrCase<"bool_to_int", 11>;
+def CK_IntegralToFloat : I32EnumAttrCase<"int_to_float", 12>;
+def CK_BooleanToFloat : I32EnumAttrCase<"bool_to_float", 13>;
+def CK_AddressSpaceConversion : I32EnumAttrCase<"address_space", 14>;
+def CK_FloatToComplex : I32EnumAttrCase<"float_to_complex", 15>;
+def CK_IntegralToComplex : I32EnumAttrCase<"int_to_complex", 16>;
+def CK_FloatComplexToReal : I32EnumAttrCase<"float_complex_to_real", 17>;
+def CK_IntegralComplexToReal : I32EnumAttrCase<"int_complex_to_real", 18>;
+def CK_FloatComplexToBoolean : I32EnumAttrCase<"float_complex_to_bool", 19>;
+def CK_IntegralComplexToBoolean : I32EnumAttrCase<"int_complex_to_bool", 20>;
+def CK_FloatComplexCast : I32EnumAttrCase<"float_complex", 21>;
+def CK_FloatComplexToIntegralComplex
+    : I32EnumAttrCase<"float_complex_to_int_complex", 22>;
+def CK_IntegralComplexCast : I32EnumAttrCase<"int_complex", 23>;
+def CK_IntegralComplexToFloatComplex
+    : I32EnumAttrCase<"int_complex_to_float_complex", 24>;
+def CK_MemberPtrToBoolean : I32EnumAttrCase<"member_ptr_to_bool", 25>;
+
+def CastKind : I32EnumAttr<
+    "CastKind",
+    "cast kind",
+    [CK_IntegralToBoolean, CK_ArrayToPointerDecay, CK_IntegralCast,
+     CK_BitCast, CK_FloatingCast, CK_PtrToBoolean, CK_FloatToIntegral,
+     CK_IntegralToPointer, CK_PointerToIntegral, CK_FloatToBoolean,
+     CK_BooleanToIntegral, CK_IntegralToFloat, CK_BooleanToFloat,
+     CK_AddressSpaceConversion, CK_FloatToComplex, CK_IntegralToComplex,
+     CK_FloatComplexToReal, CK_IntegralComplexToReal, CK_FloatComplexToBoolean,
+     CK_IntegralComplexToBoolean, CK_FloatComplexCast,
+     CK_FloatComplexToIntegralComplex, CK_IntegralComplexCast,
+     CK_IntegralComplexToFloatComplex, CK_MemberPtrToBoolean]> {
+  let cppNamespace = "::cir";
+}
+
+def CastOp : CIR_Op<"cast",
+             [Pure,
+              DeclareOpInterfaceMethods<PromotableOpInterface>]> {
+  // FIXME: not all conversions are free of side effects.
+  let summary = "Conversion between values of different types";
+  let description = [{
+    Apply C/C++ usual conversions rules between values. Currently supported kinds:
+
+    - `array_to_ptrdecay`
+    - `bitcast`
+    - `integral`
+    - `int_to_bool`
+    - `int_to_float`
+    - `floating`
+    - `float_to_int`
+    - `float_to_bool`
+    - `ptr_to_int`
+    - `ptr_to_bool`
+    - `bool_to_int`
+    - `bool_to_float`
+    - `address_space`
+    - `float_to_complex`
+    - `int_to_complex`
+    - `float_complex_to_real`
+    - `int_complex_to_real`
+    - `float_complex_to_bool`
+    - `int_complex_to_bool`
+    - `float_complex`
+    - `float_complex_to_int_complex`
+    - `int_complex`
+    - `int_complex_to_float_complex`
+
+    This is effectively a subset of the rules from
+    `llvm-project/clang/include/clang/AST/OperationKinds.def`; but note that some
+    of the conversions aren't implemented in terms of `cir.cast`, `lvalue-to-rvalue`
+    for instance is modeled as a regular `cir.load`.
+
+    ```mlir
+    %4 = cir.cast (int_to_bool, %3 : i32), !cir.bool
+    ...
+    %x = cir.cast(array_to_ptrdecay, %0 : !cir.ptr<!cir.array<i32 x 10>>), !cir.ptr<i32>
+    ```
+  }];
+
+  let arguments = (ins CastKind:$kind, CIR_AnyType:$src);
+  let results = (outs CIR_AnyType:$result);
+
+  let assemblyFormat = [{
+    `(` $kind `,` $src `:` type($src) `)`
+    `,` type($result) attr-dict
+  }];
+
+  // The input and output types should match the cast kind.
+  let hasVerifier = 1;
+  let hasFolder = 1;
+}
+
 //===----------------------------------------------------------------------===//
 // ConstantOp
 //===----------------------------------------------------------------------===//
diff --git a/clang/include/clang/CIR/MissingFeatures.h b/clang/include/clang/CIR/MissingFeatures.h
index d20cd0560a7c1..5f1ed97f4940a 100644
--- a/clang/include/clang/CIR/MissingFeatures.h
+++ b/clang/include/clang/CIR/MissingFeatures.h
@@ -73,15 +73,27 @@ struct MissingFeatures {
   static bool opFuncVisibility() { return false; }
 
   // Misc
-  static bool scalarConversionOpts() { return false; }
+  static bool cxxABI() { return false; }
   static bool tryEmitAsConstant() { return false; }
   static bool constructABIArgDirectExtend() { return false; }
   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; }
+  static bool scalableVectors() { return false; }
+  static bool fpConstraints() { return false; }
+  static bool sanitizers() { return false; }
+  static bool addHeapAllocSiteMetadata() { return false; }
+  static bool targetCodeGenInfoGetNullPointer() { return false; }
+
+  // Missing types
+  static bool dataMemberType() { return false; }
+  static bool methodType() { return false; }
+  static bool matrixType() { return false; }
+  static bool vectorType() { return false; }
 };
 
 } // namespace cir
diff --git a/clang/lib/CIR/CodeGen/CIRGenBuilder.h b/clang/lib/CIR/CodeGen/CIRGenBuilder.h
index 01d56963883cc..76e6c53d9b6e2 100644
--- a/clang/lib/CIR/CodeGen/CIRGenBuilder.h
+++ b/clang/lib/CIR/CodeGen/CIRGenBuilder.h
@@ -10,6 +10,7 @@
 #define LLVM_CLANG_LIB_CIR_CODEGEN_CIRGENBUILDER_H
 
 #include "CIRGenTypeCache.h"
+#include "clang/CIR/MissingFeatures.h"
 
 #include "clang/CIR/Dialect/Builder/CIRBaseBuilder.h"
 
@@ -33,6 +34,14 @@ class CIRGenBuilderTy : public cir::CIRBaseBuilderTy {
       llvm_unreachable("NYI: PPC double-double format for long double");
     llvm_unreachable("Unsupported format for long double");
   }
+
+  bool isInt(mlir::Type i) { return mlir::isa<cir::IntType>(i); }
+
+  // Creates constant nullptr for pointer type ty.
+  cir::ConstantOp getNullPtr(mlir::Type ty, mlir::Location loc) {
+    assert(!cir::MissingFeatures::targetCodeGenInfoGetNullPointer());
+    return create<cir::ConstantOp>(loc, ty, getConstPtrAttr(ty, 0));
+  }
 };
 
 } // namespace clang::CIRGen
diff --git a/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp b/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp
index b9e56dc4123d6..df9447841800a 100644
--- a/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp
@@ -36,6 +36,18 @@ class ScalarExprEmitter : public StmtVisitor<ScalarExprEmitter, mlir::Value> {
                     bool ira = false)
       : cgf(cgf), builder(builder), ignoreResultAssign(ira) {}
 
+  //===--------------------------------------------------------------------===//
+  //                               Utilities
+  //===--------------------------------------------------------------------===//
+
+  bool TestAndClearIgnoreResultAssign() {
+    bool i = ignoreResultAssign;
+    ignoreResultAssign = false;
+    return i;
+  }
+
+  mlir::Type convertType(QualType t) { return cgf.convertType(t); }
+
   //===--------------------------------------------------------------------===//
   //                            Visitor Methods
   //===--------------------------------------------------------------------===//
@@ -68,14 +80,14 @@ class ScalarExprEmitter : public StmtVisitor<ScalarExprEmitter, mlir::Value> {
   }
 
   mlir::Value VisitIntegerLiteral(const IntegerLiteral *e) {
-    mlir::Type type = cgf.convertType(e->getType());
+    mlir::Type type = convertType(e->getType());
     return builder.create<cir::ConstantOp>(
         cgf.getLoc(e->getExprLoc()), type,
         builder.getAttr<cir::IntAttr>(type, e->getValue()));
   }
 
   mlir::Value VisitFloatingLiteral(const FloatingLiteral *e) {
-    mlir::Type type = cgf.convertType(e->getType());
+    mlir::Type type = convertType(e->getType());
     assert(mlir::isa<cir::CIRFPTypeInterface>(type) &&
            "expect floating-point type");
     return builder.create<cir::ConstantOp>(
@@ -84,26 +96,266 @@ class ScalarExprEmitter : public StmtVisitor<ScalarExprEmitter, mlir::Value> {
   }
 
   mlir::Value VisitCXXBoolLiteralExpr(const CXXBoolLiteralExpr *e) {
-    mlir::Type type = cgf.convertType(e->getType());
+    mlir::Type type = convertType(e->getType());
     return builder.create<cir::ConstantOp>(
         cgf.getLoc(e->getExprLoc()), type,
         builder.getCIRBoolAttr(e->getValue()));
   }
 
-  mlir::Value VisitCastExpr(CastExpr *E);
+  mlir::Value VisitCastExpr(CastExpr *e);
+
+  mlir::Value VisitExplicitCastExpr(ExplicitCastExpr *e) {
+    return VisitCastExpr(e);
+  }
+
+  /// Perform a pointer to boolean conversion.
+  mlir::Value emitPointerToBoolConversion(mlir::Value v, QualType qt) {
+    // TODO(cir): comparing the ptr to null is done when lowering CIR to LLVM.
+    // We might want to have a separate pass for these types of conversions.
+    return cgf.getBuilder().createPtrToBoolCast(v);
+  }
+
+  mlir::Value emitFloatToBoolConversion(mlir::Value src, mlir::Location loc) {
+    auto boolTy = builder.getBoolTy();
+    return builder.create<cir::CastOp>(loc, boolTy,
+                                       cir::CastKind::float_to_bool, src);
+  }
+
+  mlir::Value emitIntToBoolConversion(mlir::Value srcVal, mlir::Location loc) {
+    // Because of the type rules of C, we often end up computing a
+    // logical value, then zero extending it to int, then wanting it
+    // as a logical value again.
+    // TODO: optimize this common case here or leave it for later
+    // CIR passes?
+    mlir::Type boolTy = convertType(cgf.getContext().BoolTy);
+    return builder.create<cir::CastOp>(loc, boolTy, cir::CastKind::int_to_bool,
+                                       srcVal);
+  }
+
+  /// Convert the specified expression value to a boolean (!cir.bool) truth
+  /// value. This is equivalent to "Val != 0".
+  mlir::Value emitConversionToBool(mlir::Value src, QualType srcType,
+                                   mlir::Location loc) {
+    assert(srcType.isCanonical() && "EmitScalarConversion strips typedefs");
+
+    if (srcType->isRealFloatingType())
+      return emitFloatToBoolConversion(src, loc);
+
+    if ([[maybe_unused]] auto *mpt = llvm::dyn_cast<MemberPointerType>(srcType))
+      cgf.getCIRGenModule().errorNYI(loc, "member pointer to bool conversion");
+
+    if (srcType->isIntegerType())
+      return emitIntToBoolConversion(src, loc);
+
+    assert(::mlir::isa<cir::PointerType>(src.getType()));
+    return emitPointerToBoolConversion(src, srcType);
+  }
+
+  // Emit a conversion from the specified type to the specified destination
+  // type, both of which are CIR scalar types.
+  struct ScalarConversionOpts {
+    bool treatBooleanAsSigned;
+    bool emitImplicitIntegerTruncationChecks;
+    bool emitImplicitIntegerSignChangeChecks;
+
+    ScalarConversionOpts()
+        : treatBooleanAsSigned(false),
+          emitImplicitIntegerTruncationChecks(false),
+          emitImplicitIntegerSignChangeChecks(false) {}
+
+    ScalarConversionOpts(clang::SanitizerSet sanOpts)
+        : treatBooleanAsSigned(false),
+          emitImplicitIntegerTruncationChecks(
+              sanOpts.hasOneOf(SanitizerKind::ImplicitIntegerTruncation)),
+          emitImplicitIntegerSignChangeChecks(
+              sanOpts.has(SanitizerKind::ImplicitIntegerSignChange)) {}
+  };
+
+  // Conversion from bool, integral, or floating-point to integral or
+  // floating-point. Conversions involving other types are handled elsewhere.
+  // Conversion to bool is handled elsewhere because that's a comparison against
+  // zero, not a simple cast. This handles both individual scalars and vectors.
+  mlir::Value emitScalarCast(mlir::Value src, QualType srcType,
+                             QualType dstType, mlir::Type srcTy,
+                             mlir::Type dstTy, ScalarConversionOpts opts) {
+    assert(!srcType->isMatrixType() && !dstType->isMatrixType() &&
+           "Internal error: matrix types not handled by this function.");
+    if (mlir::isa<mlir::IntegerType>(srcTy) ||
+        mlir::isa<mlir::IntegerType>(dstTy))
+      llvm_unreachable("Obsolete code. Don't use mlir::IntegerType with CIR.");
+
+    mlir::Type fullDstTy = dstTy;
+    assert(!cir::MissingFeatures::vectorType());
+
+    std::optional<cir::CastKind> castKind;
+
+    if (mlir::isa<cir::BoolType>(srcTy)) {
+      if (opts.treatBooleanAsSigned)
+        cgf.getCIRGenModule().errorNYI("signed bool");
+      if (cgf.getBuilder().isInt(dstTy)) {
+        castKind = cir::CastKind::bool_to_int;
+      } else if (mlir::isa<cir::CIRFPTypeInterface>(dstTy)) {
+        castKind = cir::CastKind::bool_to_float;
+      } else {
+        llvm_unreachable("Internal error: Cast to unexpected type");
+      }
+    } else if (cgf.getBuilder().isInt(srcTy)) {
+      if (cgf.getBuilder().isInt(dstTy)) {
+        castKind = cir::CastKind::integral;
+      } else if (mlir::isa<cir::CIRFPTypeInterface>(dstTy)) {
+        castKind = cir::CastKind::int_to_float;
+      } else {
+        llvm_unreachable("Internal error: Cast to unexpected type");
+      }
+    } else if (mlir::isa<cir::CIRFPTypeInterface>(srcTy)) {
+      if (cgf.getBuilder().isInt(dstTy)) {
+        // If we can't recognize overflow as undefined behavior, assume that
+        // overflow saturates. This protects against normal optimizations if we
+        // are compiling with non-standard FP semantics.
+        if (!cgf.cgm.getCodeGenOpts().StrictFloatCastOverflow)
+          cgf.getCIRGenModule().errorNYI("strict float cast overflow");
+        assert(!cir::MissingFeatures::fpConstraints());
+        castKind = cir::CastKind::float_to_int;
+      } else if (mlir::isa<cir::CIRFPTypeInterface>(dstTy)) {
+        cgf.getCIRGenModule().errorNYI("floating point casts");
+      } else {
+        llvm_unreachable("Internal error: Cast to unexpected type");
+      }
+    } else {
+      llvm_unreachable("Internal error: Cast from unexpected type");
+    }
+
+    assert(castKind.has_value() && "Internal error: CastKind not set.");
+    return builder.create<cir::CastOp>(src.getLoc(), fullDstTy, *castKind, src);
+  }
 
   /// Emit a conversion from the specified type to the specified destination
   /// type, both of which are CIR scalar types.
   /// TODO: do we need ScalarConversionOpts here? Should be done in another
   /// pass.
-  mlir::Value emitScalarConversion(mlir::Value src, QualType srcType,
-                                   QualType dstType, SourceLocation loc) {
-    // No sort of type conversion is implemented yet, but the path for implicit
-    // paths goes through here even if the type isn't being changed.
+  mlir::Value
+  emitScalarConversion(mlir::Value src, QualType srcType, QualType dstType,
+                       SourceLocation loc,
+                       ScalarConversionOpts opts = ScalarConversionOpts()) {
+    // All conversions involving fixed point types should be handled by the
+    // emitFixedPoint family functions. This is done to prevent bloating up
+    // this function more, and although fixed point numbers are represented by
+    // integers, we do not want to follow any logic that assumes they should be
+    // treated as integers.
+    // TODO(leonardchan): When necessary, add another if statement checking for
+    // conversions to fixed point types from other types.
+    // conversions to fixed point types from other types.
+    if (srcType->isFixedPointType())
+      cgf.getCIRGenModule().errorNYI(loc, "fixed point conversions");
+    else if (dstType->isFixedPointType())
+      cgf.getCIRGenModule().errorNYI(loc, "fixed point conversions");
+
     srcType = srcType.getCanonicalType();
     dstType = dstType.getCanonicalType();
-    if (srcType == dstType)
+    if (srcType == dstType) {
+      if (opts.emitImplicitIntegerSignChangeChecks)
+        cgf.getCIRGenModule().errorNYI(loc,
+                                       "implicit integer sign change checks");
       return src;
+    }
+
+    if (dstType->isVoidType())
+      return nullptr;
+
+    mlir::Type srcTy = src.getType();
+
+    // Handle conversions to bool first, they are special: comparisons against
+    // 0.
+    if (dstType->isBooleanType())
+      return emitConversionToBool(src, srcType, cgf.getLoc(loc));
+
+    mlir::Ty...
[truncated]

erichkeane

We have to be mroe careful I think with uses of the errorNYI in a few places, we are using it too much like assert when it is more like emitDiag.

Also, this patch uses auto a bunch in ways that are contrary to the coding standard. See : https://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable

auto is really only to be used (rule of thumb) when the type is already 'listed' on the RHS, or naming the type is so obnoxious as to be irrelevant (iterators/etc).

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

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

clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp

erichkeane · 2025-03-11T15:06:57Z

clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp

+    }
+
+    if (srcType->isMatrixType() && dstType->isMatrixType())
+      cgf.getCIRGenModule().errorNYI(loc,


here too. it ends up just falling through to the assert below which is awkward.

still valid here, I think we need to just generate a constant of the dest type.

perhaps a `return nullptr' here?

clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp

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

clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp

clang/test/CIR/CodeGen/cast.cpp

andykaylor · 2025-03-11T20:15:54Z

clang/test/CIR/CodeGen/cast.cpp

@@ -0,0 +1,58 @@
+// RUN: %clang_cc1 -std=c++17 -triple x86_64-unknown-linux-gnu -fclangir -emit-cir %s -o %t.cir


Can you expand this test to cover all the cast types you're adding (or remove any that can't be supported yet)?

Has this been addressed?

I removed all tests for the currently unsupported casting kinds. I noticed that a few were missing (bool to int/float/ptr) which I readded.

It looks like floating casts are still missing. Can you add float-to-double and double-to-float test?

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

erichkeane · 2025-03-12T14:59:49Z

clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp

+      return emitFloatToBoolConversion(src, loc);
+
+    if (llvm::isa<MemberPointerType>(srcType))
+      cgf.getCIRGenModule().errorNYI(loc, "member pointer to bool conversion");


This branch needs to return.... something.

erichkeane · 2025-03-12T15:00:47Z

clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp

+                             mlir::Type dstTy, ScalarConversionOpts opts) {
+    assert(!srcType->isMatrixType() && !dstType->isMatrixType() &&
+           "Internal error: matrix types not handled by this function.");
+    if (mlir::isa<mlir::IntegerType>(srcTy) ||


I realize this made sense in the case of 'switching from this function to another' in the incubator, but I think this should probably be an assert instead.

Same comment again.

erichkeane · 2025-03-12T15:02:22Z

clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp

+    // conversions to fixed point types from other types.
+    // conversions to fixed point types from other types.
+    if (srcType->isFixedPointType())
+      cgf.getCIRGenModule().errorNYI(loc, "fixed point conversions");


These also need to return something. Otherwise this would fall into the below and it isn't clear it would fall into anything even vaguely sensible.

clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp

erichkeane · 2025-03-12T15:09:29Z

clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp

+    }
+
+    if (srcType->isMatrixType() && dstType->isMatrixType())
+      cgf.getCIRGenModule().errorNYI(loc,


still valid here, I think we need to just generate a constant of the dest type.

erichkeane · 2025-03-12T15:10:03Z

clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp

+  case CK_AtomicToNonAtomic:
+    cgf.getCIRGenModule().errorNYI(subExpr->getSourceRange(),
+                                   "CastExpr: ", ce->getCastKindName());
+    break;


what does this fall through to? Does it result in a sensible value?

still, atomic-to-non-atomic cast can change types, so probably should return something.

clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp

erichkeane · 2025-03-12T21:33:08Z

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

+// CastOp
+//===----------------------------------------------------------------------===//
+
+// CK_Dependent


Neat! Thank you!

erichkeane · 2025-03-12T21:34:36Z

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

+  switch (castOp.getKind()) {
+  case cir::CastKind::array_to_ptrdecay: {
+    const auto ptrTy = mlir::cast<cir::PointerType>(castOp.getType());
+    auto sourceValue = adaptor.getOperands().front();


A LOT of uses of 'auto' in this function that don't match our coding standard.

erichkeane · 2025-03-12T21:37:44Z

clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp

+  }
+
+  mlir::Value emitFloatToBoolConversion(mlir::Value src, mlir::Location loc) {
+    auto boolTy = builder.getBoolTy();


probably can't do auto here either.

erichkeane · 2025-03-12T21:38:34Z

clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp

+    // as a logical value again.
+    // TODO: optimize this common case here or leave it for later
+    // CIR passes?
+    mlir::Type boolTy = cgf.convertType(cgf.getContext().BoolTy);


why convertType here but getBoolTy on 106?

erichkeane · 2025-03-12T21:39:12Z

clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp

+
+    if (llvm::isa<MemberPointerType>(srcType)) {
+      cgf.getCIRGenModule().errorNYI(loc, "member pointer to bool conversion");
+      auto boolType = cgf.getContext().getBOOLType();


Another case we can't use autos. Also a 3rd way to get a bool type?

erichkeane · 2025-03-12T21:44:04Z

clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp

+    }
+
+    if (srcType->isMatrixType() && dstType->isMatrixType())
+      cgf.getCIRGenModule().errorNYI(loc,


perhaps a `return nullptr' here?

erichkeane · 2025-03-12T21:44:18Z

clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp

+    res = emitScalarCast(src, srcType, dstType, mlirSrcType, mlirDstType, opts);
+
+    if (mlirDstType != resTy) {
+      if (cgf.getContext().getTargetInfo().useFP16ConversionIntrinsics()) {


no curleys on single-line if.

clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp

erichkeane · 2025-03-12T21:45:35Z

clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp

+  case CK_AtomicToNonAtomic:
+    cgf.getCIRGenModule().errorNYI(subExpr->getSourceRange(),
+                                   "CastExpr: ", ce->getCastKindName());
+    break;


still, atomic-to-non-atomic cast can change types, so probably should return something.

clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp

github-actions · 2025-03-12T23:44:01Z

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

erichkeane

1 nit, else LGTM.

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

clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp

bcardosolopes

LGTM pending on question left in one comment

bcardosolopes · 2025-03-13T22:14:37Z

clang/test/CIR/CodeGen/cast.cpp

@@ -0,0 +1,58 @@
+// RUN: %clang_cc1 -std=c++17 -triple x86_64-unknown-linux-gnu -fclangir -emit-cir %s -o %t.cir


Has this been addressed?

andykaylor · 2025-03-14T16:40:43Z

clang/test/CIR/CodeGen/cast.cpp

+// CHECK: cir.func @cStyleCasts_0
+
+  char a = (char)x1; // truncate
+  // CHECK: %{{[0-9]+}} = cir.cast(integral, %{{[0-9]+}} : !cir.int<u, 32>), !cir.int<s, 8>


Maybe add checks for lowering (to LLVM IR through clangir and using classic codegen) here also?

I expanded the tests to include LLVM lowering where feasible. int_to_bool is implemented in CIR but not in the lowering (as that requires comparisons) so I ifdefed those out for the LLVM checks.

andykaylor · 2025-03-14T20:12:25Z

clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp

  QualType destTy = ce->getType();
  CastKind kind = ce->getCastKind();

+  // These cases are generally not written to ignore the result of evaluating
+  // their sub-expressions, so we clear this now.
+  ignoreResultAssign = false;


The incubator does use the value returned by TestAndClearIgnoreResultAssign() in VisitBinAssign() and emitCompoundAssign() so we'll probably want to keep the call in those places, but I don't see any value in calling the function when we aren't using the value returned.

Note that ignoreResultAssign is a member variable and retains its value while we visit the expression. We might get called recursively so we should keep this store.

Yes, I agree that keeping the store is necessary. My point was that even in the incubator this value is being used.

andykaylor

LGTM with one very minor nit

andykaylor · 2025-03-14T22:02:55Z

clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp

  QualType destTy = ce->getType();
  CastKind kind = ce->getCastKind();

+  // These cases are generally not written to ignore the result of evaluating
+  // their sub-expressions, so we clear this now.
+  ignoreResultAssign = false;


Yes, I agree that keeping the store is necessary. My point was that even in the incubator this value is being used.

andykaylor · 2025-03-14T22:04:42Z

clang/test/CIR/CodeGen/cast.cpp

+}
+
+// CIR: cir.func @cxxstaticcast_0
+// CIR:    %0 = cir.alloca !cir.int<u, 32>, !cir.ptr<!cir.int<u, 32>>, ["x", init] {alignment = 4 : i64}


You should use pattern matching for %0 and similar values.

This patch upstreams ClangIR's CastOp with the following exceptions: - No Fixed/FP conversions - No casts between value categories - No complex casts - No array_to_ptrdecay - No address_space - No casts involving record types (member pointers, base/derived casts) - No casts specific to ObjC or OpenCL

Co-authored-by: Erich Keane <[email protected]>

- Remove dead functions - Remove convertType helper function - rename some variables - change __fp16 conversion codegen to emit cast even if intrinsic is requested - Move some CHECK-NOT tests out into a separate function

- CastKind enum now aligns with classic codegen CastKind - Try to emit a value of the expected type after errorNYI()s

Co-authored-by: Erich Keane <[email protected]>

- Add createDummyValue functions - Create Bool/Ptr constants on errorNYIs - return {} instead of nullptr - Replace auto with types

Co-authored-by: Erich Keane <[email protected]>

- Handle IgnoredExpr on null_to_pointer casts

- Emit dummy value and a diagnostic during LLVM lowering for int_to_bool casts - Expand tests to cover LLVM lowering

mmha requested review from lanza and bcardosolopes as code owners March 11, 2025 00:13

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

dkolsen-pgi requested review from erichkeane, dkolsen-pgi and andykaylor March 11, 2025 01:27

erichkeane reviewed Mar 11, 2025

View reviewed changes

andykaylor reviewed Mar 11, 2025

View reviewed changes

erichkeane reviewed Mar 12, 2025

View reviewed changes

mmha force-pushed the cir-casts branch from daf4a13 to 92bf5f7 Compare March 12, 2025 21:26

erichkeane reviewed Mar 12, 2025

View reviewed changes

mmha force-pushed the cir-casts branch from 5951206 to 74f2af5 Compare March 12, 2025 23:40

erichkeane approved these changes Mar 13, 2025

View reviewed changes

clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp Outdated Show resolved Hide resolved

clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp Show resolved Hide resolved

mmha force-pushed the cir-casts branch from f7dcdad to 69b6356 Compare March 13, 2025 16:10

bcardosolopes approved these changes Mar 13, 2025

View reviewed changes

mmha force-pushed the cir-casts branch 2 times, most recently from 337b0ec to 005a4aa Compare March 14, 2025 15:52

andykaylor reviewed Mar 14, 2025

View reviewed changes

mmha force-pushed the cir-casts branch from 7c89fe4 to e8e3270 Compare March 14, 2025 20:44

andykaylor approved these changes Mar 14, 2025

View reviewed changes

mmha and others added 6 commits March 18, 2025 17:07

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

2a71951

Co-authored-by: Erich Keane <[email protected]>

Address some review feedback

62b4dd6

- Remove dead functions - Remove convertType helper function - rename some variables - change __fp16 conversion codegen to emit cast even if intrinsic is requested - Move some CHECK-NOT tests out into a separate function

More review feedback changes

1d427fd

- CastKind enum now aligns with classic codegen CastKind - Try to emit a value of the expected type after errorNYI()s

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

f7313e4

Co-authored-by: Erich Keane <[email protected]>

Addressing more review feedback

8712dcf

- Add createDummyValue functions - Create Bool/Ptr constants on errorNYIs - return {} instead of nullptr - Replace auto with types

mmha and others added 9 commits March 18, 2025 17:07

Add support for CK_ToVoid

897ec50

clang-format

e4faae7

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

3d7a63f

Co-authored-by: Erich Keane <[email protected]>

- Add more test cases (int_to_bool, ptr_to_bool, float_to_bool)

f0dc882

- Handle IgnoredExpr on null_to_pointer casts

fix test

ff4bd8a

clang-format

a589426

- Correctly emit dummy value for floating casts

57be558

- Emit dummy value and a diagnostic during LLVM lowering for int_to_bool casts - Expand tests to cover LLVM lowering

Use pattern matching for CIR/LLVM registers

7f43095

Fix MissingFeatures.h

bc201b4

mmha force-pushed the cir-casts branch from dec7f63 to bc201b4 Compare March 18, 2025 16:33

andykaylor merged commit 27d8bd3 into llvm:main Mar 18, 2025
6 of 9 checks passed

		@@ -0,0 +1,58 @@
		// RUN: %clang_cc1 -std=c++17 -triple x86_64-unknown-linux-gnu -fclangir -emit-cir %s -o %t.cir

[CIR] Upstream CastOp and scalar conversions #130690

[CIR] Upstream CastOp and scalar conversions #130690

Uh oh!

Conversation

mmha commented Mar 11, 2025

Uh oh!

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

Uh oh!

Uh oh!

erichkeane left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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!

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!

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!

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!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

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

Uh oh!

llvmbot commented Mar 11, 2025 •

edited

Loading

github-actions bot commented Mar 12, 2025 •

edited

Loading