From 0766623ed5172c850cde042ffa7c46c59d0d5a0e Mon Sep 17 00:00:00 2001 From: Sirui Mu Date: Wed, 3 Jan 2024 15:36:13 +0800 Subject: [PATCH 1/2] [CIR][CodeGen] support array def after decl with unknown bound Arrays can be first declared without a known bound, and then defined with a known bound. clangir crashes on generating CIR for this case. This patch adds support. --- clang/lib/CIR/CodeGen/CIRGenModule.cpp | 45 ++++++++++++++++--- clang/lib/CIR/CodeGen/CIRGenModule.h | 11 ++++- .../test/CIR/CodeGen/array-unknown-bound.cpp | 11 +++++ 3 files changed, 60 insertions(+), 7 deletions(-) create mode 100644 clang/test/CIR/CodeGen/array-unknown-bound.cpp diff --git a/clang/lib/CIR/CodeGen/CIRGenModule.cpp b/clang/lib/CIR/CodeGen/CIRGenModule.cpp index 8dfd3c89678a..9506439035b3 100644 --- a/clang/lib/CIR/CodeGen/CIRGenModule.cpp +++ b/clang/lib/CIR/CodeGen/CIRGenModule.cpp @@ -471,7 +471,8 @@ mlir::Value CIRGenModule::getGlobalValue(const Decl *D) { mlir::cir::GlobalOp CIRGenModule::createGlobalOp(CIRGenModule &CGM, mlir::Location loc, StringRef name, mlir::Type t, - bool isCst) { + bool isCst, + mlir::Operation *insertPoint) { mlir::cir::GlobalOp g; auto &builder = CGM.getBuilder(); { @@ -486,8 +487,12 @@ mlir::cir::GlobalOp CIRGenModule::createGlobalOp(CIRGenModule &CGM, builder.setInsertionPoint(curCGF->CurFn); g = builder.create(loc, name, t, isCst); - if (!curCGF) - CGM.getModule().push_back(g); + if (!curCGF) { + if (insertPoint) + CGM.getModule().insert(insertPoint, g); + else + CGM.getModule().push_back(g); + } // Default to private until we can judge based on the initializer, // since MLIR doesn't allow public declarations. @@ -501,6 +506,31 @@ void CIRGenModule::setCommonAttributes(GlobalDecl GD, mlir::Operation *GV) { assert(!UnimplementedFeature::setCommonAttributes()); } +void CIRGenModule::replaceGlobal(mlir::cir::GlobalOp Old, + mlir::cir::GlobalOp New) { + assert(Old.getSymName() == New.getSymName() && "symbol names must match"); + + // If the types does not match, update all references to Old to the new type. + auto OldTy = Old.getSymType(); + auto NewTy = New.getSymType(); + if (OldTy != NewTy) { + auto OldSymUses = Old.getSymbolUses(theModule.getOperation()); + if (OldSymUses.has_value()) { + for (auto Use : *OldSymUses) { + auto UseOp = dyn_cast(Use.getUser()); + assert(UseOp && "GlobalOp symbol user is not a GetGlobalOp"); + + auto UseOpResultValue = UseOp.getAddr(); + UseOpResultValue.setType( + mlir::cir::PointerType::get(builder.getContext(), NewTy)); + } + } + } + + // Remove old global from the module. + Old.erase(); +} + /// If the specified mangled name is not in the module, /// create and return an mlir GlobalOp with the specified type (TODO(cir): /// address space). @@ -592,11 +622,14 @@ CIRGenModule::getOrCreateCIRGlobal(StringRef MangledName, mlir::Type Ty, // mlir::SymbolTable::Visibility::Public is the default, no need to explicitly // mark it as such. auto GV = CIRGenModule::createGlobalOp(*this, loc, MangledName, Ty, - /*isConstant=*/false); + /*isConstant=*/false, + /*insertPoint=*/Entry.getOperation()); // If we already created a global with the same mangled name (but different - // type) before, take its name and remove it from its parent. - assert(!Entry && "not implemented"); + // type) before, replace it with the new global. + if (Entry) { + replaceGlobal(Entry, GV); + } // This is the first use or definition of a mangled name. If there is a // deferred decl with this name, remember that we need to emit it at the end diff --git a/clang/lib/CIR/CodeGen/CIRGenModule.h b/clang/lib/CIR/CodeGen/CIRGenModule.h index aeb1313b38c4..a0b30e7464ab 100644 --- a/clang/lib/CIR/CodeGen/CIRGenModule.h +++ b/clang/lib/CIR/CodeGen/CIRGenModule.h @@ -220,7 +220,8 @@ class CIRGenModule : public CIRGenTypeCache { static mlir::cir::GlobalOp createGlobalOp(CIRGenModule &CGM, mlir::Location loc, StringRef name, - mlir::Type t, bool isCst = false); + mlir::Type t, bool isCst = false, + mlir::Operation *insertPoint = nullptr); /// Return the mlir::Value for the address of the given global variable. /// If Ty is non-null and if the global doesn't exist, then it will be created @@ -445,6 +446,14 @@ class CIRGenModule : public CIRGenTypeCache { void setGVProperties(mlir::Operation *Op, const NamedDecl *D) const; void setGVPropertiesAux(mlir::Operation *Op, const NamedDecl *D) const; + /// Replace the present global `Old` with the given global `New`. Their symbol + /// names must match; their types can be different. Usages of the old global + /// will be automatically updated if their types mismatch. + /// + /// This function will erase the old global. This function will NOT insert the + /// new global into the module. + void replaceGlobal(mlir::cir::GlobalOp Old, mlir::cir::GlobalOp New); + /// Determine whether the definition must be emitted; if this returns \c /// false, the definition can be emitted lazily if it's used. bool MustBeEmitted(const clang::ValueDecl *D); diff --git a/clang/test/CIR/CodeGen/array-unknown-bound.cpp b/clang/test/CIR/CodeGen/array-unknown-bound.cpp new file mode 100644 index 000000000000..27d06bed10c1 --- /dev/null +++ b/clang/test/CIR/CodeGen/array-unknown-bound.cpp @@ -0,0 +1,11 @@ +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-cir %s -o - | FileCheck %s + +extern int table[]; +// CHECK: cir.global external @table = #cir.const_array<[#cir.int<1> : !s32i, #cir.int<2> : !s32i, #cir.int<3> : !s32i]> : !cir.array + +int test() { return table[1]; } +// CHECK: cir.func @_Z4testv() -> !s32i extra( {inline = #cir.inline, optnone = #cir.optnone} ) { +// CHECK-NEXT: %0 = cir.alloca !s32i, cir.ptr , ["__retval"] {alignment = 4 : i64} +// CHECK-NEXT: %1 = cir.get_global @table : cir.ptr > + +int table[3] {1, 2, 3}; \ No newline at end of file From 5edd5ce3419de1a325ab887b207657fc8d5fefe1 Mon Sep 17 00:00:00 2001 From: Sirui Mu Date: Fri, 5 Jan 2024 19:29:54 +0800 Subject: [PATCH 2/2] remove the too restrictive assertion --- clang/lib/CIR/CodeGen/CIRGenModule.cpp | 16 ++++++++++------ clang/test/CIR/CodeGen/array-unknown-bound.cpp | 5 ++++- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/clang/lib/CIR/CodeGen/CIRGenModule.cpp b/clang/lib/CIR/CodeGen/CIRGenModule.cpp index 9506439035b3..16445dddb4cf 100644 --- a/clang/lib/CIR/CodeGen/CIRGenModule.cpp +++ b/clang/lib/CIR/CodeGen/CIRGenModule.cpp @@ -517,12 +517,16 @@ void CIRGenModule::replaceGlobal(mlir::cir::GlobalOp Old, auto OldSymUses = Old.getSymbolUses(theModule.getOperation()); if (OldSymUses.has_value()) { for (auto Use : *OldSymUses) { - auto UseOp = dyn_cast(Use.getUser()); - assert(UseOp && "GlobalOp symbol user is not a GetGlobalOp"); - - auto UseOpResultValue = UseOp.getAddr(); - UseOpResultValue.setType( - mlir::cir::PointerType::get(builder.getContext(), NewTy)); + auto *UserOp = Use.getUser(); + assert((isa(UserOp) || + isa(UserOp)) && + "GlobalOp symbol user is neither a GetGlobalOp nor a GlobalOp"); + + if (auto GGO = dyn_cast(Use.getUser())) { + auto UseOpResultValue = GGO.getAddr(); + UseOpResultValue.setType( + mlir::cir::PointerType::get(builder.getContext(), NewTy)); + } } } } diff --git a/clang/test/CIR/CodeGen/array-unknown-bound.cpp b/clang/test/CIR/CodeGen/array-unknown-bound.cpp index 27d06bed10c1..09f75ca27f27 100644 --- a/clang/test/CIR/CodeGen/array-unknown-bound.cpp +++ b/clang/test/CIR/CodeGen/array-unknown-bound.cpp @@ -3,9 +3,12 @@ extern int table[]; // CHECK: cir.global external @table = #cir.const_array<[#cir.int<1> : !s32i, #cir.int<2> : !s32i, #cir.int<3> : !s32i]> : !cir.array +int *table_ptr = table; +// CHECK: cir.global external @table_ptr = #cir.global_view<@table> : !cir.ptr + int test() { return table[1]; } // CHECK: cir.func @_Z4testv() -> !s32i extra( {inline = #cir.inline, optnone = #cir.optnone} ) { // CHECK-NEXT: %0 = cir.alloca !s32i, cir.ptr , ["__retval"] {alignment = 4 : i64} // CHECK-NEXT: %1 = cir.get_global @table : cir.ptr > -int table[3] {1, 2, 3}; \ No newline at end of file +int table[3] {1, 2, 3};