Update on "[CIR][CIRGen][NFC] Return scope result in compound stmt bu…

…ilders"

Instead of returning a boolean indicating whether the statement was
handled, returns the ReturnExpr of the statement if there is one. It
also adds some extra bookkeeping to ensure that the result is returned
when needed. This allows for better support of GCC's `ExprStmt`
extension.

The logical result was not used: it was handled but it would never fail.
Any errors within builders should likely be handled with asserts and
unreachables since they imply a programmer's error in the code.

[ghstack-poisoned]

.github/workflows/pr-code-format.yml

@@ -6,7 +6,7 @@ permissions:
 jobs:
   code_formatter:
     runs-on: ubuntu-latest
-    if: github.repository == 'llvm/llvm-project'
+    if: github.repository == 'llvm/clangir'
     steps:
       - name: Fetch LLVM sources
         uses: actions/checkout@v4

clang/include/clang/CIR/Dialect/IR/CIRDialect.h

@@ -18,9 +18,9 @@
 #include "mlir/IR/BuiltinTypes.h"
 #include "mlir/IR/Dialect.h"
 #include "mlir/IR/OpDefinition.h"
-#include "mlir/Interfaces/FunctionInterfaces.h"
 #include "mlir/Interfaces/CallInterfaces.h"
 #include "mlir/Interfaces/ControlFlowInterfaces.h"
+#include "mlir/Interfaces/FunctionInterfaces.h"
 #include "mlir/Interfaces/InferTypeOpInterface.h"
 #include "mlir/Interfaces/LoopLikeInterface.h"
 #include "mlir/Interfaces/SideEffectInterfaces.h"

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

@@ -582,6 +582,25 @@ def TernaryOp : CIR_Op<"ternary",
   }];
 }
 
+//===----------------------------------------------------------------------===//
+// ConditionOp
+//===----------------------------------------------------------------------===//
+
+def ConditionOp : CIR_Op<"condition", [
+  Terminator,
+  DeclareOpInterfaceMethods<RegionBranchTerminatorOpInterface,
+                            ["getSuccessorRegions"]>
+]> {
+  let summary = "Loop continuation condition.";
+  let description = [{
+    The `cir.condition` termintes loop's conditional regions. It takes a single
+    `cir.bool` operand. if the operand is true, the loop continues, otherwise
+    it terminates.
+  }];
+  let arguments = (ins CIR_BoolType:$condition);
+  let assemblyFormat = " `(` $condition `)` attr-dict ";
+}
+
 //===----------------------------------------------------------------------===//
 // YieldOp
 //===----------------------------------------------------------------------===//

clang/include/clang/CIR/Dialect/IR/CIRTypesDetails.h

@@ -1,4 +1,4 @@
-//===- CIRTypesDetails.h - Details of CIR dialect types -----------*- C++ -*-===//
+//===- CIRTypesDetails.h - Details of CIR dialect types ---------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.

clang/lib/CIR/CodeGen/CIRAsm.cpp

@@ -41,8 +41,8 @@ mlir::LogicalResult CIRGenFunction::buildAsmStmt(const AsmStmt &S) {
 
   AsmDialect AsmDialect = inferDialect(CGM, S);
 
-  builder.create<mlir::cir::InlineAsmOp>(
-      getLoc(S.getAsmLoc()), ResultType, AsmString, AsmDialect);
+  builder.create<mlir::cir::InlineAsmOp>(getLoc(S.getAsmLoc()), ResultType,
+                                         AsmString, AsmDialect);
 
   return mlir::success();
 }

clang/lib/CIR/CodeGen/CIRGenBuilder.h

@@ -578,6 +578,11 @@ class CIRGenBuilderTy : public CIRBaseBuilderTy {
     return create<mlir::cir::CopyOp>(dst.getLoc(), dst, src);
   }
 
+  /// Create a loop condition.
+  mlir::cir::ConditionOp createCondition(mlir::Value condition) {
+    return create<mlir::cir::ConditionOp>(condition.getLoc(), condition);
+  }
+
   mlir::cir::MemCpyOp createMemCpy(mlir::Location loc, mlir::Value dst,
                                    mlir::Value src, mlir::Value len) {
     return create<mlir::cir::MemCpyOp>(loc, dst, src, len);
@@ -787,10 +792,10 @@ class CIRGenBuilderTy : public CIRBaseBuilderTy {
     return create<mlir::cir::StackSaveOp>(loc, ty);
   }
 
-  mlir::cir::StackRestoreOp createStackRestore(mlir::Location loc, mlir::Value v) {
+  mlir::cir::StackRestoreOp createStackRestore(mlir::Location loc,
+                                               mlir::Value v) {
     return create<mlir::cir::StackRestoreOp>(loc, v);
   }
-
 };
 
 } // namespace cir

clang/lib/CIR/CodeGen/CIRGenCoroutine.cpp

@@ -194,10 +194,9 @@ CIRGenFunction::buildCoroAllocBuiltinCall(mlir::Location loc) {
 
   mlir::cir::FuncOp fnOp;
   if (!builtin) {
-    fnOp = CGM.createCIRFunction(
-        loc, CGM.builtinCoroAlloc,
-        mlir::cir::FuncType::get({int32Ty}, boolTy),
-        /*FD=*/nullptr);
+    fnOp = CGM.createCIRFunction(loc, CGM.builtinCoroAlloc,
+                                 mlir::cir::FuncType::get({int32Ty}, boolTy),
+                                 /*FD=*/nullptr);
     assert(fnOp && "should always succeed");
     fnOp.setBuiltinAttr(mlir::UnitAttr::get(builder.getContext()));
   } else
@@ -217,8 +216,7 @@ CIRGenFunction::buildCoroBeginBuiltinCall(mlir::Location loc,
   if (!builtin) {
     fnOp = CGM.createCIRFunction(
         loc, CGM.builtinCoroBegin,
-        mlir::cir::FuncType::get({int32Ty, VoidPtrTy},
-                                 VoidPtrTy),
+        mlir::cir::FuncType::get({int32Ty, VoidPtrTy}, VoidPtrTy),
         /*FD=*/nullptr);
     assert(fnOp && "should always succeed");
     fnOp.setBuiltinAttr(mlir::UnitAttr::get(builder.getContext()));

clang/lib/CIR/CodeGen/CIRGenDecl.cpp

@@ -892,7 +892,7 @@ struct CallCleanupFunction final : EHScopeStack::Cleanup {
 /// Push the standard destructor for the given type as
 /// at least a normal cleanup.
 void CIRGenFunction::pushDestroy(QualType::DestructionKind dtorKind,
-                                  Address addr, QualType type) {
+                                 Address addr, QualType type) {
   assert(dtorKind && "cannot push destructor for trivial type");
 
   CleanupKind cleanupKind = getCleanupKind(dtorKind);

clang/lib/CIR/CodeGen/CIRGenExpr.cpp

@@ -234,8 +234,8 @@ Address CIRGenFunction::getAddrOfBitFieldStorage(LValue base,
 
   auto fieldPtr =
       mlir::cir::PointerType::get(getBuilder().getContext(), fieldType);
-  auto sea = getBuilder().createGetMember(
-    loc, fieldPtr, base.getPointer(), field->getName(), index);
+  auto sea = getBuilder().createGetMember(loc, fieldPtr, base.getPointer(),
+                                          field->getName(), index);
 
   return Address(sea, CharUnits::One());
 }
@@ -341,7 +341,7 @@ LValue CIRGenFunction::buildLValueForField(LValue base,
     if (!IsInPreservedAIRegion &&
         (!getDebugInfo() || !rec->hasAttr<BPFPreserveAccessIndexAttr>())) {
       llvm::StringRef fieldName = field->getName();
-      auto& layout = CGM.getTypes().getCIRGenRecordLayout(field->getParent());
+      auto &layout = CGM.getTypes().getCIRGenRecordLayout(field->getParent());
       unsigned fieldIndex = layout.getCIRFieldNo(field);
 
       if (CGM.LambdaFieldToName.count(field))
@@ -396,7 +396,7 @@ LValue CIRGenFunction::buildLValueForFieldInitialization(
   if (!FieldType->isReferenceType())
     return buildLValueForField(Base, Field);
 
-  auto& layout = CGM.getTypes().getCIRGenRecordLayout(Field->getParent());
+  auto &layout = CGM.getTypes().getCIRGenRecordLayout(Field->getParent());
   unsigned FieldIndex = layout.getCIRFieldNo(Field);
 
   Address V = buildAddrOfFieldStorage(*this, Base.getAddress(), Field,

clang/lib/CIR/CodeGen/CIRGenExprAgg.cpp

@@ -60,37 +60,36 @@ static bool isBlockVarRef(const Expr *E) {
     // FIXME: pointer arithmetic?
     return false;
 
-  // Check both sides of a conditional operator.
-  } else if (const AbstractConditionalOperator *op
-               = dyn_cast<AbstractConditionalOperator>(E)) {
-    return isBlockVarRef(op->getTrueExpr())
-        || isBlockVarRef(op->getFalseExpr());
-
-  // OVEs are required to support BinaryConditionalOperators.
-  } else if (const OpaqueValueExpr *op
-               = dyn_cast<OpaqueValueExpr>(E)) {
+    // Check both sides of a conditional operator.
+  } else if (const AbstractConditionalOperator *op =
+                 dyn_cast<AbstractConditionalOperator>(E)) {
+    return isBlockVarRef(op->getTrueExpr()) ||
+           isBlockVarRef(op->getFalseExpr());
+
+    // OVEs are required to support BinaryConditionalOperators.
+  } else if (const OpaqueValueExpr *op = dyn_cast<OpaqueValueExpr>(E)) {
     if (const Expr *src = op->getSourceExpr())
       return isBlockVarRef(src);
 
-  // Casts are necessary to get things like (*(int*)&var) = foo().
-  // We don't really care about the kind of cast here, except
-  // we don't want to look through l2r casts, because it's okay
-  // to get the *value* in a __block variable.
+    // Casts are necessary to get things like (*(int*)&var) = foo().
+    // We don't really care about the kind of cast here, except
+    // we don't want to look through l2r casts, because it's okay
+    // to get the *value* in a __block variable.
   } else if (const CastExpr *cast = dyn_cast<CastExpr>(E)) {
     if (cast->getCastKind() == CK_LValueToRValue)
       return false;
     return isBlockVarRef(cast->getSubExpr());
 
-  // Handle unary operators.  Again, just aggressively look through
-  // it, ignoring the operation.
+    // Handle unary operators.  Again, just aggressively look through
+    // it, ignoring the operation.
   } else if (const UnaryOperator *uop = dyn_cast<UnaryOperator>(E)) {
     return isBlockVarRef(uop->getSubExpr());
 
-  // Look into the base of a field access.
+    // Look into the base of a field access.
   } else if (const MemberExpr *mem = dyn_cast<MemberExpr>(E)) {
     return isBlockVarRef(mem->getBase());
 
-  // Look into the base of a subscript.
+    // Look into the base of a subscript.
   } else if (const ArraySubscriptExpr *sub = dyn_cast<ArraySubscriptExpr>(E)) {
     return isBlockVarRef(sub->getBase());
   }
@@ -113,7 +112,8 @@ class AggExprEmitter : public StmtVisitor<AggExprEmitter> {
                            llvm::function_ref<RValue(ReturnValueSlot)> Fn);
 
   AggValueSlot EnsureSlot(mlir::Location loc, QualType T) {
-    if (!Dest.isIgnored()) return Dest;
+    if (!Dest.isIgnored())
+      return Dest;
     return CGF.CreateAggTemp(T, loc, "agg.tmp.ensured");
   }
 
@@ -213,11 +213,11 @@ class AggExprEmitter : public StmtVisitor<AggExprEmitter> {
     // For an assignment to work, the value on the right has
     // to be compatible with the value on the left.
     assert(CGF.getContext().hasSameUnqualifiedType(E->getLHS()->getType(),
-                                                   E->getRHS()->getType())
-         && "Invalid assignment");
+                                                   E->getRHS()->getType()) &&
+           "Invalid assignment");
 
     if (isBlockVarRef(E->getLHS()) &&
-      E->getRHS()->HasSideEffects(CGF.getContext())) {
+        E->getRHS()->HasSideEffects(CGF.getContext())) {
       llvm_unreachable("NYI");
     }
 
@@ -233,12 +233,11 @@ class AggExprEmitter : public StmtVisitor<AggExprEmitter> {
 
     // Codegen the RHS so that it stores directly into the LHS.
     AggValueSlot lhsSlot = AggValueSlot::forLValue(
-      lhs, AggValueSlot::IsDestructed, AggValueSlot::DoesNotNeedGCBarriers,
-      AggValueSlot::IsAliased, AggValueSlot::MayOverlap);
+        lhs, AggValueSlot::IsDestructed, AggValueSlot::DoesNotNeedGCBarriers,
+        AggValueSlot::IsAliased, AggValueSlot::MayOverlap);
 
     // A non-volatile aggregate destination might have volatile member.
-    if (!lhsSlot.isVolatile() &&
-        CGF.hasVolatileMember(E->getLHS()->getType()))
+    if (!lhsSlot.isVolatile() && CGF.hasVolatileMember(E->getLHS()->getType()))
       assert(!UnimplementedFeature::atomicTypes());
 
     CGF.buildAggExpr(E->getRHS(), lhsSlot);
@@ -247,10 +246,10 @@ class AggExprEmitter : public StmtVisitor<AggExprEmitter> {
     buildFinalDestCopy(E->getType(), lhs);
 
     if (!Dest.isIgnored() && !Dest.isExternallyDestructed() &&
-      E->getType().isDestructedType() == QualType::DK_nontrivial_c_struct)
+        E->getType().isDestructedType() == QualType::DK_nontrivial_c_struct)
       CGF.pushDestroy(QualType::DK_nontrivial_c_struct, Dest.getAddress(),
-                     E->getType());
-   }
+                      E->getType());
+  }
 
   void VisitBinComma(const BinaryOperator *E) { llvm_unreachable("NYI"); }
   void VisitBinCmp(const BinaryOperator *E) { llvm_unreachable("NYI"); }
@@ -356,8 +355,8 @@ void AggExprEmitter::buildFinalDestCopy(QualType type, const LValue &src,
     assert(!UnimplementedFeature::volatileTypes());
 
   if (SrcValueKind == EVK_RValue) {
-     if (type.isNonTrivialToPrimitiveDestructiveMove() == QualType::PCK_Struct) {
-        llvm_unreachable("move assignment/move ctor for rvalue is NYI");
+    if (type.isNonTrivialToPrimitiveDestructiveMove() == QualType::PCK_Struct) {
+      llvm_unreachable("move assignment/move ctor for rvalue is NYI");
     }
   } else {
     if (type.isNonTrivialToPrimitiveCopy() == QualType::PCK_Struct)
@@ -672,8 +671,8 @@ void AggExprEmitter::VisitLambdaExpr(LambdaExpr *E) {
     }
 
     // Emit initialization
-    LValue LV = CGF.buildLValueForFieldInitialization(
-        SlotLV, *CurField, fieldName);
+    LValue LV =
+        CGF.buildLValueForFieldInitialization(SlotLV, *CurField, fieldName);
     if (CurField->hasCapturedVLAType()) {
       llvm_unreachable("NYI");
     }
@@ -820,8 +819,8 @@ void AggExprEmitter::withReturnValueSlot(
   if (!UseTemp) {
     RetAddr = Dest.getAddress();
   } else {
-    RetAddr = CGF.CreateMemTemp(RetTy, CGF.getLoc(E->getSourceRange()),
-                                "tmp", &RetAddr);
+    RetAddr = CGF.CreateMemTemp(RetTy, CGF.getLoc(E->getSourceRange()), "tmp",
+                                &RetAddr);
     assert(!UnimplementedFeature::shouldEmitLifetimeMarkers() && "NYI");
   }
 
@@ -940,8 +939,8 @@ void AggExprEmitter::VisitCXXParenListOrInitListExpr(
     if (curInitIndex == NumInitElements && Dest.isZeroed() &&
         CGF.getTypes().isZeroInitializable(ExprToVisit->getType()))
       break;
-    LValue LV = CGF.buildLValueForFieldInitialization(
-        DestLV, field, field->getName());
+    LValue LV =
+        CGF.buildLValueForFieldInitialization(DestLV, field, field->getName());
     // We never generate write-barries for initialized fields.
     assert(!UnimplementedFeature::setNonGC());
 

clang/lib/CIR/CodeGen/CIRGenExprConst.cpp

@@ -907,7 +907,7 @@ class ConstExprEmitter
       // Look through the temporary; it's just converting the value to an lvalue
       // to pass it to the constructor.
       if (auto *MTE = dyn_cast<MaterializeTemporaryExpr>(Arg))
-          return Visit(MTE->getSubExpr(), Ty);
+        return Visit(MTE->getSubExpr(), Ty);
       // Don't try to support arbitrary lvalue-to-rvalue conversions for now.
       return nullptr;
     }
@@ -1074,8 +1074,7 @@ class ConstantLValueEmitter
   ConstantLValue applyOffset(ConstantLValue &C) {
 
     // Handle attribute constant LValues.
-    if (auto Attr = 
-    C.Value.dyn_cast<mlir::Attribute>()) {
+    if (auto Attr = C.Value.dyn_cast<mlir::Attribute>()) {
       if (auto GV = Attr.dyn_cast<mlir::cir::GlobalViewAttr>()) {
         auto baseTy = GV.getType().cast<mlir::cir::PointerType>().getPointee();
         auto destTy = CGM.getTypes().convertTypeForMem(DestType);
@@ -1338,7 +1337,7 @@ mlir::Attribute ConstantEmitter::tryEmitPrivateForVarInit(const VarDecl &D) {
   }
   InConstantContext = D.hasConstantInitialization();
 
-  const Expr * E = D.getInit();
+  const Expr *E = D.getInit();
   assert(E && "No initializer to emit");
 
   QualType destType = D.getType();

clang/lib/CIR/CodeGen/CIRGenFunction.h

@@ -771,7 +771,7 @@ class CIRGenFunction : public CIRGenTypeCache {
   /// Create a check for a function parameter that may potentially be
   /// declared as non-null.
   void buildNonNullArgCheck(RValue RV, QualType ArgType, SourceLocation ArgLoc,
-                           AbstractCallee AC, unsigned ParmNum);
+                            AbstractCallee AC, unsigned ParmNum);
 
   void buildCallArg(CallArgList &args, const clang::Expr *E,
                     clang::QualType ArgType);
@@ -1364,7 +1364,7 @@ class CIRGenFunction : public CIRGenTypeCache {
   AggValueSlot::Overlap_t getOverlapForFieldInit(const FieldDecl *FD);
   LValue buildLValueForField(LValue Base, const clang::FieldDecl *Field);
   LValue buildLValueForBitField(LValue base, const FieldDecl *field);
-  
+
   /// Like buildLValueForField, excpet that if the Field is a reference, this
   /// will return the address of the reference and not the address of the value
   /// stored in the reference.
@@ -1522,8 +1522,8 @@ class CIRGenFunction : public CIRGenTypeCache {
 
   static Destroyer destroyCXXObject;
 
-  void pushDestroy(QualType::DestructionKind dtorKind,
-                   Address addr, QualType type);
+  void pushDestroy(QualType::DestructionKind dtorKind, Address addr,
+                   QualType type);
 
   void pushDestroy(CleanupKind kind, Address addr, QualType type,
                    Destroyer *destroyer, bool useEHCleanupForArray);

clang/lib/CIR/CodeGen/CIRGenModule.cpp

@@ -100,10 +100,10 @@ CIRGenModule::CIRGenModule(mlir::MLIRContext &context,
                            const clang::CodeGenOptions &CGO,
                            DiagnosticsEngine &Diags)
     : builder(context, *this), astCtx(astctx), langOpts(astctx.getLangOpts()),
-      codeGenOpts(CGO), theModule{mlir::ModuleOp::create(
-                            builder.getUnknownLoc())},
-      Diags(Diags), target(astCtx.getTargetInfo()),
-      ABI(createCXXABI(*this)), genTypes{*this}, VTables{*this} {
+      codeGenOpts(CGO),
+      theModule{mlir::ModuleOp::create(builder.getUnknownLoc())}, Diags(Diags),
+      target(astCtx.getTargetInfo()), ABI(createCXXABI(*this)), genTypes{*this},
+      VTables{*this} {
 
   // Initialize CIR signed integer types cache.
   SInt8Ty =