Update on "[CIR] Add reconcile unrealized casts pass"

[ghstack-poisoned]

Author: sitio-couto

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

@@ -1377,6 +1377,9 @@ def StructElementAddr : CIR_Op<"struct_element_addr"> {
     The `cir.struct_element_addr` operaration gets the address of a particular
     named member from the input struct.
 
+    It expects a pointer to the base struct as well as the name of the member
+    and its field index.
+
     Example:
     ```mlir
     !ty_22struct2EBar22 = type !cir.struct<"struct.Bar", i32, i8>
@@ -1391,10 +1394,25 @@ def StructElementAddr : CIR_Op<"struct_element_addr"> {
 
   let arguments = (ins
     Arg<CIR_PointerType, "the address to load from", [MemRead]>:$struct_addr,
-    StrAttr:$member_name);
+    StrAttr:$member_name,
+    IndexAttr:$member_index);
 
   let results = (outs Res<CIR_PointerType, "">:$result);
 
+  let builders = [
+    OpBuilder<(ins "Type":$type, "Value":$value, "llvm::StringRef":$name,
+              "unsigned":$index),
+    [{
+      mlir::APInt fieldIdx(64, index);
+      build($_builder, $_state, type, value, name, fieldIdx);
+    }]>
+  ];
+
+  let extraClassDeclaration = [{
+    /// Return the index of the struct member being accessed.
+    uint64_t getIndex() { return getMemberIndex().getZExtValue(); }
+  }];
+
   // FIXME: add verifier.
 }
 

clang/lib/CIR/CodeGen/CIRGenClass.cpp

@@ -207,7 +207,8 @@ static void buildLValueForAnyFieldInitialization(CIRGenFunction &CGF,
   if (MemberInit->isIndirectMemberInitializer()) {
     llvm_unreachable("NYI");
   } else {
-    LHS = CGF.buildLValueForFieldInitialization(LHS, Field, Field->getName());
+    LHS = CGF.buildLValueForFieldInitialization(LHS, Field, Field->getName(),
+                                                Field->getFieldIndex());
   }
 }
 

clang/lib/CIR/CodeGen/CIRGenExpr.cpp

@@ -16,13 +16,15 @@
 #include "CIRGenCstEmitter.h"
 #include "CIRGenFunction.h"
 #include "CIRGenModule.h"
+#include "CIRGenValue.h"
 #include "UnimplementedFeatureGuarding.h"
 
 #include "clang/AST/GlobalDecl.h"
 #include "clang/Basic/Builtins.h"
 #include "clang/CIR/Dialect/IR/CIRDialect.h"
 #include "clang/CIR/Dialect/IR/CIRTypes.h"
 #include "llvm/Support/Casting.h"
+#include "llvm/Support/ErrorHandling.h"
 
 #include "mlir/Dialect/Func/IR/FuncOps.h"
 #include "mlir/IR/Value.h"
@@ -50,7 +52,8 @@ static Address buildPreserveStructAccess(CIRGenFunction &CGF, LValue base,
 /// doesn't necessarily have the right type.
 static Address buildAddrOfFieldStorage(CIRGenFunction &CGF, Address Base,
                                        const FieldDecl *field,
-                                       llvm::StringRef fieldName) {
+                                       llvm::StringRef fieldName,
+                                       unsigned fieldIndex) {
   if (field->isZeroSize(CGF.getContext()))
     llvm_unreachable("NYI");
 
@@ -63,7 +66,7 @@ static Address buildAddrOfFieldStorage(CIRGenFunction &CGF, Address Base,
   // which do not currently carry the name, so it can be passed down from the
   // CaptureStmt.
   auto sea = CGF.getBuilder().create<mlir::cir::StructElementAddr>(
-      loc, fieldPtr, Base.getPointer(), fieldName);
+      loc, fieldPtr, Base.getPointer(), fieldName, fieldIndex);
 
   // TODO: We could get the alignment from the CIRGenRecordLayout, but given the
   // member name based lookup of the member here we probably shouldn't be. We'll
@@ -235,9 +238,10 @@ LValue CIRGenFunction::buildLValueForField(LValue base,
     if (!IsInPreservedAIRegion &&
         (!getDebugInfo() || !rec->hasAttr<BPFPreserveAccessIndexAttr>())) {
       llvm::StringRef fieldName = field->getName();
+      unsigned fieldIndex = field->getFieldIndex();
       if (CGM.LambdaFieldToName.count(field))
         fieldName = CGM.LambdaFieldToName[field];
-      addr = buildAddrOfFieldStorage(*this, addr, field, fieldName);
+      addr = buildAddrOfFieldStorage(*this, addr, field, fieldName, fieldIndex);
     } else
       // Remember the original struct field index
       addr = buildPreserveStructAccess(*this, base, addr, field);
@@ -281,14 +285,15 @@ LValue CIRGenFunction::buildLValueForField(LValue base,
 }
 
 LValue CIRGenFunction::buildLValueForFieldInitialization(
-    LValue Base, const clang::FieldDecl *Field, llvm::StringRef FieldName) {
+    LValue Base, const clang::FieldDecl *Field, llvm::StringRef FieldName,
+    unsigned FieldIndex) {
   QualType FieldType = Field->getType();
 
   if (!FieldType->isReferenceType())
     return buildLValueForField(Base, Field);
 
-  Address V =
-      buildAddrOfFieldStorage(*this, Base.getAddress(), Field, FieldName);
+  Address V = buildAddrOfFieldStorage(*this, Base.getAddress(), Field,
+                                      FieldName, FieldIndex);
 
   // Make sure that the address is pointing to the right type.
   auto memTy = getTypes().convertTypeForMem(FieldType);
@@ -577,9 +582,9 @@ LValue CIRGenFunction::buildDeclRefLValue(const DeclRefExpr *E) {
 
   if (const auto *VD = dyn_cast<VarDecl>(ND)) {
     // Global Named registers access via intrinsics only
-    if (VD->getStorageClass() == SC_Register &&
-        VD->hasAttr<AsmLabelAttr>() && !VD->isLocalVarDecl())
-        llvm_unreachable("NYI");
+    if (VD->getStorageClass() == SC_Register && VD->hasAttr<AsmLabelAttr>() &&
+        !VD->isLocalVarDecl())
+      llvm_unreachable("NYI");
 
     assert(E->isNonOdrUse() != NOUR_Constant && "not implemented");
 
@@ -1172,12 +1177,11 @@ LValue CIRGenFunction::buildArraySubscriptExpr(const ArraySubscriptExpr *E,
                                                bool Accessed) {
   // The index must always be an integer, which is not an aggregate.  Emit it
   // in lexical order (this complexity is, sadly, required by C++17).
-  // llvm::Value *IdxPre =
-  //     (E->getLHS() == E->getIdx()) ? EmitScalarExpr(E->getIdx()) : nullptr;
-  assert(E->getLHS() != E->getIdx() && "not implemented");
+  mlir::Value IdxPre =
+      (E->getLHS() == E->getIdx()) ? buildScalarExpr(E->getIdx()) : nullptr;
   bool SignedIndices = false;
-  auto EmitIdxAfterBase = [&](bool Promote) -> mlir::Value {
-    mlir::Value Idx;
+  auto EmitIdxAfterBase = [&, IdxPre](bool Promote) -> mlir::Value {
+    mlir::Value Idx = IdxPre;
     if (E->getLHS() != E->getIdx()) {
       assert(E->getRHS() == E->getIdx() && "index was neither LHS nor RHS");
       Idx = buildScalarExpr(E->getIdx());
@@ -1187,39 +1191,41 @@ LValue CIRGenFunction::buildArraySubscriptExpr(const ArraySubscriptExpr *E,
     bool IdxSigned = IdxTy->isSignedIntegerOrEnumerationType();
     SignedIndices |= IdxSigned;
 
-    assert(!SanOpts.has(SanitizerKind::ArrayBounds) && "not implemented");
+    if (SanOpts.has(SanitizerKind::ArrayBounds))
+      llvm_unreachable("array bounds sanitizer is NYI");
 
-    // TODO: Extend or truncate the index type to 32 or 64-bits.
-    // if (Promote && !Idx.getType().isa<::mlir::cir::PointerType>()) {
-    //   Idx = Builder.CreateIntCast(Idx, IntPtrTy, IdxSigned, "idxprom");
-    // }
+    // Extend or truncate the index type to 32 or 64-bits.
+    auto ptrTy = Idx.getType().dyn_cast<mlir::cir::PointerType>();
+    if (Promote && ptrTy && ptrTy.getPointee().isa<mlir::cir::IntType>())
+      llvm_unreachable("index type cast is NYI");
 
     return Idx;
   };
+  IdxPre = nullptr;
 
   // If the base is a vector type, then we are forming a vector element
   // with this subscript.
   if (E->getBase()->getType()->isVectorType() &&
       !isa<ExtVectorElementExpr>(E->getBase())) {
-    assert(0 && "not implemented");
+    llvm_unreachable("vector subscript is NYI");
   }
 
   // All the other cases basically behave like simple offsetting.
 
   // Handle the extvector case we ignored above.
   if (isa<ExtVectorElementExpr>(E->getBase())) {
-    assert(0 && "not implemented");
+    llvm_unreachable("extvector subscript is NYI");
   }
 
-  // TODO: TBAAAccessInfo
+  assert(!UnimplementedFeature::tbaa() && "TBAA is NYI");
   LValueBaseInfo EltBaseInfo;
   Address Addr = Address::invalid();
   if (const VariableArrayType *vla =
           getContext().getAsVariableArrayType(E->getType())) {
-    assert(0 && "not implemented");
+    llvm_unreachable("variable array subscript is NYI");
   } else if (const ObjCObjectType *OIT =
                  E->getType()->getAs<ObjCObjectType>()) {
-    assert(0 && "not implemented");
+    llvm_unreachable("ObjC object type subscript is NYI");
   } else if (const Expr *Array = isSimpleArrayDecayOperand(E->getBase())) {
     // If this is A[i] where A is an array, the frontend will have decayed
     // the base to be a ArrayToPointerDecay implicit cast.  While correct, it is
@@ -1230,26 +1236,26 @@ LValue CIRGenFunction::buildArraySubscriptExpr(const ArraySubscriptExpr *E,
     LValue ArrayLV;
     // For simple multidimensional array indexing, set the 'accessed' flag
     // for better bounds-checking of the base expression.
-    // if (const auto *ASE = dyn_cast<ArraySubscriptExpr>(Array))
-    //   ArrayLV = buildArraySubscriptExpr(ASE, /*Accessed*/ true);
-    assert(!llvm::isa<ArraySubscriptExpr>(Array) &&
-           "multidimensional array indexing not implemented");
-
-    ArrayLV = buildLValue(Array);
+    if (const auto *ASE = dyn_cast<ArraySubscriptExpr>(Array))
+      ArrayLV = buildArraySubscriptExpr(ASE, /*Accessed=*/true);
+    else
+      ArrayLV = buildLValue(Array);
     auto Idx = EmitIdxAfterBase(/*Promote=*/true);
-    QualType arrayType = Array->getType();
 
     // Propagate the alignment from the array itself to the result.
+    QualType arrayType = Array->getType();
     Addr = buildArraySubscriptPtr(
         *this, CGM.getLoc(Array->getBeginLoc()), CGM.getLoc(Array->getEndLoc()),
         ArrayLV.getAddress(), {Idx}, E->getType(),
         !getLangOpts().isSignedOverflowDefined(), SignedIndices,
         CGM.getLoc(E->getExprLoc()), &arrayType, E->getBase());
     EltBaseInfo = ArrayLV.getBaseInfo();
-    // TODO: EltTBAAInfo
+    // TODO(cir): EltTBAAInfo
+    assert(!UnimplementedFeature::tbaa() && "TBAA is NYI");
   } else {
     // The base must be a pointer; emit it with an estimate of its alignment.
     // TODO(cir): EltTBAAInfo
+    assert(!UnimplementedFeature::tbaa() && "TBAA is NYI");
     Addr = buildPointerWithAlignment(E->getBase(), &EltBaseInfo);
     auto Idx = EmitIdxAfterBase(/*Promote*/ true);
     QualType ptrType = E->getBase()->getType();
@@ -1260,9 +1266,11 @@ LValue CIRGenFunction::buildArraySubscriptExpr(const ArraySubscriptExpr *E,
   }
 
   LValue LV = LValue::makeAddr(Addr, E->getType(), EltBaseInfo);
+
   if (getLangOpts().ObjC && getLangOpts().getGC() != LangOptions::NonGC) {
-    assert(0 && "not implemented");
+    llvm_unreachable("ObjC is NYI");
   }
+
   return LV;
 }
 

clang/lib/CIR/CodeGen/CIRGenExprAgg.cpp

@@ -445,8 +445,8 @@ void AggExprEmitter::VisitLambdaExpr(LambdaExpr *E) {
     }
 
     // Emit initialization
-    LValue LV =
-        CGF.buildLValueForFieldInitialization(SlotLV, *CurField, fieldName);
+    LValue LV = CGF.buildLValueForFieldInitialization(
+        SlotLV, *CurField, fieldName, CurField->getFieldIndex());
     if (CurField->hasCapturedVLAType()) {
       llvm_unreachable("NYI");
     }
@@ -701,8 +701,8 @@ void AggExprEmitter::VisitCXXParenListOrInitListExpr(
         CGF.getTypes().isZeroInitializable(ExprToVisit->getType()))
       break;
 
-    LValue LV =
-        CGF.buildLValueForFieldInitialization(DestLV, field, field->getName());
+    LValue LV = CGF.buildLValueForFieldInitialization(
+        DestLV, field, field->getName(), field->getFieldIndex());
     // We never generate write-barries for initialized fields.
     assert(!UnimplementedFeature::setNonGC());
 

clang/lib/CIR/CodeGen/CIRGenFunction.h

@@ -44,7 +44,7 @@ class CallOp;
 namespace {
 class ScalarExprEmitter;
 class AggExprEmitter;
-}
+} // namespace
 
 namespace cir {
 
@@ -1448,7 +1448,8 @@ class CIRGenFunction : public CIRGenTypeCache {
   /// stored in the reference.
   LValue buildLValueForFieldInitialization(LValue Base,
                                            const clang::FieldDecl *Field,
-                                           llvm::StringRef FieldName);
+                                           llvm::StringRef FieldName,
+                                           unsigned FieldIndex);
 
   void buildInitializerForField(clang::FieldDecl *Field, LValue LHS,
                                 clang::Expr *Init);

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

@@ -15,6 +15,7 @@
 
 #include "mlir/IR/Matchers.h"
 #include "mlir/IR/PatternMatch.h"
+#include "mlir/Support/LogicalResult.h"
 #include "mlir/Transforms/GreedyPatternRewriteDriver.h"
 
 using namespace mlir;
@@ -156,10 +157,17 @@ template <>
 mlir::LogicalResult
 SimplifyRetYieldBlocks<ScopeOp>::replaceScopeLikeOp(PatternRewriter &rewriter,
                                                     ScopeOp scopeOp) const {
-  auto regionChanged = mlir::failure();
+  // Scope region empty: just remove scope.
+  if (scopeOp.getRegion().empty()) {
+    rewriter.eraseOp(scopeOp);
+    return mlir::success();
+  }
+
+  // Scope region non-empty: clean it up.
   if (checkAndRewriteRegion(scopeOp.getRegion(), rewriter).succeeded())
-    regionChanged = mlir::success();
-  return regionChanged;
+    return mlir::success();
+
+  return mlir::failure();
 }
 
 template <>

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

@@ -1104,7 +1104,7 @@ class CIRBinOpLowering : public mlir::OpConversionPattern<mlir::cir::BinOp> {
         if (ty.isUnsigned())
           rewriter.replaceOpWithNewOp<mlir::LLVM::UDivOp>(op, llvmTy, lhs, rhs);
         else
-          llvm_unreachable("signed integer division binop lowering NYI");
+          rewriter.replaceOpWithNewOp<mlir::LLVM::SDivOp>(op, llvmTy, lhs, rhs);
       } else
         rewriter.replaceOpWithNewOp<mlir::LLVM::FDivOp>(op, llvmTy, lhs, rhs);
       break;
@@ -1113,7 +1113,7 @@ class CIRBinOpLowering : public mlir::OpConversionPattern<mlir::cir::BinOp> {
         if (ty.isUnsigned())
           rewriter.replaceOpWithNewOp<mlir::LLVM::URemOp>(op, llvmTy, lhs, rhs);
         else
-          llvm_unreachable("signed integer remainder binop lowering NYI");
+          rewriter.replaceOpWithNewOp<mlir::LLVM::SRemOp>(op, llvmTy, lhs, rhs);
       } else
         rewriter.replaceOpWithNewOp<mlir::LLVM::FRemOp>(op, llvmTy, lhs, rhs);
       break;
@@ -1134,7 +1134,7 @@ class CIRBinOpLowering : public mlir::OpConversionPattern<mlir::cir::BinOp> {
         if (ty.isUnsigned())
           rewriter.replaceOpWithNewOp<mlir::LLVM::LShrOp>(op, llvmTy, lhs, rhs);
         else
-          llvm_unreachable("signed integer shift binop lowering NYI");
+          rewriter.replaceOpWithNewOp<mlir::LLVM::AShrOp>(op, llvmTy, lhs, rhs);
         break;
       }
     }
@@ -1286,18 +1286,37 @@ class CIRBrOpLowering : public mlir::OpConversionPattern<mlir::cir::BrOp> {
   }
 };
 
+class CIRStructElementAddrOpLowering
+    : public mlir::OpConversionPattern<mlir::cir::StructElementAddr> {
+public:
+  using mlir::OpConversionPattern<
+      mlir::cir::StructElementAddr>::OpConversionPattern;
+
+  mlir::LogicalResult
+  matchAndRewrite(mlir::cir::StructElementAddr op, OpAdaptor adaptor,
+                  mlir::ConversionPatternRewriter &rewriter) const override {
+    auto llResTy = getTypeConverter()->convertType(op.getType());
+    // Since the base address is a pointer to structs, the first offset is
+    // always zero. The second offset tell us which member it will access.
+    llvm::SmallVector<mlir::LLVM::GEPArg> offset{0, op.getIndex()};
+    rewriter.replaceOpWithNewOp<mlir::LLVM::GEPOp>(
+        op, llResTy, adaptor.getStructAddr(), offset);
+    return mlir::success();
+  }
+};
+
 void populateCIRToLLVMConversionPatterns(mlir::RewritePatternSet &patterns,
                                          mlir::TypeConverter &converter) {
   patterns.add<CIRReturnLowering>(patterns.getContext());
-  patterns.add<CIRCmpOpLowering, CIRLoopOpLowering, CIRBrCondOpLowering,
-               CIRPtrStrideOpLowering, CIRCallLowering, CIRUnaryOpLowering,
-               CIRBinOpLowering, CIRLoadLowering, CIRConstantLowering,
-               CIRStoreLowering, CIRAllocaLowering, CIRFuncLowering,
-               CIRScopeOpLowering, CIRCastOpLowering, CIRIfLowering,
-               CIRGlobalOpLowering, CIRGetGlobalOpLowering, CIRVAStartLowering,
-               CIRVAEndLowering, CIRVACopyLowering, CIRVAArgLowering,
-               CIRBrOpLowering, CIRTernaryOpLowering>(converter,
-                                                      patterns.getContext());
+  patterns.add<
+      CIRCmpOpLowering, CIRLoopOpLowering, CIRBrCondOpLowering,
+      CIRPtrStrideOpLowering, CIRCallLowering, CIRUnaryOpLowering,
+      CIRBinOpLowering, CIRLoadLowering, CIRConstantLowering, CIRStoreLowering,
+      CIRAllocaLowering, CIRFuncLowering, CIRScopeOpLowering, CIRCastOpLowering,
+      CIRIfLowering, CIRGlobalOpLowering, CIRGetGlobalOpLowering,
+      CIRVAStartLowering, CIRVAEndLowering, CIRVACopyLowering, CIRVAArgLowering,
+      CIRBrOpLowering, CIRTernaryOpLowering, CIRStructElementAddrOpLowering>(
+      converter, patterns.getContext());
 }
 
 namespace {