Update on "[CIR][Bugfix] Fix cir.array getTypeSizeInBits method"

sitio-couto · sitio-couto · commit 614adaa927e6 · 2023-08-07T12:04:53.000-03:00
Constant initialization of static local arrays would fail due to a
mismatch between the variable and the initializer type size. This patch
fixes the data layout interface implementation for the cir.array type.

A complete array in C/C++ should have its type size in bits equal to
the size of the array times the size of the element type.

[ghstack-poisoned]
diff --git a/clang-tools-extra/clang-tidy/cir-tidy/CIRASTConsumer.cpp b/clang-tools-extra/clang-tidy/cir-tidy/CIRASTConsumer.cpp
@@ -10,7 +10,6 @@
 #include "CIRChecks.h"
 
 #include "../utils/OptionsUtils.h"
-#include "ClangTidyCheck.h"
 #include "mlir/IR/BuiltinOps.h"
 #include "mlir/IR/MLIRContext.h"
 #include "mlir/Pass/Pass.h"
@@ -26,7 +25,16 @@ namespace tidy {
 
 CIRASTConsumer::CIRASTConsumer(CompilerInstance &CI, StringRef inputFile,
                                clang::tidy::ClangTidyContext &Context)
-    : Context(Context) {
+    : Context(Context),
+      OptsView(ClangTidyCheck::OptionsView(cir::checks::LifetimeCheckName,
+                                           Context.getOptions().CheckOptions,
+                                           &Context)) {
+  // Setup CIR codegen options via config specified information.
+  CI.getCodeGenOpts().ClangIRBuildDeferredThreshold =
+      OptsView.get("CodeGenBuildDeferredThreshold", 500U);
+  CI.getCodeGenOpts().ClangIRSkipFunctionsFromSystemHeaders =
+      OptsView.get("CodeGenSkipFunctionsFromSystemHeaders", false);
+
   Gen = std::make_unique<CIRGenerator>(CI.getDiagnostics(), nullptr,
                                        CI.getCodeGenOpts());
 }
@@ -138,10 +146,6 @@ void CIRASTConsumer::HandleTranslationUnit(ASTContext &C) {
   mlir::PassManager pm(mlirCtx.get());
   pm.addPass(mlir::createMergeCleanupsPass());
 
-  clang::tidy::ClangTidyOptions Opts = Context.getOptions();
-  ClangTidyCheck::OptionsView OptsView(cir::checks::LifetimeCheckName,
-                                       Opts.CheckOptions, &Context);
-
   auto remarks =
       utils::options::parseStringList(OptsView.get("RemarksList", ""));
   auto hist =
diff --git a/clang-tools-extra/clang-tidy/cir-tidy/CIRASTConsumer.h b/clang-tools-extra/clang-tidy/cir-tidy/CIRASTConsumer.h
@@ -1,4 +1,5 @@
 #include "../ClangTidyDiagnosticConsumer.h"
+#include "ClangTidyCheck.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/CIR/CIRGenerator.h"
 #include "clang/Frontend/CompilerInstance.h"
@@ -19,6 +20,7 @@ class CIRASTConsumer : public ASTConsumer {
   std::unique_ptr<CIRGenerator> Gen;
   ASTContext *AstContext{nullptr};
   clang::tidy::ClangTidyContext &Context;
+  clang::tidy::ClangTidyCheck::OptionsView OptsView;
 };
 } // namespace tidy
 } // namespace cir
diff --git a/clang-tools-extra/test/cir-tidy/lifetime-basic.cpp b/clang-tools-extra/test/cir-tidy/lifetime-basic.cpp
@@ -3,6 +3,8 @@
 // RUN: -config='{CheckOptions: \
 // RUN:  [{key: cir-lifetime-check.RemarksList, value: "all"}, \
 // RUN:   {key: cir-lifetime-check.HistLimit, value: "1"}, \
+// RUN:   {key: cir-lifetime-check.CodeGenBuildDeferredThreshold, value: "500"}, \
+// RUN:   {key: cir-lifetime-check.CodeGenSkipFunctionsFromSystemHeaders, value: "false"}, \
 // RUN:   {key: cir-lifetime-check.HistoryList, value: "invalid;null"}]}' \
 // RUN: --
 // RUN: FileCheck -input-file=%t.yaml -check-prefix=CHECK-YAML %s
diff --git a/clang/lib/CIR/CodeGen/CIRGenBuilder.h b/clang/lib/CIR/CodeGen/CIRGenBuilder.h
@@ -102,6 +102,23 @@ class CIRGenBuilderTy : public mlir::OpBuilder {
   // Attribute helpers
   // -----------------
   //
+
+  /// Get constant address of a global variable as an MLIR attribute.
+  /// This wrapper infers the attribute type through the global op.
+  mlir::cir::GlobalViewAttr getGlobalViewAttr(mlir::cir::GlobalOp globalOp,
+                                              mlir::ArrayAttr indices = {}) {
+    auto type = getPointerTo(globalOp.getSymType());
+    return getGlobalViewAttr(type, globalOp, indices);
+  }
+
+  /// Get constant address of a global variable as an MLIR attribute.
+  mlir::cir::GlobalViewAttr getGlobalViewAttr(mlir::cir::PointerType type,
+                                              mlir::cir::GlobalOp globalOp,
+                                              mlir::ArrayAttr indices = {}) {
+    auto symbol = mlir::FlatSymbolRefAttr::get(globalOp.getSymNameAttr());
+    return mlir::cir::GlobalViewAttr::get(type, symbol, indices);
+  }
+
   mlir::TypedAttr getZeroAttr(mlir::Type t) {
     return mlir::cir::ZeroAttr::get(getContext(), t);
   }
diff --git a/clang/lib/CIR/CodeGen/CIRGenExprConst.cpp b/clang/lib/CIR/CodeGen/CIRGenExprConst.cpp
@@ -17,6 +17,7 @@
 #include "CIRGenModule.h"
 #include "mlir/IR/Attributes.h"
 #include "mlir/IR/BuiltinAttributeInterfaces.h"
+#include "mlir/IR/BuiltinAttributes.h"
 #include "clang/AST/APValue.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/Attr.h"
@@ -996,16 +997,16 @@ namespace {
 /// A struct which can be used to peephole certain kinds of finalization
 /// that normally happen during l-value emission.
 struct ConstantLValue {
-  using SymbolTy = mlir::SymbolRefAttr;
-  llvm::PointerUnion<mlir::Value, SymbolTy> Value;
+  llvm::PointerUnion<mlir::Value, mlir::Attribute> Value;
   bool HasOffsetApplied;
 
   /*implicit*/ ConstantLValue(mlir::Value value, bool hasOffsetApplied = false)
       : Value(value), HasOffsetApplied(hasOffsetApplied) {}
 
-  /*implicit*/ ConstantLValue(SymbolTy address) : Value(address) {}
+  /*implicit*/ ConstantLValue(mlir::SymbolRefAttr address) : Value(address) {}
 
   ConstantLValue(std::nullptr_t) : ConstantLValue({}, false) {}
+  ConstantLValue(mlir::Attribute value) : Value(value) {}
 };
 
 /// A helper class for emitting constant l-values.
@@ -1050,10 +1051,13 @@ class ConstantLValueEmitter
   /// Return the value offset.
   mlir::Attribute getOffset() { llvm_unreachable("NYI"); }
 
+  // TODO(cir): create a proper interface to absctract CIR constant values.
+
   /// Apply the value offset to the given constant.
-  mlir::Attribute applyOffset(mlir::Attribute C) {
+  ConstantLValue applyOffset(ConstantLValue &C) {
     if (!hasNonZeroOffset())
       return C;
+
     // TODO(cir): use ptr_stride, or something...
     llvm_unreachable("NYI");
   }
@@ -1089,15 +1093,15 @@ mlir::Attribute ConstantLValueEmitter::tryEmit() {
     return {};
 
   // Apply the offset if necessary and not already done.
-  if (!result.HasOffsetApplied && !value.is<ConstantLValue::SymbolTy>()) {
-    assert(0 && "NYI");
+  if (!result.HasOffsetApplied && !value.is<mlir::Attribute>()) {
+    value = applyOffset(result).Value;
   }
 
   // Convert to the appropriate type; this could be an lvalue for
   // an integer. FIXME: performAddrSpaceCast
   if (destTy.isa<mlir::cir::PointerType>()) {
-    if (value.is<ConstantLValue::SymbolTy>())
-      return value.get<ConstantLValue::SymbolTy>();
+    if (value.is<mlir::Attribute>())
+      return value.get<mlir::Attribute>();
     llvm_unreachable("NYI");
   }
 
@@ -1121,7 +1125,30 @@ ConstantLValue
 ConstantLValueEmitter::tryEmitBase(const APValue::LValueBase &base) {
   // Handle values.
   if (const ValueDecl *D = base.dyn_cast<const ValueDecl *>()) {
-    assert(0 && "NYI");
+    // The constant always points to the canonical declaration. We want to look
+    // at properties of the most recent declaration at the point of emission.
+    D = cast<ValueDecl>(D->getMostRecentDecl());
+
+    if (D->hasAttr<WeakRefAttr>())
+      llvm_unreachable("emit pointer base for weakref is NYI");
+
+    if (auto *FD = dyn_cast<FunctionDecl>(D))
+      llvm_unreachable("emit pointer base for fun decl is NYI");
+
+    if (auto *VD = dyn_cast<VarDecl>(D)) {
+      // We can never refer to a variable with local storage.
+      if (!VD->hasLocalStorage()) {
+        if (VD->isFileVarDecl() || VD->hasExternalStorage())
+          return CGM.getAddrOfGlobalVarAttr(VD);
+
+        if (VD->isLocalVarDecl()) {
+          auto linkage =
+              CGM.getCIRLinkageVarDefinition(VD, /*IsConstant=*/false);
+          return CGM.getBuilder().getGlobalViewAttr(
+              CGM.getOrCreateStaticVarDecl(*VD, linkage));
+        }
+      }
+    }
   }
 
   // Handle typeid(T).
diff --git a/clang/lib/CIR/CodeGen/CIRGenItaniumCXXABI.cpp b/clang/lib/CIR/CodeGen/CIRGenItaniumCXXABI.cpp
@@ -1107,9 +1107,8 @@ mlir::Attribute CIRGenItaniumRTTIBuilder::BuildTypeInfo(mlir::Location loc,
   if (OldGV && !OldGV.isDeclaration()) {
     assert(!OldGV.hasAvailableExternallyLinkage() &&
            "available_externally typeinfos not yet implemented");
-    return mlir::cir::GlobalViewAttr::get(
-        CGM.getBuilder().getUInt8PtrTy(),
-        mlir::FlatSymbolRefAttr::get(OldGV.getSymNameAttr()));
+    return CGM.getBuilder().getGlobalViewAttr(CGM.getBuilder().getUInt8PtrTy(),
+                                              OldGV);
   }
 
   // Check if there is already an external RTTI descriptor for this type.
@@ -1275,10 +1274,9 @@ void CIRGenItaniumRTTIBuilder::BuildVTablePointer(mlir::Location loc,
   } else {
     SmallVector<mlir::Attribute, 4> offsets{
         mlir::cir::IntAttr::get(PtrDiffTy, 2)};
-    field = mlir::cir::GlobalViewAttr::get(
-        builder.getUInt8PtrTy(),
-        mlir::FlatSymbolRefAttr::get(VTable.getSymNameAttr()),
-        mlir::ArrayAttr::get(builder.getContext(), offsets));
+    auto indices = mlir::ArrayAttr::get(builder.getContext(), offsets);
+    field = CGM.getBuilder().getGlobalViewAttr(CGM.getBuilder().getUInt8PtrTy(),
+                                               VTable, indices);
   }
 
   assert(field && "expected attribute");
@@ -1347,9 +1345,7 @@ CIRGenItaniumRTTIBuilder::GetAddrOfExternalRTTIDescriptor(mlir::Location loc,
       llvm_unreachable("NYI");
   }
 
-  return mlir::cir::GlobalViewAttr::get(
-      builder.getUInt8PtrTy(),
-      mlir::FlatSymbolRefAttr::get(GV.getSymNameAttr()));
+  return builder.getGlobalViewAttr(builder.getUInt8PtrTy(), GV);
 }
 
 mlir::Attribute CIRGenItaniumRTTIBuilder::BuildTypeInfo(
@@ -1374,9 +1370,8 @@ mlir::Attribute CIRGenItaniumRTTIBuilder::BuildTypeInfo(
     // for global pointers.  This is very ARM64-specific.
     llvm_unreachable("NYI");
   } else {
-    TypeNameField = mlir::cir::GlobalViewAttr::get(
-        builder.getUInt8PtrTy(),
-        mlir::FlatSymbolRefAttr::get(TypeName.getSymNameAttr()));
+    TypeNameField =
+        builder.getGlobalViewAttr(builder.getUInt8PtrTy(), TypeName);
   }
   Fields.push_back(TypeNameField);
 
@@ -1539,9 +1534,7 @@ mlir::Attribute CIRGenItaniumRTTIBuilder::BuildTypeInfo(
   assert(!UnimplementedFeature::setDSOLocal());
   CIRGenModule::setInitializer(GV, init);
 
-  return mlir::cir::GlobalViewAttr::get(
-      builder.getUInt8PtrTy(),
-      mlir::FlatSymbolRefAttr::get(GV.getSymNameAttr()));
+  return builder.getGlobalViewAttr(builder.getUInt8PtrTy(), GV);;
 }
 
 mlir::Attribute CIRGenItaniumCXXABI::getAddrOfRTTIDescriptor(mlir::Location loc,
diff --git a/clang/lib/CIR/CodeGen/CIRGenModule.cpp b/clang/lib/CIR/CodeGen/CIRGenModule.cpp
@@ -699,6 +699,18 @@ mlir::Value CIRGenModule::getAddrOfGlobalVar(const VarDecl *D, mlir::Type Ty,
                                                 ptrTy, g.getSymName());
 }
 
+mlir::cir::GlobalViewAttr
+CIRGenModule::getAddrOfGlobalVarAttr(const VarDecl *D, mlir::Type Ty,
+                                     ForDefinition_t IsForDefinition) {
+  assert(D->hasGlobalStorage() && "Not a global variable");
+  QualType ASTTy = D->getType();
+  if (!Ty)
+    Ty = getTypes().convertTypeForMem(ASTTy);
+
+  auto globalOp = buildGlobal(D, Ty, IsForDefinition);
+  return builder.getGlobalViewAttr(builder.getPointerTo(Ty), globalOp);
+}
+
 mlir::Operation* CIRGenModule::getWeakRefReference(const ValueDecl *VD) {
   const AliasAttr *AA = VD->getAttr<AliasAttr>();
   assert(AA && "No alias?");
diff --git a/clang/lib/CIR/CodeGen/CIRGenModule.h b/clang/lib/CIR/CodeGen/CIRGenModule.h
@@ -231,6 +231,11 @@ class CIRGenModule : public CIRGenTypeCache {
   getAddrOfGlobalVar(const VarDecl *D, mlir::Type Ty = {},
                      ForDefinition_t IsForDefinition = NotForDefinition);
 
+  /// Return the mlir::GlobalViewAttr for the address of the given global.
+  mlir::cir::GlobalViewAttr
+  getAddrOfGlobalVarAttr(const VarDecl *D, mlir::Type Ty = {},
+                         ForDefinition_t IsForDefinition = NotForDefinition);
+
   /// Get a reference to the target of VD.
   mlir::Operation* getWeakRefReference(const ValueDecl *VD);
 
diff --git a/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp b/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
@@ -1209,6 +1209,15 @@ class CIRGlobalOpLowering
           op->getLoc(), lowerCirAttrAsValue(structAttr, op->getLoc(), rewriter,
                                             typeConverter));
       return mlir::success();
+    } else if (auto attr = init.value().dyn_cast<mlir::cir::GlobalViewAttr>()) {
+      setupRegionInitializedLLVMGlobalOp(op, rewriter);
+
+      // Return the address of the global symbol.
+      auto elementType = typeConverter->convertType(attr.getType());
+      auto addrOfOp = rewriter.create<mlir::LLVM::AddressOfOp>(
+          op->getLoc(), elementType, attr.getSymbol());
+      rewriter.create<mlir::LLVM::ReturnOp>(op->getLoc(), addrOfOp.getResult());
+      return mlir::success();
     } else {
       op.emitError() << "usupported initializer '" << init.value() << "'";
       return mlir::failure();
diff --git a/clang/test/CIR/CodeGen/globals.cpp b/clang/test/CIR/CodeGen/globals.cpp
@@ -115,3 +115,8 @@ int testExternVar(void) { return externVar; }
 // CHECK: cir.global "private" external @externVar : !s32i
 // CHECK: cir.func @{{.+}}testExternVar
 // CHECK:   cir.get_global @externVar : cir.ptr <!s32i>
+
+// Should constant initialize global with constant address.
+int var = 1;
+int *constAddr = &var;
+// CHECK-DAG: cir.global external @constAddr = #cir.global_view<@var> : !cir.ptr<!s32i>
diff --git a/clang/test/CIR/CodeGen/static-vars.c b/clang/test/CIR/CodeGen/static-vars.c
@@ -36,6 +36,13 @@ void func2(void) {
   // CHECK-DAG: cir.global "private" internal @func2.j = 0.000000e+00 : f32
 }
 
+// Should const initialize static vars with constant addresses.
+void func3(void) {
+  static int var;
+  static int *constAddr = &var;
+  // CHECK-DAG: cir.global "private" internal @func3.constAddr = #cir.global_view<@func3.var> : !cir.ptr<!s32i>
+}
+
 // Should match type size in bytes between var and initializer.
 void func4(void) {
   static char string[] = "Hello";
diff --git a/clang/test/CIR/Lowering/globals.cir b/clang/test/CIR/Lowering/globals.cir
@@ -1,5 +1,7 @@
-// RUN: cir-opt %s -cir-to-llvm -o - | FileCheck %s -check-prefix=MLIR
-// RUN: cir-translate %s -cir-to-llvmir  | FileCheck %s -check-prefix=LLVM
+// RUN: cir-opt %s -cir-to-llvm -o %t.cir
+// RUN: FileCheck --input-file=%t.cir %s -check-prefix=MLIR
+// RUN: cir-translate %s -cir-to-llvmir -o %t.ll
+// RUN: FileCheck --input-file=%t.ll %s -check-prefix=LLVM
 
 !s16i = !cir.int<s, 16>
 !s32i = !cir.int<s, 32>
@@ -26,6 +28,11 @@ module {
   // MLIR: }
   // LLVM: @.str = internal constant [8 x i8] c"example\00"
   // LLVM: @s = global ptr @.str
+  cir.global external @aPtr = #cir.global_view<@a> : !cir.ptr<!s32i>
+  // MLIR: llvm.mlir.global external @aPtr() {addr_space = 0 : i32} : !llvm.ptr<i32> {
+  // MLIR:   %0 = llvm.mlir.addressof @a : !llvm.ptr<i32>
+  // MLIR:   llvm.return %0 : !llvm.ptr<i32>
+  // MLIR: }
   cir.global "private" constant internal @".str1" = #cir.const_array<"example1\00" : !cir.array<!s8i x 9>> : !cir.array<!s8i x 9> {alignment = 1 : i64}
   cir.global external @s1 = @".str1": !cir.ptr<!s8i>
   cir.global external @s2 = @".str": !cir.ptr<!s8i>
