[PATCH] D84534: [AIX] Static init frontend recovery and backend support
This revision was automatically updated to reflect the committed changes.
Closed by commit rG6ef801aa6bc0: [AIX] Static init frontend recovery and
backend support (authored by Xiangling_L).
Changed prior to commit:
https://reviews.llvm.org/D84534?vs=283712=284355#toc
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D84534/new/
https://reviews.llvm.org/D84534
Files:
clang/lib/CodeGen/CGDeclCXX.cpp
clang/lib/CodeGen/CodeGenModule.h
clang/lib/CodeGen/ItaniumCXXABI.cpp
clang/test/CodeGenCXX/aix-static-init-debug-info.cpp
clang/test/CodeGenCXX/aix-static-init-temp-spec-and-inline-var.cpp
clang/test/CodeGenCXX/aix-static-init.cpp
clang/unittests/CodeGen/IncrementalProcessingTest.cpp
llvm/include/llvm/CodeGen/AsmPrinter.h
llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp
llvm/test/CodeGen/PowerPC/aix-static-init-default-priority.ll
llvm/test/CodeGen/PowerPC/aix-static-init-key-object.ll
llvm/test/CodeGen/PowerPC/aix-static-init-no-unique-module-id.ll
llvm/test/CodeGen/PowerPC/aix-static-init-non-default-priority.ll
Index: llvm/test/CodeGen/PowerPC/aix-static-init-non-default-priority.ll
===
--- /dev/null
+++ llvm/test/CodeGen/PowerPC/aix-static-init-non-default-priority.ll
@@ -0,0 +1,10 @@
+; RUN: not --crash llc -mtriple powerpc-ibm-aix-xcoff < %s 2>&1 | FileCheck %s
+; RUN: not --crash llc -mtriple powerpc64-ibm-aix-xcoff < %s 2>&1 | FileCheck %s
+
+@llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 655, void ()* @foo, i8* null }]
+
+define void @foo() {
+ ret void
+}
+
+; CHECK: LLVM ERROR: prioritized sinit and sterm functions are not yet supported
Index: llvm/test/CodeGen/PowerPC/aix-static-init-no-unique-module-id.ll
===
--- /dev/null
+++ llvm/test/CodeGen/PowerPC/aix-static-init-no-unique-module-id.ll
@@ -0,0 +1,10 @@
+; RUN: not --crash llc -mtriple powerpc-ibm-aix-xcoff %s 2>&1 | FileCheck %s
+; RUN: not --crash llc -mtriple powerpc64-ibm-aix-xcoff %s 2>&1 | FileCheck %s
+
+@llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @foo, i8* null }]
+
+define internal void @foo() {
+ ret void
+}
+
+; CHECK: LLVM ERROR: cannot produce a unique identifier for this module based on strong external symbols
Index: llvm/test/CodeGen/PowerPC/aix-static-init-key-object.ll
===
--- /dev/null
+++ llvm/test/CodeGen/PowerPC/aix-static-init-key-object.ll
@@ -0,0 +1,12 @@
+; RUN: not --crash llc -mtriple powerpc-ibm-aix-xcoff < %s 2>&1 | FileCheck %s
+; RUN: not --crash llc -mtriple powerpc64-ibm-aix-xcoff < %s 2>&1 | FileCheck %s
+
+@v = global i8 0
+
+@llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @foo, i8* @v}]
+
+define void @foo() {
+ ret void
+}
+
+; CHECK: LLVM ERROR: associated data of XXStructor list is not yet supported on AIX
Index: llvm/test/CodeGen/PowerPC/aix-static-init-default-priority.ll
===
--- /dev/null
+++ llvm/test/CodeGen/PowerPC/aix-static-init-default-priority.ll
@@ -0,0 +1,60 @@
+; RUN: llc -mtriple powerpc-ibm-aix-xcoff < %s | FileCheck %s
+; RUN: llc -mtriple powerpc64-ibm-aix-xcoff < %s | FileCheck %s
+
+@llvm.global_ctors = appending global [2 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @init1, i8* null }, { i32, void ()*, i8* } { i32 65535, void ()* @init2, i8* null }]
+@llvm.global_dtors = appending global [2 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @destruct1, i8* null }, { i32, void ()*, i8* } { i32 65535, void ()* @destruct2, i8* null }]
+
+define i32 @extFunc() {
+entry:
+ ret i32 3
+}
+
+define internal void @init1() {
+ ret void
+}
+
+define internal void @destruct1() {
+ ret void
+}
+
+define internal void @init2() {
+ ret void
+}
+
+define internal void @destruct2() {
+ ret void
+}
+
+; CHECK: .lglobl init1[DS]
+; CHECK: .lglobl .init1
+; CHECK: .csect init1[DS]
+; CHECK: __sinit8000_clang_ac404299654d2af7eae71e75c17f7c9b_0: # @init1
+; CHECK: .init1:
+; CHECK: .__sinit8000_clang_ac404299654d2af7eae71e75c17f7c9b_0:
+; CHECK: .lglobl destruct1[DS]
+; CHECK: .lglobl .destruct1
+; CHECK: .csect destruct1[DS]
+; CHECK: __sterm8000_clang_ac404299654d2af7eae71e75c17f7c9b_0: # @destruct1
+; CHECK: .destruct1:
+; CHECK: .__sterm8000_clang_ac404299654d2af7eae71e75c17f7c9b_0:
+; CHECK: .lglobl init2[DS]
+; CHECK: .lglobl .init2
+; CHECK: .csect init2[DS]
+; CHECK: __sinit8000_clang_ac404299654d2af7eae71e75c17f7c9b_1: # @init2
+; CHECK: .init2:
+; CHECK:
[PATCH] D84534: [AIX] Static init frontend recovery and backend support
jasonliu accepted this revision. jasonliu added a comment. This revision is now accepted and ready to land. Thanks. LGTM. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84534/new/ https://reviews.llvm.org/D84534 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D84534: [AIX] Static init frontend recovery and backend support
Xiangling_L updated this revision to Diff 283712.
Xiangling_L marked an inline comment as done.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D84534/new/
https://reviews.llvm.org/D84534
Files:
clang/lib/CodeGen/CGDeclCXX.cpp
clang/lib/CodeGen/CodeGenModule.h
clang/lib/CodeGen/ItaniumCXXABI.cpp
clang/test/CodeGenCXX/aix-static-init-debug-info.cpp
clang/test/CodeGenCXX/aix-static-init-temp-spec-and-inline-var.cpp
clang/test/CodeGenCXX/aix-static-init.cpp
clang/unittests/CodeGen/IncrementalProcessingTest.cpp
llvm/include/llvm/CodeGen/AsmPrinter.h
llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp
llvm/test/CodeGen/PowerPC/aix-static-init-default-priority.ll
llvm/test/CodeGen/PowerPC/aix-static-init-key-object.ll
llvm/test/CodeGen/PowerPC/aix-static-init-no-unique-module-id.ll
llvm/test/CodeGen/PowerPC/aix-static-init-non-default-priority.ll
Index: llvm/test/CodeGen/PowerPC/aix-static-init-non-default-priority.ll
===
--- /dev/null
+++ llvm/test/CodeGen/PowerPC/aix-static-init-non-default-priority.ll
@@ -0,0 +1,10 @@
+; RUN: not llc -mtriple powerpc-ibm-aix-xcoff < %s 2>&1 | FileCheck %s
+; RUN: not llc -mtriple powerpc64-ibm-aix-xcoff < %s 2>&1 | FileCheck %s
+
+@llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 655, void ()* @foo, i8* null }]
+
+define void @foo() {
+ ret void
+}
+
+// CHECK: LLVM ERROR: prioritized sinit and sterm functions are not yet supported
Index: llvm/test/CodeGen/PowerPC/aix-static-init-no-unique-module-id.ll
===
--- /dev/null
+++ llvm/test/CodeGen/PowerPC/aix-static-init-no-unique-module-id.ll
@@ -0,0 +1,7 @@
+; RUN: not llc -mtriple powerpc-ibm-aix-xcoff < %s 2>&1 | FileCheck %s
+; RUN: not llc -mtriple powerpc64-ibm-aix-xcoff < %s 2>&1 | FileCheck %s
+
+@llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @foo, i8* null }]
+@llvm.global_dtors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @bar, i8* null }]
+
+// CHECK: LLVM ERROR: cannot produce a unique identifier for this module based on strong external symbols
Index: llvm/test/CodeGen/PowerPC/aix-static-init-key-object.ll
===
--- /dev/null
+++ llvm/test/CodeGen/PowerPC/aix-static-init-key-object.ll
@@ -0,0 +1,12 @@
+; RUN: not llc -mtriple powerpc-ibm-aix-xcoff < %s 2>&1 | FileCheck %s
+; RUN: not llc -mtriple powerpc64-ibm-aix-xcoff < %s 2>&1 | FileCheck %s
+
+@v = global i8 0
+
+@llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @foo, i8* @v}]
+
+define void @foo() {
+ ret void
+}
+
+// CHECK: LLVM ERROR: associated data of XXStructor list is not yet supported on AIX
Index: llvm/test/CodeGen/PowerPC/aix-static-init-default-priority.ll
===
--- /dev/null
+++ llvm/test/CodeGen/PowerPC/aix-static-init-default-priority.ll
@@ -0,0 +1,60 @@
+; RUN: llc -mtriple powerpc-ibm-aix-xcoff < %s | FileCheck %s
+; RUN: llc -mtriple powerpc64-ibm-aix-xcoff < %s | FileCheck %s
+
+@llvm.global_ctors = appending global [2 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @init1, i8* null }, { i32, void ()*, i8* } { i32 65535, void ()* @init2, i8* null }]
+@llvm.global_dtors = appending global [2 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @destruct1, i8* null }, { i32, void ()*, i8* } { i32 65535, void ()* @destruct2, i8* null }]
+
+define i32 @extFunc() {
+entry:
+ ret i32 3
+}
+
+define internal void @init1() {
+ ret void
+}
+
+define internal void @destruct1() {
+ ret void
+}
+
+define internal void @init2() {
+ ret void
+}
+
+define internal void @destruct2() {
+ ret void
+}
+
+; CHECK: .lglobl init1[DS]
+; CHECK: .lglobl .init1
+; CHECK: .csect init1[DS]
+; CHECK: __sinit8000_clang_ac404299654d2af7eae71e75c17f7c9b_0: # @init1
+; CHECK: .init1:
+; CHECK: .__sinit8000_clang_ac404299654d2af7eae71e75c17f7c9b_0:
+; CHECK: .lglobl destruct1[DS]
+; CHECK: .lglobl .destruct1
+; CHECK: .csect destruct1[DS]
+; CHECK: __sterm8000_clang_ac404299654d2af7eae71e75c17f7c9b_0: # @destruct1
+; CHECK: .destruct1:
+; CHECK: .__sterm8000_clang_ac404299654d2af7eae71e75c17f7c9b_0:
+; CHECK: .lglobl init2[DS]
+; CHECK: .lglobl .init2
+; CHECK: .csect init2[DS]
+; CHECK: __sinit8000_clang_ac404299654d2af7eae71e75c17f7c9b_1: # @init2
+; CHECK: .init2:
+; CHECK: .__sinit8000_clang_ac404299654d2af7eae71e75c17f7c9b_1:
+; CHECK: .lglobl destruct2[DS]
+; CHECK: .lglobl .destruct2
+; CHECK: .csect destruct2[DS]
+; CHECK:
[PATCH] D84534: [AIX] Static init frontend recovery and backend support
Xiangling_L marked 3 inline comments as done. Xiangling_L added inline comments. Comment at: llvm/include/llvm/CodeGen/AsmPrinter.h:466 + + bool preprocessStructorList(const DataLayout , const Constant *List, + SmallVector ); jasonliu wrote: > Xiangling_L wrote: > > jasonliu wrote: > > > Xiangling_L wrote: > > > > jasonliu wrote: > > > > > A doxygen comment describe what this function does, and what its > > > > > return value means, and mention `Structors` is an output argument. > > > > > By looking at what this function does, it seems `buildStructorList` > > > > > is a better name. > > > > I meant to and should've named this function to > > > > `preprocessXXStructorList`, let me know if you still prefer > > > > `buildStructorList`. And if you do, since the underneath of > > > > `SmallVector` is a variable-sized array, maybe we should try > > > > `buildSortedStructorArray`? > > > `preprocess` sounds like we are already having a XXStructorList and now > > > we try to do something on it. > > > But right now, we are actually passing in an empty StructorList/Array and > > > build it from scratch. So I would still prefer the name of `build` in it. > > > I don't mind changing to a more accurate name as you suggested. > > I think we do have a `XXStructorList` here which is the initializer of > > `llvm.gloal_ctors`or `llvm.gloal_dtors` array? The usage of this term is > > consistent with other spots. > My understanding is that before we enter this `preprocessXXStructorList`, we > do not have a list of XXStructor. We only have a list of `Constant`. This > functions turns a list of `Constant` to a list of `XXStructor`. Just leave a record here, as we discussed offline, we agree that the meaning of term `XXStructorList` is `the initializer of `llvm.gloal_ctors` or `llvm.gloal_dtors` array. So I will keep using `preprocessXXStructorList` as the function name. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84534/new/ https://reviews.llvm.org/D84534 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D84534: [AIX] Static init frontend recovery and backend support
jasonliu added inline comments. Comment at: llvm/include/llvm/CodeGen/AsmPrinter.h:466 + + bool preprocessStructorList(const DataLayout , const Constant *List, + SmallVector ); Xiangling_L wrote: > jasonliu wrote: > > Xiangling_L wrote: > > > jasonliu wrote: > > > > A doxygen comment describe what this function does, and what its return > > > > value means, and mention `Structors` is an output argument. > > > > By looking at what this function does, it seems `buildStructorList` is > > > > a better name. > > > I meant to and should've named this function to > > > `preprocessXXStructorList`, let me know if you still prefer > > > `buildStructorList`. And if you do, since the underneath of `SmallVector` > > > is a variable-sized array, maybe we should try `buildSortedStructorArray`? > > `preprocess` sounds like we are already having a XXStructorList and now we > > try to do something on it. > > But right now, we are actually passing in an empty StructorList/Array and > > build it from scratch. So I would still prefer the name of `build` in it. > > I don't mind changing to a more accurate name as you suggested. > I think we do have a `XXStructorList` here which is the initializer of > `llvm.gloal_ctors`or `llvm.gloal_dtors` array? The usage of this term is > consistent with other spots. My understanding is that before we enter this `preprocessXXStructorList`, we do not have a list of XXStructor. We only have a list of `Constant`. This functions turns a list of `Constant` to a list of `XXStructor`. Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:2107 + if (!isa(List)) +return false; Xiangling_L wrote: > jasonliu wrote: > > Return of boolean seems unnecessary. > > Callee could check the size of the Structors to decide if they want an > > early return or not (or in this particular case, the for loop would just do > > nothing and no need for extra condition if you don't mind the call to > > getPointerPrefAlignment or assign to 0 to Index)? > > So we could just return void for this function? > Yeah, we could do that. But it looks a boolean return value makes the code > flow natural. And if any target does want to control the early return in the > future, it's flexbile for them to do that. I am wondering is there any big > difference between bool and void return value for this function? If target need to control early return in the future, they could still do so by querying if the output `Structors` is empty or not. I just don't want unnecessary returns, as it's one more thing that user of the function need to care about when they examine this function. And the user of this function would have the same question of why we need to return a boolean here. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84534/new/ https://reviews.llvm.org/D84534 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D84534: [AIX] Static init frontend recovery and backend support
Xiangling_L marked 5 inline comments as done. Xiangling_L added inline comments. Comment at: llvm/include/llvm/CodeGen/AsmPrinter.h:466 + + bool preprocessStructorList(const DataLayout , const Constant *List, + SmallVector ); jasonliu wrote: > Xiangling_L wrote: > > jasonliu wrote: > > > A doxygen comment describe what this function does, and what its return > > > value means, and mention `Structors` is an output argument. > > > By looking at what this function does, it seems `buildStructorList` is a > > > better name. > > I meant to and should've named this function to `preprocessXXStructorList`, > > let me know if you still prefer `buildStructorList`. And if you do, since > > the underneath of `SmallVector` is a variable-sized array, maybe we should > > try `buildSortedStructorArray`? > `preprocess` sounds like we are already having a XXStructorList and now we > try to do something on it. > But right now, we are actually passing in an empty StructorList/Array and > build it from scratch. So I would still prefer the name of `build` in it. > I don't mind changing to a more accurate name as you suggested. I think we do have a `XXStructorList` here which is the initializer of `llvm.gloal_ctors`or `llvm.gloal_dtors` array? The usage of this term is consistent with other spots. Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:2107 + if (!isa(List)) +return false; jasonliu wrote: > Return of boolean seems unnecessary. > Callee could check the size of the Structors to decide if they want an early > return or not (or in this particular case, the for loop would just do nothing > and no need for extra condition if you don't mind the call to > getPointerPrefAlignment or assign to 0 to Index)? > So we could just return void for this function? Yeah, we could do that. But it looks a boolean return value makes the code flow natural. And if any target does want to control the early return in the future, it's flexbile for them to do that. I am wondering is there any big difference between bool and void return value for this function? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84534/new/ https://reviews.llvm.org/D84534 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D84534: [AIX] Static init frontend recovery and backend support
jasonliu added inline comments.
Comment at: llvm/include/llvm/CodeGen/AsmPrinter.h:391
+ /// @param[out] Structors Sorted Structor structs by Priority.
+ /// @return false if List is not an array of '{ i32, void ()*, i8* }'
structs.
+ bool preprocessXXStructorList(const DataLayout , const Constant *List,
This description is not entirely true. We only see if the array is a
ConstantArray and returning false. We are not returning false if the array's
element is not `{ i32, void ()*, i8* }`.
Comment at: llvm/include/llvm/CodeGen/AsmPrinter.h:466
+
+ bool preprocessStructorList(const DataLayout , const Constant *List,
+ SmallVector );
Xiangling_L wrote:
> jasonliu wrote:
> > A doxygen comment describe what this function does, and what its return
> > value means, and mention `Structors` is an output argument.
> > By looking at what this function does, it seems `buildStructorList` is a
> > better name.
> I meant to and should've named this function to `preprocessXXStructorList`,
> let me know if you still prefer `buildStructorList`. And if you do, since the
> underneath of `SmallVector` is a variable-sized array, maybe we should try
> `buildSortedStructorArray`?
`preprocess` sounds like we are already having a XXStructorList and now we try
to do something on it.
But right now, we are actually passing in an empty StructorList/Array and build
it from scratch. So I would still prefer the name of `build` in it.
I don't mind changing to a more accurate name as you suggested.
Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:2107
+ if (!isa(List))
+return false;
Return of boolean seems unnecessary.
Callee could check the size of the Structors to decide if they want an early
return or not (or in this particular case, the for loop would just do nothing
and no need for extra condition if you don't mind the call to
getPointerPrefAlignment or assign to 0 to Index)?
So we could just return void for this function?
Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:2129
}
-
// Emit the function pointers in the target-specific order
llvm::stable_sort(Structors, [](const Structor , const Structor ) {
nit: Missing blank line.
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:1966
+
+Index++;
+ }
nit: Use `++Index` instead.
https://llvm.org/docs/CodingStandards.html#prefer-preincrement
Or use `Index++` at line 1963 so that we don't need this line.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D84534/new/
https://reviews.llvm.org/D84534
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D84534: [AIX] Static init frontend recovery and backend support
Xiangling_L updated this revision to Diff 283396.
Xiangling_L marked 2 inline comments as done.
Xiangling_L added a comment.
Added descriptions for struct and functions;
Addressed other comments;
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D84534/new/
https://reviews.llvm.org/D84534
Files:
clang/lib/CodeGen/CGDeclCXX.cpp
clang/lib/CodeGen/CodeGenModule.h
clang/lib/CodeGen/ItaniumCXXABI.cpp
clang/test/CodeGenCXX/aix-static-init-debug-info.cpp
clang/test/CodeGenCXX/aix-static-init-temp-spec-and-inline-var.cpp
clang/test/CodeGenCXX/aix-static-init.cpp
clang/unittests/CodeGen/IncrementalProcessingTest.cpp
llvm/include/llvm/CodeGen/AsmPrinter.h
llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp
llvm/test/CodeGen/PowerPC/aix-static-init-default-priority.ll
llvm/test/CodeGen/PowerPC/aix-static-init-key-object.ll
llvm/test/CodeGen/PowerPC/aix-static-init-no-unique-module-id.ll
llvm/test/CodeGen/PowerPC/aix-static-init-non-default-priority.ll
Index: llvm/test/CodeGen/PowerPC/aix-static-init-non-default-priority.ll
===
--- /dev/null
+++ llvm/test/CodeGen/PowerPC/aix-static-init-non-default-priority.ll
@@ -0,0 +1,10 @@
+; RUN: not llc -mtriple powerpc-ibm-aix-xcoff < %s 2>&1 | FileCheck %s
+; RUN: not llc -mtriple powerpc64-ibm-aix-xcoff < %s 2>&1 | FileCheck %s
+
+@llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 655, void ()* @foo, i8* null }]
+
+define void @foo() {
+ ret void
+}
+
+// CHECK: LLVM ERROR: prioritized sinit and sterm functions are not yet supported
Index: llvm/test/CodeGen/PowerPC/aix-static-init-no-unique-module-id.ll
===
--- /dev/null
+++ llvm/test/CodeGen/PowerPC/aix-static-init-no-unique-module-id.ll
@@ -0,0 +1,7 @@
+; RUN: not llc -mtriple powerpc-ibm-aix-xcoff < %s 2>&1 | FileCheck %s
+; RUN: not llc -mtriple powerpc64-ibm-aix-xcoff < %s 2>&1 | FileCheck %s
+
+@llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @foo, i8* null }]
+@llvm.global_dtors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @bar, i8* null }]
+
+// CHECK: LLVM ERROR: cannot produce a unique identifier for this module based on strong external symbols
Index: llvm/test/CodeGen/PowerPC/aix-static-init-key-object.ll
===
--- /dev/null
+++ llvm/test/CodeGen/PowerPC/aix-static-init-key-object.ll
@@ -0,0 +1,12 @@
+; RUN: not llc -mtriple powerpc-ibm-aix-xcoff < %s 2>&1 | FileCheck %s
+; RUN: not llc -mtriple powerpc64-ibm-aix-xcoff < %s 2>&1 | FileCheck %s
+
+@v = global i8 0
+
+@llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @foo, i8* @v}]
+
+define void @foo() {
+ ret void
+}
+
+// CHECK: LLVM ERROR: associated data of XXStructor list is not yet supported on AIX
Index: llvm/test/CodeGen/PowerPC/aix-static-init-default-priority.ll
===
--- /dev/null
+++ llvm/test/CodeGen/PowerPC/aix-static-init-default-priority.ll
@@ -0,0 +1,60 @@
+; RUN: llc -mtriple powerpc-ibm-aix-xcoff < %s | FileCheck %s
+; RUN: llc -mtriple powerpc64-ibm-aix-xcoff < %s | FileCheck %s
+
+@llvm.global_ctors = appending global [2 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @init1, i8* null }, { i32, void ()*, i8* } { i32 65535, void ()* @init2, i8* null }]
+@llvm.global_dtors = appending global [2 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @destruct1, i8* null }, { i32, void ()*, i8* } { i32 65535, void ()* @destruct2, i8* null }]
+
+define i32 @extFunc() {
+entry:
+ ret i32 3
+}
+
+define internal void @init1() {
+ ret void
+}
+
+define internal void @destruct1() {
+ ret void
+}
+
+define internal void @init2() {
+ ret void
+}
+
+define internal void @destruct2() {
+ ret void
+}
+
+; CHECK: .lglobl init1[DS]
+; CHECK: .lglobl .init1
+; CHECK: .csect init1[DS]
+; CHECK: __sinit8000_clang_ac404299654d2af7eae71e75c17f7c9b_0: # @init1
+; CHECK: .init1:
+; CHECK: .__sinit8000_clang_ac404299654d2af7eae71e75c17f7c9b_0:
+; CHECK: .lglobl destruct1[DS]
+; CHECK: .lglobl .destruct1
+; CHECK: .csect destruct1[DS]
+; CHECK: __sterm8000_clang_ac404299654d2af7eae71e75c17f7c9b_0: # @destruct1
+; CHECK: .destruct1:
+; CHECK: .__sterm8000_clang_ac404299654d2af7eae71e75c17f7c9b_0:
+; CHECK: .lglobl init2[DS]
+; CHECK: .lglobl .init2
+; CHECK: .csect init2[DS]
+; CHECK: __sinit8000_clang_ac404299654d2af7eae71e75c17f7c9b_1: # @init2
+; CHECK: .init2:
+; CHECK: .__sinit8000_clang_ac404299654d2af7eae71e75c17f7c9b_1:
+; CHECK: .lglobl
[PATCH] D84534: [AIX] Static init frontend recovery and backend support
Xiangling_L marked 7 inline comments as done.
Xiangling_L added inline comments.
Comment at: llvm/include/llvm/CodeGen/AsmPrinter.h:466
+
+ bool preprocessStructorList(const DataLayout , const Constant *List,
+ SmallVector );
jasonliu wrote:
> A doxygen comment describe what this function does, and what its return value
> means, and mention `Structors` is an output argument.
> By looking at what this function does, it seems `buildStructorList` is a
> better name.
I meant to and should've named this function to `preprocessXXStructorList`, let
me know if you still prefer `buildStructorList`. And if you do, since the
underneath of `SmallVector` is a variable-sized array, maybe we should try
`buildSortedStructorArray`?
Comment at: llvm/test/CodeGen/PowerPC/aix-static-init-key-object.ll:6
+
+@llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32,
void ()*, i8* } { i32 65535, void ()* @foo, i8* @v}]
+
jasonliu wrote:
> Adding this test case would looks like we already decided how to handle .ref
> in clang and llvm. But in fact, we haven't. I would prefer not having this
> test.
As we discussed offline, I would adjust the error message instead.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D84534/new/
https://reviews.llvm.org/D84534
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D84534: [AIX] Static init frontend recovery and backend support
jasonliu added inline comments.
Comment at: format:1
+//===-- PPCAsmPrinter.cpp - Print machine instrs to PowerPC assembly
--===//
+//
Redundant file?
Comment at: llvm/include/llvm/CodeGen/AsmPrinter.h:455
}
+ struct Structor {
The new block is not all `Overridable Hooks`, seems better belong in `Coarse
grained IR lowering routines`.
Comment at: llvm/include/llvm/CodeGen/AsmPrinter.h:456
+ struct Structor {
+int Priority = 0;
This struct got moved to header, means it's not an implementation detail any
more.
We would need doxygen comment on it.
Comment at: llvm/include/llvm/CodeGen/AsmPrinter.h:460
+// In most cases, `Key` represents a `ComdatKey`. Except on AIX, a `Key` is
+// used to identify a csect where we should emit `.ref` when needed.
+GlobalValue *Key = nullptr;
I have a slight preference to leave it as ComdatKey, and change the name when
we actually need to handle `.ref` because without seeing the actual
implementation for `.ref` we don't know if this is the way we want to pursue.
Comment at: llvm/include/llvm/CodeGen/AsmPrinter.h:466
+
+ bool preprocessStructorList(const DataLayout , const Constant *List,
+ SmallVector );
A doxygen comment describe what this function does, and what its return value
means, and mention `Structors` is an output argument.
By looking at what this function does, it seems `buildStructorList` is a better
name.
Comment at: llvm/include/llvm/CodeGen/AsmPrinter.h:468
+ SmallVector );
+ /// Targets can override this to change how `llvm.global_ctors` and
+ /// `llvm.global_dtors` lists get handled.
Add a blank line above this.
Comment at: llvm/test/CodeGen/PowerPC/aix-static-init-key-object.ll:6
+
+@llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32,
void ()*, i8* } { i32 65535, void ()* @foo, i8* @v}]
+
Adding this test case would looks like we already decided how to handle .ref in
clang and llvm. But in fact, we haven't. I would prefer not having this test.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D84534/new/
https://reviews.llvm.org/D84534
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D84534: [AIX] Static init frontend recovery and backend support
Xiangling_L added inline comments.
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:1865
+if (isSpecialLLVMGlobalArrayForStaticInit()) {
+ if (GlobalUniqueModuleId.empty()) {
+GlobalUniqueModuleId = getUniqueModuleId();
jasonliu wrote:
> We will need to move this part of the if statement to the overrided
> `emitXXStructorList` as well. (If we think overriding `emitXXStructorList`
> and calling `emitSpecialLLVMGlobal ` directly is a good idea.)
`getUniqueModuleId` takes `Module` as a parameter, if we want to invoke this
function inside of `emitXXStructorList` through `emitSpecialLLVMGlobal`, we
have to change the interface of them, which seems not ideal.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D84534/new/
https://reviews.llvm.org/D84534
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D84534: [AIX] Static init frontend recovery and backend support
Xiangling_L marked 6 inline comments as done.
Xiangling_L added inline comments.
Comment at: clang/lib/CodeGen/CodeGenModule.h:1058
+ /// Add an sterm finalizer to its own llvm.global_dtors entry.
+ void AddCXXStermFinalizerToGlobalDtor(llvm::Function *StermFinalizer,
+int Priority) {
jasonliu wrote:
> This wrapper seems redundant. Calling ` AddGlobalDtor(StermFinalizer,
> Priority);` directly from the callee side already convey what we are trying
> to achieve here.
I added this wrapper function to keep the symmetry between `AddGlobalCtor` and
`AddGlobalDtor`. They are private functions within `CodeGenModule` class. And I
feel it's kinda weird to only move `AddGlobalDtor` to a public function.
Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:4602
+llvm::report_fatal_error(
+"prioritized __sinit and __sterm functions are not yet supported");
+ else if (isTemplateInstantiation(D.getTemplateSpecializationKind()) ||
jasonliu wrote:
> Could we trigger this error? If so, could we have a test for it?
I should've put an assertion here. The init priority attribute has been
disabled on AIX in the previous patch.
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:1874
+ }
+ emitSpecialLLVMGlobal();
+ continue;
jasonliu wrote:
> Have some suggestion on structure backend code change:
>
> 1. Remove `isSpecialLLVMGlobalArrayForStaticInit` and
> `isSpecialLLVMGlobalArrayToSkip`, and call `emitSpecialLLVMGlobal` directly
> instead. This would handle all the `llvm.` variable for us. We would need do
> early return for names start with `llvm.` in emitGlobalVariable as well,
> since they are all handled here.
>
> 2. We could override emitXXStructorList because how XCOFF handle the list is
> vastly different than the other target. We could make the common part(i.e.
> processing and sorting) a utility function, say "preprocessStructorList".
>
> 3. It's not necessary to use the same name `emitXXStructor` if the interface
> is different. It might not provide much help when we kinda know no other
> target is going to use this interface. So we could turn it into a lambda
> inside of the overrided emitXXStructorList, or simply no need for a new
> function because the logic is fairly straightforward.
>
1. As we discussed offline, we would leave the handling of `llvm.used` and
`llvm.metadata` later and don't include them in the scope of this patch.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D84534/new/
https://reviews.llvm.org/D84534
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D84534: [AIX] Static init frontend recovery and backend support
jasonliu added inline comments.
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:1865
+if (isSpecialLLVMGlobalArrayForStaticInit()) {
+ if (GlobalUniqueModuleId.empty()) {
+GlobalUniqueModuleId = getUniqueModuleId();
We will need to move this part of the if statement to the overrided
`emitXXStructorList` as well. (If we think overriding `emitXXStructorList` and
calling `emitSpecialLLVMGlobal ` directly is a good idea.)
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D84534/new/
https://reviews.llvm.org/D84534
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D84534: [AIX] Static init frontend recovery and backend support
jasonliu added inline comments.
Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:24
#include "llvm/Support/Path.h"
#include "llvm/Transforms/Utils/ModuleUtils.h"
We are removing the usage of "getUniqueModuleId" in this file, So I assume this
include could get removed as well.
Comment at: clang/lib/CodeGen/CodeGenModule.h:1058
+ /// Add an sterm finalizer to its own llvm.global_dtors entry.
+ void AddCXXStermFinalizerToGlobalDtor(llvm::Function *StermFinalizer,
+int Priority) {
This wrapper seems redundant. Calling ` AddGlobalDtor(StermFinalizer,
Priority);` directly from the callee side already convey what we are trying to
achieve here.
Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:4602
+llvm::report_fatal_error(
+"prioritized __sinit and __sterm functions are not yet supported");
+ else if (isTemplateInstantiation(D.getTemplateSpecializationKind()) ||
Could we trigger this error? If so, could we have a test for it?
Comment at: llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp:2119
-unsigned Priority, const MCSymbol *KeySym) const {
- report_fatal_error("XCOFF dtor section not yet implemented.");
-}
I think it's still useful to keep these functions around to prevent
accidentally calling to these functions on AIX. We could rephrase error message
to "static constructor/destructor section is not needed on AIX."
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:1874
+ }
+ emitSpecialLLVMGlobal();
+ continue;
Have some suggestion on structure backend code change:
1. Remove `isSpecialLLVMGlobalArrayForStaticInit` and
`isSpecialLLVMGlobalArrayToSkip`, and call `emitSpecialLLVMGlobal` directly
instead. This would handle all the `llvm.` variable for us. We would need do
early return for names start with `llvm.` in emitGlobalVariable as well, since
they are all handled here.
2. We could override emitXXStructorList because how XCOFF handle the list is
vastly different than the other target. We could make the common part(i.e.
processing and sorting) a utility function, say "preprocessStructorList".
3. It's not necessary to use the same name `emitXXStructor` if the interface is
different. It might not provide much help when we kinda know no other target is
going to use this interface. So we could turn it into a lambda inside of the
overrided emitXXStructorList, or simply no need for a new function because the
logic is fairly straightforward.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D84534/new/
https://reviews.llvm.org/D84534
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D84534: [AIX] Static init frontend recovery and backend support
Xiangling_L updated this revision to Diff 281938.
Xiangling_L added a comment.
Removed the disablement in IncrementalProcessingTest.cpp cross-target test;
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D84534/new/
https://reviews.llvm.org/D84534
Files:
clang/lib/CodeGen/CGDeclCXX.cpp
clang/lib/CodeGen/CodeGenModule.h
clang/lib/CodeGen/ItaniumCXXABI.cpp
clang/test/CodeGenCXX/aix-static-init-debug-info.cpp
clang/test/CodeGenCXX/aix-static-init-temp-spec-and-inline-var.cpp
clang/test/CodeGenCXX/aix-static-init.cpp
clang/unittests/CodeGen/IncrementalProcessingTest.cpp
llvm/include/llvm/CodeGen/AsmPrinter.h
llvm/include/llvm/CodeGen/TargetLoweringObjectFileImpl.h
llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp
llvm/test/CodeGen/PowerPC/aix-static-init-default-priority.ll
llvm/test/CodeGen/PowerPC/aix-static-init-no-unique-module-id.ll
llvm/test/CodeGen/PowerPC/aix-static-init-non-default-priority.ll
Index: llvm/test/CodeGen/PowerPC/aix-static-init-non-default-priority.ll
===
--- /dev/null
+++ llvm/test/CodeGen/PowerPC/aix-static-init-non-default-priority.ll
@@ -0,0 +1,10 @@
+; RUN: not llc -mtriple powerpc-ibm-aix-xcoff < %s 2>&1 | FileCheck %s
+; RUN: not llc -mtriple powerpc64-ibm-aix-xcoff < %s 2>&1 | FileCheck %s
+
+@llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 655, void ()* @foo, i8* null }]
+
+define void @foo() {
+ ret void
+}
+
+// CHECK: LLVM ERROR: prioritized sinit and sterm functions are not yet supported
Index: llvm/test/CodeGen/PowerPC/aix-static-init-no-unique-module-id.ll
===
--- /dev/null
+++ llvm/test/CodeGen/PowerPC/aix-static-init-no-unique-module-id.ll
@@ -0,0 +1,7 @@
+; RUN: not llc -mtriple powerpc-ibm-aix-xcoff < %s 2>&1 | FileCheck %s
+; RUN: not llc -mtriple powerpc64-ibm-aix-xcoff < %s 2>&1 | FileCheck %s
+
+@llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @foo, i8* null }]
+@llvm.global_dtors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @bar, i8* null }]
+
+// CHECK: LLVM ERROR: cannot produce a unique identifier for this module based on strong external symbols
Index: llvm/test/CodeGen/PowerPC/aix-static-init-default-priority.ll
===
--- /dev/null
+++ llvm/test/CodeGen/PowerPC/aix-static-init-default-priority.ll
@@ -0,0 +1,60 @@
+; RUN: llc -mtriple powerpc-ibm-aix-xcoff < %s | FileCheck %s
+; RUN: llc -mtriple powerpc64-ibm-aix-xcoff < %s | FileCheck %s
+
+@llvm.global_ctors = appending global [2 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @init1, i8* null }, { i32, void ()*, i8* } { i32 65535, void ()* @init2, i8* null }]
+@llvm.global_dtors = appending global [2 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @destruct1, i8* null }, { i32, void ()*, i8* } { i32 65535, void ()* @destruct2, i8* null }]
+
+define i32 @extFunc() {
+entry:
+ ret i32 3
+}
+
+define internal void @init1() {
+ ret void
+}
+
+define internal void @destruct1() {
+ ret void
+}
+
+define internal void @init2() {
+ ret void
+}
+
+define internal void @destruct2() {
+ ret void
+}
+
+; CHECK: .lglobl init1[DS]
+; CHECK: .lglobl .init1
+; CHECK: .csect init1[DS]
+; CHECK: __sinit8000_clang_ac404299654d2af7eae71e75c17f7c9b_0: # @init1
+; CHECK: .init1:
+; CHECK: .__sinit8000_clang_ac404299654d2af7eae71e75c17f7c9b_0:
+; CHECK: .lglobl destruct1[DS]
+; CHECK: .lglobl .destruct1
+; CHECK: .csect destruct1[DS]
+; CHECK: __sterm8000_clang_ac404299654d2af7eae71e75c17f7c9b_0: # @destruct1
+; CHECK: .destruct1:
+; CHECK: .__sterm8000_clang_ac404299654d2af7eae71e75c17f7c9b_0:
+; CHECK: .lglobl init2[DS]
+; CHECK: .lglobl .init2
+; CHECK: .csect init2[DS]
+; CHECK: __sinit8000_clang_ac404299654d2af7eae71e75c17f7c9b_1: # @init2
+; CHECK: .init2:
+; CHECK: .__sinit8000_clang_ac404299654d2af7eae71e75c17f7c9b_1:
+; CHECK: .lglobl destruct2[DS]
+; CHECK: .lglobl .destruct2
+; CHECK: .csect destruct2[DS]
+; CHECK: __sterm8000_clang_ac404299654d2af7eae71e75c17f7c9b_1: # @destruct2
+; CHECK: .destruct2:
+; CHECK: .__sterm8000_clang_ac404299654d2af7eae71e75c17f7c9b_1:
+
+; CHECK: .globl __sinit8000_clang_ac404299654d2af7eae71e75c17f7c9b_0
+; CHECK: .globl .__sinit8000_clang_ac404299654d2af7eae71e75c17f7c9b_0
+; CHECK: .globl __sinit8000_clang_ac404299654d2af7eae71e75c17f7c9b_1
+; CHECK: .globl .__sinit8000_clang_ac404299654d2af7eae71e75c17f7c9b_1
+; CHECK: .globl __sterm8000_clang_ac404299654d2af7eae71e75c17f7c9b_0
+; CHECK: .globl
[PATCH] D84534: [AIX] Static init frontend recovery and backend support
Xiangling_L updated this revision to Diff 281240.
Xiangling_L marked 4 inline comments as done.
Xiangling_L added a comment.
Created alias for sinit and sterm;
Adjusted the testcase accordingly;
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D84534/new/
https://reviews.llvm.org/D84534
Files:
clang/lib/CodeGen/CGDeclCXX.cpp
clang/lib/CodeGen/CodeGenModule.h
clang/lib/CodeGen/ItaniumCXXABI.cpp
clang/test/CodeGenCXX/aix-static-init-debug-info.cpp
clang/test/CodeGenCXX/aix-static-init-temp-spec-and-inline-var.cpp
clang/test/CodeGenCXX/aix-static-init.cpp
llvm/include/llvm/CodeGen/AsmPrinter.h
llvm/include/llvm/CodeGen/TargetLoweringObjectFileImpl.h
llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp
llvm/test/CodeGen/PowerPC/aix-static-init-default-priority.ll
llvm/test/CodeGen/PowerPC/aix-static-init-no-unique-module-id.ll
llvm/test/CodeGen/PowerPC/aix-static-init-non-default-priority.ll
Index: llvm/test/CodeGen/PowerPC/aix-static-init-non-default-priority.ll
===
--- /dev/null
+++ llvm/test/CodeGen/PowerPC/aix-static-init-non-default-priority.ll
@@ -0,0 +1,10 @@
+; RUN: not llc -mtriple powerpc-ibm-aix-xcoff < %s 2>&1 | FileCheck %s
+; RUN: not llc -mtriple powerpc64-ibm-aix-xcoff < %s 2>&1 | FileCheck %s
+
+@llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 655, void ()* @foo, i8* null }]
+
+define void @foo() {
+ ret void
+}
+
+// CHECK: LLVM ERROR: prioritized sinit and sterm functions are not yet supported
Index: llvm/test/CodeGen/PowerPC/aix-static-init-no-unique-module-id.ll
===
--- /dev/null
+++ llvm/test/CodeGen/PowerPC/aix-static-init-no-unique-module-id.ll
@@ -0,0 +1,7 @@
+; RUN: not llc -mtriple powerpc-ibm-aix-xcoff < %s 2>&1 | FileCheck %s
+; RUN: not llc -mtriple powerpc64-ibm-aix-xcoff < %s 2>&1 | FileCheck %s
+
+@llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @foo, i8* null }]
+@llvm.global_dtors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @bar, i8* null }]
+
+// CHECK: LLVM ERROR: cannot produce a unique identifier for this module based on strong external symbols
Index: llvm/test/CodeGen/PowerPC/aix-static-init-default-priority.ll
===
--- /dev/null
+++ llvm/test/CodeGen/PowerPC/aix-static-init-default-priority.ll
@@ -0,0 +1,60 @@
+; RUN: llc -mtriple powerpc-ibm-aix-xcoff < %s | FileCheck %s
+; RUN: llc -mtriple powerpc64-ibm-aix-xcoff < %s | FileCheck %s
+
+@llvm.global_ctors = appending global [2 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @init1, i8* null }, { i32, void ()*, i8* } { i32 65535, void ()* @init2, i8* null }]
+@llvm.global_dtors = appending global [2 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @destruct1, i8* null }, { i32, void ()*, i8* } { i32 65535, void ()* @destruct2, i8* null }]
+
+define i32 @extFunc() {
+entry:
+ ret i32 3
+}
+
+define internal void @init1() {
+ ret void
+}
+
+define internal void @destruct1() {
+ ret void
+}
+
+define internal void @init2() {
+ ret void
+}
+
+define internal void @destruct2() {
+ ret void
+}
+
+; CHECK: .lglobl init1[DS]
+; CHECK: .lglobl .init1
+; CHECK: .csect init1[DS]
+; CHECK: __sinit8000_clang_ac404299654d2af7eae71e75c17f7c9b_0: # @init1
+; CHECK: .init1:
+; CHECK: .__sinit8000_clang_ac404299654d2af7eae71e75c17f7c9b_0:
+; CHECK: .lglobl destruct1[DS]
+; CHECK: .lglobl .destruct1
+; CHECK: .csect destruct1[DS]
+; CHECK: __sterm8000_clang_ac404299654d2af7eae71e75c17f7c9b_0: # @destruct1
+; CHECK: .destruct1:
+; CHECK: .__sterm8000_clang_ac404299654d2af7eae71e75c17f7c9b_0:
+; CHECK: .lglobl init2[DS]
+; CHECK: .lglobl .init2
+; CHECK: .csect init2[DS]
+; CHECK: __sinit8000_clang_ac404299654d2af7eae71e75c17f7c9b_1: # @init2
+; CHECK: .init2:
+; CHECK: .__sinit8000_clang_ac404299654d2af7eae71e75c17f7c9b_1:
+; CHECK: .lglobl destruct2[DS]
+; CHECK: .lglobl .destruct2
+; CHECK: .csect destruct2[DS]
+; CHECK: __sterm8000_clang_ac404299654d2af7eae71e75c17f7c9b_1: # @destruct2
+; CHECK: .destruct2:
+; CHECK: .__sterm8000_clang_ac404299654d2af7eae71e75c17f7c9b_1:
+
+; CHECK: .globl __sinit8000_clang_ac404299654d2af7eae71e75c17f7c9b_0
+; CHECK: .globl .__sinit8000_clang_ac404299654d2af7eae71e75c17f7c9b_0
+; CHECK: .globl __sinit8000_clang_ac404299654d2af7eae71e75c17f7c9b_1
+; CHECK: .globl .__sinit8000_clang_ac404299654d2af7eae71e75c17f7c9b_1
+; CHECK: .globl __sterm8000_clang_ac404299654d2af7eae71e75c17f7c9b_0
+; CHECK: .globl .__sterm8000_clang_ac404299654d2af7eae71e75c17f7c9b_0
[PATCH] D84534: [AIX] Static init frontend recovery and backend support
Xiangling_L marked 5 inline comments as done.
Xiangling_L added inline comments.
Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:4609
+// their own llvm.global_dtors entry.
+CGM.AddCXXStermFinalizerToGlobalDtor(StermFinalizer, 65535);
+ else
jasonliu wrote:
> Handling template instantiation seems fairly orthogonal to "moving naming
> logic from frontend to backend". Could we put it in a separate patch (which
> could be a child of this one)?
The reason I chose to handle template instantiation and inline variable in this
patch is that I want to show the scenarios where we have separate
initialization. And this is also the reason why we want to embed array index
into sinit/sterm functions. That is, if we move this part into a separate
patch, I am worried that ppl will feel it's weird to embed index into function
names.
Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:2165
+ emitXXStructor(DL, S.Priority, index, S.Func, isCtor);
+ ++index;
+ continue;
jasonliu wrote:
> Maybe a naive quesiton, in what situation would we have name collision and
> need `index` as suffix to differentiate?
> Could we just report_fatal_error in those situation?
As far as I know, there are several situations we would have separate
initialization without considering priority:
1) template specialization
2) inline variable
3) ctor/dtor attribute functions
By embedding the index, we can group those sinit/sterm with same priority
within current file together.
And as I mentioned above, I chose to handle the former two cases in this patch
to show why we need to embed the index.
And regarding the third scenario, we've already report_fatal_error on those
cases.
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:1848
+// beforing emitting them.
+if (isSpecialLLVMGlobalArrayForStaticInit()) {
+ if (GlobalUniqueModuleId.empty()) {
jasonliu wrote:
> This would have conflict with D84363. You might want to rebase it later.
Thanks for the reminder, I will update the patch accordingly.
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:1935
+ Function *Func = cast(CV);
+ Func->setLinkage(GlobalValue::ExternalLinkage);
+ Func->setName((isCtor ? llvm::Twine("__sinit") : llvm::Twine("__sterm")) +
jasonliu wrote:
> Changing Function name and linkage underneath looks a bit scary. People would
> have a hard time to map IR to final assembly that gets produced. Have you
> thought about creating an alias (with the correct linkage and name) to the
> original function instead?
Good point. I will adjust the implementation here.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D84534/new/
https://reviews.llvm.org/D84534
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D84534: [AIX] Static init frontend recovery and backend support
jasonliu added inline comments.
Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:4609
+// their own llvm.global_dtors entry.
+CGM.AddCXXStermFinalizerToGlobalDtor(StermFinalizer, 65535);
+ else
Handling template instantiation seems fairly orthogonal to "moving naming logic
from frontend to backend". Could we put it in a separate patch (which could be
a child of this one)?
Comment at: llvm/include/llvm/CodeGen/AsmPrinter.h:460
+ virtual void emitXXStructor(const DataLayout , const int Priority,
+ const unsigned index, Constant *CV, bool isCtor)
{
+llvm_unreachable("Emit CXXStructor with priority is target-specific.");
Remove `const` from value type (e.g. Priority and index), as there is no real
need for it.
Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:2165
+ emitXXStructor(DL, S.Priority, index, S.Func, isCtor);
+ ++index;
+ continue;
Maybe a naive quesiton, in what situation would we have name collision and need
`index` as suffix to differentiate?
Could we just report_fatal_error in those situation?
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:1848
+// beforing emitting them.
+if (isSpecialLLVMGlobalArrayForStaticInit()) {
+ if (GlobalUniqueModuleId.empty()) {
This would have conflict with D84363. You might want to rebase it later.
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:1935
+ Function *Func = cast(CV);
+ Func->setLinkage(GlobalValue::ExternalLinkage);
+ Func->setName((isCtor ? llvm::Twine("__sinit") : llvm::Twine("__sterm")) +
Changing Function name and linkage underneath looks a bit scary. People would
have a hard time to map IR to final assembly that gets produced. Have you
thought about creating an alias (with the correct linkage and name) to the
original function instead?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D84534/new/
https://reviews.llvm.org/D84534
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D84534: [AIX] Static init frontend recovery and backend support
Xiangling_L updated this revision to Diff 280897.
Xiangling_L added a comment.
Fix clang-tidy errors;
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D84534/new/
https://reviews.llvm.org/D84534
Files:
clang/lib/CodeGen/CGDeclCXX.cpp
clang/lib/CodeGen/CodeGenModule.h
clang/lib/CodeGen/ItaniumCXXABI.cpp
clang/test/CodeGenCXX/aix-static-init-debug-info.cpp
clang/test/CodeGenCXX/aix-static-init-temp-spec-and-inline-var.cpp
clang/test/CodeGenCXX/aix-static-init.cpp
llvm/include/llvm/CodeGen/AsmPrinter.h
llvm/include/llvm/CodeGen/TargetLoweringObjectFileImpl.h
llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp
llvm/test/CodeGen/PowerPC/aix-static-init-default-priority.ll
llvm/test/CodeGen/PowerPC/aix-static-init-no-unique-module-id.ll
llvm/test/CodeGen/PowerPC/aix-static-init-non-default-priority.ll
Index: llvm/test/CodeGen/PowerPC/aix-static-init-non-default-priority.ll
===
--- /dev/null
+++ llvm/test/CodeGen/PowerPC/aix-static-init-non-default-priority.ll
@@ -0,0 +1,10 @@
+; RUN: not llc -mtriple powerpc-ibm-aix-xcoff < %s 2>&1 | FileCheck %s
+; RUN: not llc -mtriple powerpc64-ibm-aix-xcoff < %s 2>&1 | FileCheck %s
+
+@llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 655, void ()* @foo, i8* null }]
+
+define void @foo() {
+ ret void
+}
+
+// CHECK: LLVM ERROR: prioritized sinit and sterm functions are not yet supported
Index: llvm/test/CodeGen/PowerPC/aix-static-init-no-unique-module-id.ll
===
--- /dev/null
+++ llvm/test/CodeGen/PowerPC/aix-static-init-no-unique-module-id.ll
@@ -0,0 +1,7 @@
+; RUN: not llc -mtriple powerpc-ibm-aix-xcoff < %s 2>&1 | FileCheck %s
+; RUN: not llc -mtriple powerpc64-ibm-aix-xcoff < %s 2>&1 | FileCheck %s
+
+@llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @foo, i8* null }]
+@llvm.global_dtors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @bar, i8* null }]
+
+// CHECK: LLVM ERROR: cannot produce a unique identifier for this module based on strong external symbols
Index: llvm/test/CodeGen/PowerPC/aix-static-init-default-priority.ll
===
--- /dev/null
+++ llvm/test/CodeGen/PowerPC/aix-static-init-default-priority.ll
@@ -0,0 +1,31 @@
+; RUN: llc -mtriple powerpc-ibm-aix-xcoff < %s | FileCheck %s
+; RUN: llc -mtriple powerpc64-ibm-aix-xcoff < %s | FileCheck %s
+
+@llvm.global_ctors = appending global [2 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @init1, i8* null }, { i32, void ()*, i8* } { i32 65535, void ()* @init2, i8* null }]
+@llvm.global_dtors = appending global [2 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @destruct1, i8* null }, { i32, void ()*, i8* } { i32 65535, void ()* @destruct2, i8* null }]
+
+define i32 @extFunc() {
+entry:
+ ret i32 3
+}
+
+define internal void @init1() {
+ ret void
+}
+
+define internal void @destruct1() {
+ ret void
+}
+
+define internal void @init2() {
+ ret void
+}
+
+define internal void @destruct2() {
+ ret void
+}
+
+; CHECK: .globl .__sinit8000_clang_ac404299654d2af7eae71e75c17f7c9b_0
+; CHECK: .globl .__sterm8000_clang_ac404299654d2af7eae71e75c17f7c9b_0
+; CHECK: .globl .__sinit8000_clang_ac404299654d2af7eae71e75c17f7c9b_1
+; CHECK: .globl .__sterm8000_clang_ac404299654d2af7eae71e75c17f7c9b_1
Index: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp
===
--- llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp
+++ llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp
@@ -67,6 +67,7 @@
#include "llvm/Support/TargetRegistry.h"
#include "llvm/Support/raw_ostream.h"
#include "llvm/Target/TargetMachine.h"
+#include "llvm/Transforms/Utils/ModuleUtils.h"
#include
#include
#include
@@ -153,6 +154,9 @@
/// linkage for them in AIX.
SmallPtrSet ExtSymSDNodeSymbols;
+ /// A unique trailing identifier as a part of sinit/sterm functions.
+ std::string GlobalUniqueModuleId;
+
static void ValidateGV(const GlobalVariable *GV);
// Record a list of GlobalAlias associated with a GlobalObject.
// This is used for AIX's extra-label-at-definition aliasing strategy.
@@ -171,6 +175,9 @@
bool doInitialization(Module ) override;
+ void emitXXStructor(const DataLayout , const int Priority,
+ const unsigned Index, Constant *CV, bool IsCtor) override;
+
void SetupMachineFunction(MachineFunction ) override;
void emitGlobalVariable(const GlobalVariable *GV) override;
@@ -1687,10 +1694,7 @@
void PPCAIXAsmPrinter::emitGlobalVariable(const GlobalVariable *GV) {
ValidateGV(GV);
- // TODO: Update the handling of global arrays for static
[PATCH] D84534: [AIX] Static init frontend recovery and backend support
Xiangling_L created this revision.
Xiangling_L added reviewers: jasonliu, hubert.reinterpretcast, yusra.syeda,
zarko, xingxue.
Xiangling_L added a project: LLVM.
Herald added subscribers: llvm-commits, cfe-commits, jfb, kbarton, hiraditya,
nemanjai.
Herald added a project: clang.
1. Frontend side
2. Recovered AIX static init frontend to use the linkage type and function
names Clang chooses for sinit related function;
3. Removed the `GlobalUniqueModuleId` calculation and usage;
4. Adjusted the FE testcases accordingly;
5. Added one frontend testcase to demonstrate and validate separate
initialization on AIX;
2. Backend side on the assembly path only
3. Set correct linkage and function names for sinit/sterm functions
4. Added testcases
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D84534
Files:
clang/lib/CodeGen/CGDeclCXX.cpp
clang/lib/CodeGen/CodeGenModule.h
clang/lib/CodeGen/ItaniumCXXABI.cpp
clang/test/CodeGenCXX/aix-static-init-debug-info.cpp
clang/test/CodeGenCXX/aix-static-init-temp-spec-and-inline-var.cpp
clang/test/CodeGenCXX/aix-static-init.cpp
llvm/include/llvm/CodeGen/AsmPrinter.h
llvm/include/llvm/CodeGen/TargetLoweringObjectFileImpl.h
llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp
llvm/test/CodeGen/PowerPC/aix-static-init-default-priority.ll
llvm/test/CodeGen/PowerPC/aix-static-init-no-unique-module-id.ll
llvm/test/CodeGen/PowerPC/aix-static-init-non-default-priority.ll
Index: llvm/test/CodeGen/PowerPC/aix-static-init-non-default-priority.ll
===
--- /dev/null
+++ llvm/test/CodeGen/PowerPC/aix-static-init-non-default-priority.ll
@@ -0,0 +1,10 @@
+; RUN: not llc -mtriple powerpc-ibm-aix-xcoff < %s 2>&1 | FileCheck %s
+; RUN: not llc -mtriple powerpc64-ibm-aix-xcoff < %s 2>&1 | FileCheck %s
+
+@llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 655, void ()* @foo, i8* null }]
+
+define void @foo() {
+ ret void
+}
+
+// CHECK: LLVM ERROR: prioritized sinit and sterm functions are not yet supported
Index: llvm/test/CodeGen/PowerPC/aix-static-init-no-unique-module-id.ll
===
--- /dev/null
+++ llvm/test/CodeGen/PowerPC/aix-static-init-no-unique-module-id.ll
@@ -0,0 +1,7 @@
+; RUN: not llc -mtriple powerpc-ibm-aix-xcoff < %s 2>&1 | FileCheck %s
+; RUN: not llc -mtriple powerpc64-ibm-aix-xcoff < %s 2>&1 | FileCheck %s
+
+@llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @foo, i8* null }]
+@llvm.global_dtors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @bar, i8* null }]
+
+// CHECK: LLVM ERROR: cannot produce a unique identifier for this module based on strong external symbols
Index: llvm/test/CodeGen/PowerPC/aix-static-init-default-priority.ll
===
--- /dev/null
+++ llvm/test/CodeGen/PowerPC/aix-static-init-default-priority.ll
@@ -0,0 +1,31 @@
+; RUN: llc -mtriple powerpc-ibm-aix-xcoff < %s | FileCheck %s
+; RUN: llc -mtriple powerpc64-ibm-aix-xcoff < %s | FileCheck %s
+
+@llvm.global_ctors = appending global [2 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @init1, i8* null }, { i32, void ()*, i8* } { i32 65535, void ()* @init2, i8* null }]
+@llvm.global_dtors = appending global [2 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @destruct1, i8* null }, { i32, void ()*, i8* } { i32 65535, void ()* @destruct2, i8* null }]
+
+define i32 @extFunc() {
+entry:
+ ret i32 3
+}
+
+define internal void @init1() {
+ ret void
+}
+
+define internal void @destruct1() {
+ ret void
+}
+
+define internal void @init2() {
+ ret void
+}
+
+define internal void @destruct2() {
+ ret void
+}
+
+; CHECK: .globl .__sinit8000_clang_ac404299654d2af7eae71e75c17f7c9b_0
+; CHECK: .globl .__sterm8000_clang_ac404299654d2af7eae71e75c17f7c9b_0
+; CHECK: .globl .__sinit8000_clang_ac404299654d2af7eae71e75c17f7c9b_1
+; CHECK: .globl .__sterm8000_clang_ac404299654d2af7eae71e75c17f7c9b_1
Index: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp
===
--- llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp
+++ llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp
@@ -67,6 +67,7 @@
#include "llvm/Support/TargetRegistry.h"
#include "llvm/Support/raw_ostream.h"
#include "llvm/Target/TargetMachine.h"
+#include "llvm/Transforms/Utils/ModuleUtils.h"
#include
#include
#include
@@ -153,6 +154,9 @@
/// linkage for them in AIX.
SmallPtrSet ExtSymSDNodeSymbols;
+ /// A unique trailing identifier as a part of sinit/sterm functions.
+ std::string GlobalUniqueModuleId;
+
static void ValidateGV(const GlobalVariable *GV);
// Record a list of
