[CIR][ABI] Apply CC lowering pass by default (#842)

Before this patch, the CC lowering pass was applied only when explicitly
requested by the user. This update changes the default behavior to
always apply the CC lowering pass, with an option to disable it using
the `-fno-clangir-call-conv-lowering` flag if necessary.

The primary objective is to make this pass a mandatory step in the
compilation pipeline. This ensures that future contributions correctly
implement the CC lowering for both existing and new targets, resulting
in more consistent and accurate code generation.

From an implementation perspective, several `llvm_unreachable`
statements have been substituted with a new `assert_or_abort` macro.
This macro can be configured to either trigger a non-blocking assertion
or a blocking unreachable statement. This facilitates a test-by-testa
incremental development as it does not required you to know which code
path a test will trigger an just cause a crash if it does.

A few notable changes:

 - Support multi-block function in CC lowering
 - Ignore pointer-related CC lowering
 - Ignore no-proto functions CC lowering
 - Handle missing type evaluation kinds
 - Fix CC lowering for function declarations
 - Unblock indirect function calls
 - Disable CC lowering pass on several tests

Author: sitio-couto

clang/include/clang/CIR/Dialect/Passes.h

@@ -42,7 +42,7 @@ std::unique_ptr<Pass> createGotoSolverPass();
 /// Create a pass to lower ABI-independent function definitions/calls.
 std::unique_ptr<Pass> createCallConvLoweringPass();
 
-void populateCIRPreLoweringPasses(mlir::OpPassManager &pm);
+void populateCIRPreLoweringPasses(mlir::OpPassManager &pm, bool useCCLowering);
 
 //===----------------------------------------------------------------------===//
 // Registration

clang/include/clang/CIR/MissingFeatures.h

@@ -15,6 +15,27 @@
 #ifndef CLANG_CIR_MISSINGFEATURES_H
 #define CLANG_CIR_MISSINGFEATURES_H
 
+constexpr bool cirMissingFeatureAssertionMode =
+    true; // Change to `false` to use llvm_unreachable
+
+#define NOTE                                                                   \
+  " Target lowering is now required. Disable it with "                         \
+  "-fno-clangir-call-conv-lowering."
+
+// Special assertion to be used in the target lowering library.
+#define cir_tl_assert(cond) assert((cond) && NOTE);
+
+// Some assertions knowingly generate incorrect code. This macro allows us to
+// switch between using `assert` and `llvm_unreachable` for these cases.
+#define cir_assert_or_abort(cond, msg)                                         \
+  do {                                                                         \
+    if (cirMissingFeatureAssertionMode) {                                      \
+      assert((cond) && msg NOTE);                                              \
+    } else {                                                                   \
+      llvm_unreachable(msg NOTE);                                              \
+    }                                                                          \
+  } while (0)
+
 namespace cir {
 
 struct MissingFeatures {
@@ -212,6 +233,26 @@ struct MissingFeatures {
 
   //===--- ABI lowering --===//
 
+  static bool SPIRVABI() { return false; }
+
+  static bool AArch64TypeClassification() { return false; }
+
+  static bool X86ArgTypeClassification() { return false; }
+  static bool X86DefaultABITypeConvertion() { return false; }
+  static bool X86GetFPTypeAtOffset() { return false; }
+  static bool X86RetTypeClassification() { return false; }
+  static bool X86TypeClassification() { return false; }
+
+  static bool ABIClangTypeKind() { return false; }
+  static bool ABIEnterStructForCoercedAccess() { return false; }
+  static bool ABIFuncPtr() { return false; }
+  static bool ABIInRegAttribute() { return false; }
+  static bool ABINestedRecordLayout() { return false; }
+  static bool ABINoProtoFunctions() { return false; }
+  static bool ABIParameterCoercion() { return false; }
+  static bool ABIPointerParameterAttrs() { return false; }
+  static bool ABITransparentUnionHandling() { return false; }
+
   //-- Missing AST queries
 
   static bool CXXRecordDeclIsEmptyCXX11() { return false; }

clang/include/clang/Driver/Options.td

@@ -2978,10 +2978,6 @@ def fclangir_lib_opt : Flag<["-"], "fclangir-lib-opt">,
   Visibility<[ClangOption, CC1Option]>, Group<f_Group>,
   Alias<fclangir_lib_opt_EQ>,
   HelpText<"Enable C/C++ library based optimizations">;
-def fclangir_call_conv_lowering : Flag<["-"], "fclangir-call-conv-lowering">,
-  Visibility<[ClangOption, CC1Option]>, Group<f_Group>,
-  HelpText<"Enable ClangIR calling convention lowering">,
-  MarshallingInfoFlag<FrontendOpts<"ClangIREnableCallConvLowering">>;
 def fclangir_mem2reg : Flag<["-"], "fclangir-mem2reg">,
   Visibility<[ClangOption, CC1Option]>, Group<f_Group>,
   HelpText<"Enable mem2reg on the flat ClangIR">,
@@ -3012,6 +3008,13 @@ defm clangir_analysis_only : BoolFOption<"clangir-analysis-only",
   PosFlag<SetTrue, [], [ClangOption, CC1Option],
     "Enable CIR analysis but keep traditional LLVM codegen (not through CIR)">,
   NegFlag<SetFalse, [], [ClangOption, CC1Option],  "">>;
+// FIXME(cir): Remove this option once all pre-existing tests are compatible with
+// the calling convention lowering pass.
+defm clangir_call_conv_lowering : BoolFOption<"clangir-call-conv-lowering",
+  FrontendOpts<"ClangIRCallConvLowering">, DefaultTrue,
+  PosFlag<SetTrue, [], [ClangOption, CC1Option], "Transform CIR to abide to calling convetions during lowering">,
+  NegFlag<SetFalse, [], [ClangOption, CC1Option], "Ignore calling convetion during lowering">,
+  BothFlags<[], [ClangOption, CC1Option], "">>;
 
 def emit_cir : Flag<["-"], "emit-cir">, Visibility<[CC1Option]>,
   Group<Action_Group>, HelpText<"Build ASTs and then lower to ClangIR, emit the .cir file">;

clang/include/clang/Frontend/FrontendOptions.h

@@ -445,7 +445,7 @@ class FrontendOptions {
   unsigned ClangIRLibOpt : 1;
 
   // Enable Clang IR call conv lowering pass.
-  unsigned ClangIREnableCallConvLowering : 1;
+  unsigned ClangIRCallConvLowering : 1;
 
   // Enable Clang IR mem2reg pass on the flat CIR.
   unsigned ClangIREnableMem2Reg : 1;

clang/lib/CIR/CodeGen/CIRPasses.cpp

@@ -71,13 +71,8 @@ mlir::LogicalResult runCIRToCIRPasses(
 
   pm.addPass(mlir::createLoweringPreparePass(&astCtx));
 
-  // FIXME(cir): This pass should run by default, but it is lacking support for
-  // several code bits. Once it's more mature, we should fix this.
-  if (enableCallConvLowering)
-    pm.addPass(mlir::createCallConvLoweringPass());
-
   if (flattenCIR || enableMem2Reg)
-    mlir::populateCIRPreLoweringPasses(pm);
+    mlir::populateCIRPreLoweringPasses(pm, enableCallConvLowering);
 
   if (enableMem2Reg)
     pm.addPass(mlir::createMem2Reg());
@@ -97,7 +92,9 @@ mlir::LogicalResult runCIRToCIRPasses(
 
 namespace mlir {
 
-void populateCIRPreLoweringPasses(OpPassManager &pm) {
+void populateCIRPreLoweringPasses(OpPassManager &pm, bool useCCLowering) {
+  if (useCCLowering)
+    pm.addPass(createCallConvLoweringPass());
   pm.addPass(createFlattenCFGPass());
   pm.addPass(createGotoSolverPass());
 }

clang/lib/CIR/Dialect/Transforms/CallConvLowering.cpp

@@ -6,14 +6,14 @@
 //
 //===----------------------------------------------------------------------===//
 
-
 #include "TargetLowering/LowerModule.h"
 #include "mlir/Dialect/LLVMIR/LLVMDialect.h"
 #include "mlir/IR/BuiltinOps.h"
 #include "mlir/IR/PatternMatch.h"
 #include "mlir/Pass/Pass.h"
 #include "mlir/Transforms/GreedyPatternRewriteDriver.h"
 #include "clang/CIR/Dialect/IR/CIRDialect.h"
+#include "clang/CIR/MissingFeatures.h"
 
 #define GEN_PASS_DEF_CALLCONVLOWERING
 #include "clang/CIR/Dialect/Passes.h.inc"
@@ -44,6 +44,12 @@ struct CallConvLoweringPattern : public OpRewritePattern<FuncOp> {
     auto calls = op.getSymbolUses(module);
     if (calls.has_value()) {
       for (auto call : calls.value()) {
+        // FIXME(cir): Function pointers are ignored.
+        if (isa<GetGlobalOp>(call.getUser())) {
+          cir_assert_or_abort(!::cir::MissingFeatures::ABIFuncPtr(), "NYI");
+          continue;
+        }
+
         auto callOp = cast<CallOp>(call.getUser());
         if (lowerModule->rewriteFunctionCall(callOp, op).failed())
           return failure();

clang/lib/CIR/Dialect/Transforms/TargetLowering/ABIInfo.cpp

@@ -37,7 +37,7 @@ bool ABIInfo::isPromotableIntegerTypeForABI(Type Ty) const {
   if (getContext().isPromotableIntegerType(Ty))
     return true;
 
-  assert(!::cir::MissingFeatures::fixedWidthIntegers());
+  cir_tl_assert(!::cir::MissingFeatures::fixedWidthIntegers());
 
   return false;
 }

clang/lib/CIR/Dialect/Transforms/TargetLowering/ABIInfoImpl.cpp

@@ -26,21 +26,22 @@ bool classifyReturnType(const CIRCXXABI &CXXABI, LowerFunctionInfo &FI,
   Type Ty = FI.getReturnType();
 
   if (const auto RT = dyn_cast<StructType>(Ty)) {
-    assert(!::cir::MissingFeatures::isCXXRecordDecl());
+    cir_tl_assert(!::cir::MissingFeatures::isCXXRecordDecl());
   }
 
   return CXXABI.classifyReturnType(FI);
 }
 
 bool isAggregateTypeForABI(Type T) {
-  assert(!::cir::MissingFeatures::functionMemberPointerType());
+  cir_tl_assert(!::cir::MissingFeatures::functionMemberPointerType());
   return !LowerFunction::hasScalarEvaluationKind(T);
 }
 
 Type useFirstFieldIfTransparentUnion(Type Ty) {
   if (auto RT = dyn_cast<StructType>(Ty)) {
     if (RT.isUnion())
-      llvm_unreachable("NYI");
+      cir_assert_or_abort(
+          !::cir::MissingFeatures::ABITransparentUnionHandling(), "NYI");
   }
   return Ty;
 }

clang/lib/CIR/Dialect/Transforms/TargetLowering/CIRLowerContext.cpp

@@ -55,7 +55,10 @@ clang::TypeInfo CIRLowerContext::getTypeInfoImpl(const Type T) const {
   } else if (isa<StructType>(T)) {
     typeKind = clang::Type::Record;
   } else {
-    llvm_unreachable("Unhandled type class");
+    cir_assert_or_abort(!::cir::MissingFeatures::ABIClangTypeKind(),
+                        "Unhandled type class");
+    // FIXME(cir): Completely wrong. Just here to make it non-blocking.
+    typeKind = clang::Type::Builtin;
   }
 
   // FIXME(cir): Here we fetch the width and alignment of a type considering the
@@ -96,10 +99,10 @@ clang::TypeInfo CIRLowerContext::getTypeInfoImpl(const Type T) const {
   }
   case clang::Type::Record: {
     const auto RT = dyn_cast<StructType>(T);
-    assert(!::cir::MissingFeatures::tagTypeClassAbstraction());
+    cir_tl_assert(!::cir::MissingFeatures::tagTypeClassAbstraction());
 
     // Only handle TagTypes (names types) for now.
-    assert(RT.getName() && "Anonymous record is NYI");
+    cir_tl_assert(RT.getName() && "Anonymous record is NYI");
 
     // NOTE(cir): Clang does some hanlding of invalid tagged declarations here.
     // Not sure if this is necessary in CIR.
@@ -111,22 +114,22 @@ clang::TypeInfo CIRLowerContext::getTypeInfoImpl(const Type T) const {
     const CIRRecordLayout &Layout = getCIRRecordLayout(RT);
     Width = toBits(Layout.getSize());
     Align = toBits(Layout.getAlignment());
-    assert(!::cir::MissingFeatures::recordDeclHasAlignmentAttr());
+    cir_tl_assert(!::cir::MissingFeatures::recordDeclHasAlignmentAttr());
     break;
   }
   default:
     llvm_unreachable("Unhandled type class");
   }
 
-  assert(llvm::isPowerOf2_32(Align) && "Alignment must be power of 2");
+  cir_tl_assert(llvm::isPowerOf2_32(Align) && "Alignment must be power of 2");
   return clang::TypeInfo(Width, Align, AlignRequirement);
 }
 
 Type CIRLowerContext::initBuiltinType(clang::BuiltinType::Kind K) {
   Type Ty;
 
   // NOTE(cir): Clang does more stuff here. Not sure if we need to do the same.
-  assert(!::cir::MissingFeatures::qualifiedTypes());
+  cir_tl_assert(!::cir::MissingFeatures::qualifiedTypes());
   switch (K) {
   case clang::BuiltinType::Char_S:
     Ty = IntType::get(getMLIRContext(), 8, true);
@@ -141,7 +144,7 @@ Type CIRLowerContext::initBuiltinType(clang::BuiltinType::Kind K) {
 
 void CIRLowerContext::initBuiltinTypes(const clang::TargetInfo &Target,
                                        const clang::TargetInfo *AuxTarget) {
-  assert((!this->Target || this->Target == &Target) &&
+  cir_tl_assert((!this->Target || this->Target == &Target) &&
          "Incorrect target reinitialization");
   this->Target = &Target;
   this->AuxTarget = AuxTarget;

clang/lib/CIR/Dialect/Transforms/TargetLowering/CIRRecordLayout.cpp

@@ -38,16 +38,16 @@ CIRRecordLayout::CIRRecordLayout(
   FieldOffsets.insert(FieldOffsets.end(), fieldoffsets.begin(),
                       fieldoffsets.end());
 
-  assert(!PrimaryBase && "Layout for class with inheritance is NYI");
+  cir_tl_assert(!PrimaryBase && "Layout for class with inheritance is NYI");
   // CXXInfo->PrimaryBase.setPointer(PrimaryBase);
-  assert(!IsPrimaryBaseVirtual && "Layout for virtual base class is NYI");
+  cir_tl_assert(!IsPrimaryBaseVirtual && "Layout for virtual base class is NYI");
   // CXXInfo->PrimaryBase.setInt(IsPrimaryBaseVirtual);
   CXXInfo->NonVirtualSize = nonvirtualsize;
   CXXInfo->NonVirtualAlignment = nonvirtualalignment;
   CXXInfo->PreferredNVAlignment = preferrednvalignment;
   CXXInfo->SizeOfLargestEmptySubobject = SizeOfLargestEmptySubobject;
   // FIXME(cir): Initialize base classes offsets.
-  assert(!::cir::MissingFeatures::getCXXRecordBases());
+  cir_tl_assert(!::cir::MissingFeatures::getCXXRecordBases());
   CXXInfo->HasOwnVFPtr = hasOwnVFPtr;
   CXXInfo->VBPtrOffset = vbptroffset;
   CXXInfo->HasExtendableVFPtr = hasExtendableVFPtr;

clang/lib/CIR/Dialect/Transforms/TargetLowering/CIRToCIRArgMapping.h

@@ -58,7 +58,7 @@ class CIRToCIRArgMapping {
   unsigned totalIRArgs() const { return TotalIRArgs; }
 
   bool hasPaddingArg(unsigned ArgNo) const {
-    assert(ArgNo < ArgInfo.size());
+    cir_tl_assert(ArgNo < ArgInfo.size());
     return ArgInfo[ArgNo].PaddingArgIndex != InvalidIndex;
   }
 
@@ -77,7 +77,7 @@ class CIRToCIRArgMapping {
         onlyRequiredArgs ? FI.getNumRequiredArgs() : FI.arg_size();
     for (LowerFunctionInfo::const_arg_iterator I = FI.arg_begin();
          ArgNo < NumArgs; ++I, ++ArgNo) {
-      assert(I != FI.arg_end());
+      cir_tl_assert(I != FI.arg_end());
       // Type ArgType = I->type;
       const ::cir::ABIArgInfo &AI = I->info;
       // Collect data about IR arguments corresponding to Clang argument ArgNo.
@@ -91,7 +91,7 @@ class CIRToCIRArgMapping {
       case ::cir::ABIArgInfo::Extend:
       case ::cir::ABIArgInfo::Direct: {
         // FIXME(cir): handle sseregparm someday...
-        assert(AI.getCoerceToType() && "Missing coerced type!!");
+        cir_tl_assert(AI.getCoerceToType() && "Missing coerced type!!");
         StructType STy = dyn_cast<StructType>(AI.getCoerceToType());
         if (AI.isDirect() && AI.getCanBeFlattened() && STy) {
           llvm_unreachable("NYI");
@@ -114,7 +114,7 @@ class CIRToCIRArgMapping {
       if (IRArgNo == 1 && SwapThisWithSRet)
         IRArgNo++;
     }
-    assert(ArgNo == ArgInfo.size());
+    cir_tl_assert(ArgNo == ArgInfo.size());
 
     if (::cir::MissingFeatures::inallocaArgs()) {
       llvm_unreachable("NYI");
@@ -126,7 +126,7 @@ class CIRToCIRArgMapping {
   /// Returns index of first IR argument corresponding to ArgNo, and their
   /// quantity.
   std::pair<unsigned, unsigned> getIRArgs(unsigned ArgNo) const {
-    assert(ArgNo < ArgInfo.size());
+    cir_tl_assert(ArgNo < ArgInfo.size());
     return std::make_pair(ArgInfo[ArgNo].FirstArgIndex,
                           ArgInfo[ArgNo].NumberOfArgs);
   }

clang/lib/CIR/Dialect/Transforms/TargetLowering/ItaniumCXXABI.cpp

@@ -46,9 +46,9 @@ class ItaniumCXXABI : public CIRCXXABI {
 
   // FIXME(cir): This expects a CXXRecordDecl! Not any record type.
   RecordArgABI getRecordArgABI(const StructType RD) const override {
-    assert(!::cir::MissingFeatures::recordDeclIsCXXDecl());
+    cir_tl_assert(!::cir::MissingFeatures::recordDeclIsCXXDecl());
     // If C++ prohibits us from making a copy, pass by address.
-    assert(!::cir::MissingFeatures::recordDeclCanPassInRegisters());
+    cir_tl_assert(!::cir::MissingFeatures::recordDeclCanPassInRegisters());
     return RAA_Default;
   }
 };
@@ -76,7 +76,7 @@ CIRCXXABI *CreateItaniumCXXABI(LowerModule &LM) {
   case clang::TargetCXXABI::AppleARM64:
     // TODO: this isn't quite right, clang uses AppleARM64CXXABI which inherits
     // from ARMCXXABI. We'll have to follow suit.
-    assert(!::cir::MissingFeatures::appleArm64CXXABI());
+    cir_tl_assert(!::cir::MissingFeatures::appleArm64CXXABI());
     return new ItaniumCXXABI(LM, /*UseARMMethodPtrABI=*/true,
                              /*UseARMGuardVarABI=*/true);