[PATCH] D74166: [AIX][Frontend] Static init implementation for AIX considering no priority

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi added a comment.

In D74166#2107831 , @plotfi wrote:

> @rjmccall
>
> FYI this diff appears to be breaking a ptrauth test on 
> apple/swift/master-next 
> (clang/test/CodeGenCXX/ptrauth-static-destructors.cpp).
>
> Investigating this further.


Looks like this was addressed in:

https://github.com/apple/llvm-project/commit/8e8176c0bee7215b397d78cfff6e617cfc50e248

and 
https://github.com/apple/llvm-project/commit/4292b7edfcf0672da076a0b4e3d4fde8b2672359#diff-75a7118d42116297f6a0b1a0fd020604


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74166/new/

https://reviews.llvm.org/D74166



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D74166: [AIX][Frontend] Static init implementation for AIX considering no priority

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi added subscribers: rjmccall, plotfi.
plotfi added a comment.

@rjmccall

FYI this diff appears to be breaking a ptrauth test on apple/swift/master-next 
(clang/test/CodeGenCXX/ptrauth-static-destructors.cpp).

Investigating this further.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74166/new/

https://reviews.llvm.org/D74166



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D74166: [AIX][Frontend] Static init implementation for AIX considering no priority

[Xiangling Liao via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Xiangling_L marked an inline comment as done.
Closed by commit rG22337bfe7d87: [AIX][Frontend] Static init implementation for 
AIX considering no priority (authored by Xiangling_L).

Changed prior to commit:
  https://reviews.llvm.org/D74166?vs=271747=272035#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74166/new/

https://reviews.llvm.org/D74166

Files:
  clang/include/clang/AST/Mangle.h
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/CodeGen/CGCXXABI.h
  clang/lib/CodeGen/CGDeclCXX.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/CodeGen/MicrosoftCXXABI.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGen/aix-constructor-attribute.cpp
  clang/test/CodeGen/aix-destructor-attribute.cpp
  clang/test/CodeGen/aix-init-priority-attribute.cpp
  clang/test/CodeGen/static-init.cpp
  clang/test/CodeGenCXX/aix-static-init.cpp

Index: clang/test/CodeGenCXX/aix-static-init.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/aix-static-init.cpp
@@ -0,0 +1,193 @@
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -S -emit-llvm -x c++ \
+// RUN: -fno-use-cxa-atexit -std=c++2a < %s | \
+// RUN:   FileCheck --check-prefixes=CHECK,CHECK32 %s
+
+// RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff -S -emit-llvm -x c++ \
+// RUN: -fno-use-cxa-atexit -std=c++2a < %s | \
+// RUN:   FileCheck --check-prefixes=CHECK,CHECK64 %s
+
+namespace test1 {
+  struct Test1 {
+Test1();
+~Test1();
+  } t1, t2;
+} // namespace test1
+
+namespace test2 {
+  int foo() { return 3; }
+  int x = foo();
+} // namespace test2
+
+namespace test3 {
+  struct Test3 {
+constexpr Test3() {};
+~Test3() {};
+  };
+
+  constinit Test3 t;
+} // namespace test3
+
+namespace test4 {
+  struct Test4 {
+Test4();
+~Test4();
+  };
+
+  void f() {
+static Test4 staticLocal;
+  }
+} // namespace test4
+
+// CHECK: @llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @__sinit8000_clang_1145401da454a7baad10bfe313c46638, i8* null }]
+// CHECK: @llvm.global_dtors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @__sterm8000_clang_1145401da454a7baad10bfe313c46638, i8* null }]
+
+// CHECK: define internal void @__cxx_global_var_init() [[ATTR:#[0-9]+]] {
+// CHECK: entry:
+// CHECK:   call void @_ZN5test15Test1C1Ev(%"struct.test1::Test1"* @_ZN5test12t1E)
+// CHECK:   %0 = call i32 @atexit(void ()* @__dtor__ZN5test12t1E)
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: define internal void @__dtor__ZN5test12t1E() [[ATTR:#[0-9]+]] {
+// CHECK: entry:
+// CHECK:   call void @_ZN5test15Test1D1Ev(%"struct.test1::Test1"* @_ZN5test12t1E)
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: declare i32 @atexit(void ()*)
+
+// CHECK: define internal void @__finalize__ZN5test12t1E() [[ATTR:#[0-9]+]] {
+// CHECK: entry:
+// CHECK:   %0 = call i32 @unatexit(void ()* @__dtor__ZN5test12t1E)
+// CHECK:   %needs_destruct = icmp eq i32 %0, 0
+// CHECK:   br i1 %needs_destruct, label %destruct.call, label %destruct.end
+
+// CHECK: destruct.call:
+// CHECK:   call void @__dtor__ZN5test12t1E()
+// CHECK:   br label %destruct.end
+
+// CHECK: destruct.end:
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: declare i32 @unatexit(void ()*)
+
+// CHECK: define internal void @__cxx_global_var_init.1() [[ATTR:#[0-9]+]] {
+// CHECK: entry:
+// CHECK:   call void @_ZN5test15Test1C1Ev(%"struct.test1::Test1"* @_ZN5test12t2E)
+// CHECK:   %0 = call i32 @atexit(void ()* @__dtor__ZN5test12t2E)
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: define internal void @__dtor__ZN5test12t2E() [[ATTR:#[0-9]+]] {
+// CHECK: entry:
+// CHECK:   call void @_ZN5test15Test1D1Ev(%"struct.test1::Test1"* @_ZN5test12t2E)
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: define internal void @__finalize__ZN5test12t2E() [[ATTR:#[0-9]+]] {
+// CHECK: entry:
+// CHECK:   %0 = call i32 @unatexit(void ()* @__dtor__ZN5test12t2E)
+// CHECK:   %needs_destruct = icmp eq i32 %0, 0
+// CHECK:   br i1 %needs_destruct, label %destruct.call, label %destruct.end
+
+// CHECK: destruct.call:
+// CHECK:   call void @__dtor__ZN5test12t2E()
+// CHECK:   br label %destruct.end
+
+// CHECK: destruct.end:
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: define internal void @__cxx_global_var_init.2() [[ATTR:#[0-9]+]] {
+// CHECK: entry:
+// CHECK32: %call = call i32 @_ZN5test23fooEv()
+// CHECK64: %call = call signext i32 @_ZN5test23fooEv()
+// CHECK:   store i32 %call, i32* @_ZN5test21xE
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: define internal void @__cxx_global_var_init.3() [[ATTR:#[0-9]+]] {
+// CHECK: entry:
+// CHECK:   %0 = call i32 @atexit(void ()* 

[PATCH] D74166: [AIX][Frontend] Static init implementation for AIX considering no priority

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast accepted this revision.
hubert.reinterpretcast marked an inline comment as done.
hubert.reinterpretcast added a comment.

Thanks; LGTM with minor nit.




Comment at: clang/lib/CodeGen/CodeGenModule.h:1060
+
+  /// Add a sterm finalizer to the C++ global cleanup function.
+  void AddCXXStermFinalizerEntry(llvm::FunctionCallee DtorFn) {

Minor nit: "a sterm" should be "an sterm" if "sterm" is pronounced like 
"es-term".




Comment at: clang/test/CodeGenCXX/aix-static-init.cpp:188
+// CHECK: entry:
+// CHECK:   call void @__finalize__ZZN5test41fEvE11staticLocal()
+// CHECK:   call void @__finalize__ZN5test31tE()

Just a comment: The finalization order of static locals in relation to 
non-locals cannot (using the mechanisms available) be guaranteed to be 
consistent with the reverse order of initialization. I don't think that there 
is a clearly "correct" order, so "whatever falls out" is probably okay.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74166/new/

https://reviews.llvm.org/D74166



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D74166: [AIX][Frontend] Static init implementation for AIX considering no priority

[Jason Liu via Phabricator via cfe-commits]

jasonliu accepted this revision.
jasonliu added a comment.
This revision is now accepted and ready to land.

LGTM. But I suggest to wait for @hubert.reinterpretcast to have another scan at 
this before landing.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74166/new/

https://reviews.llvm.org/D74166



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D74166: [AIX][Frontend] Static init implementation for AIX considering no priority

[Xiangling Liao via Phabricator via cfe-commits]

Xiangling_L added inline comments.



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:345
+// rarely.
+Weights = nullptr;
+  } else if (Kind == GuardKind::VariableGuard && !D->isLocalVarDecl()) {

jasonliu wrote:
> Do we need to change/complicate the interface for this function, just to do a 
> call to Builder.CreateCondBr()?
> Could we call that function directly from where it's needed?
Sure, we can. Thank you for your suggestion. I was hoping to use one function 
to synthesize the guarded init or destruct branch. But I think it seems better 
if we wait for further more usage of guarded destruct branch to do so and not 
complicate stuff in this patch.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74166/new/

https://reviews.llvm.org/D74166



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D74166: [AIX][Frontend] Static init implementation for AIX considering no priority

[Xiangling Liao via Phabricator via cfe-commits]

Xiangling_L updated this revision to Diff 271747.
Xiangling_L marked 3 inline comments as done.
Xiangling_L added a comment.

Removed a redundant header file;
Addressed comments;


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74166/new/

https://reviews.llvm.org/D74166

Files:
  clang/include/clang/AST/Mangle.h
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/CodeGen/CGCXXABI.h
  clang/lib/CodeGen/CGDeclCXX.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/CodeGen/MicrosoftCXXABI.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGen/aix-constructor-attribute.cpp
  clang/test/CodeGen/aix-destructor-attribute.cpp
  clang/test/CodeGen/aix-init-priority-attribute.cpp
  clang/test/CodeGen/static-init.cpp
  clang/test/CodeGenCXX/aix-static-init.cpp

Index: clang/test/CodeGenCXX/aix-static-init.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/aix-static-init.cpp
@@ -0,0 +1,193 @@
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -S -emit-llvm -x c++ \
+// RUN: -fno-use-cxa-atexit -std=c++2a < %s | \
+// RUN:   FileCheck --check-prefixes=CHECK,CHECK32 %s
+
+// RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff -S -emit-llvm -x c++ \
+// RUN: -fno-use-cxa-atexit -std=c++2a < %s | \
+// RUN:   FileCheck --check-prefixes=CHECK,CHECK64 %s
+
+namespace test1 {
+  struct Test1 {
+Test1();
+~Test1();
+  } t1, t2;
+} // namespace test1
+
+namespace test2 {
+  int foo() { return 3; }
+  int x = foo();
+} // namespace test2
+
+namespace test3 {
+  struct Test3 {
+constexpr Test3() {};
+~Test3() {};
+  };
+
+  constinit Test3 t;
+} // namespace test3
+
+namespace test4 {
+  struct Test4 {
+Test4();
+~Test4();
+  };
+
+  void f() {
+static Test4 staticLocal;
+  }
+} // namespace test4
+
+// CHECK: @llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @__sinit8000_clang_1145401da454a7baad10bfe313c46638, i8* null }]
+// CHECK: @llvm.global_dtors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @__sterm8000_clang_1145401da454a7baad10bfe313c46638, i8* null }]
+
+// CHECK: define internal void @__cxx_global_var_init() [[ATTR:#[0-9]+]] {
+// CHECK: entry:
+// CHECK:   call void @_ZN5test15Test1C1Ev(%"struct.test1::Test1"* @_ZN5test12t1E)
+// CHECK:   %0 = call i32 @atexit(void ()* @__dtor__ZN5test12t1E)
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: define internal void @__dtor__ZN5test12t1E() [[ATTR:#[0-9]+]] {
+// CHECK: entry:
+// CHECK:   call void @_ZN5test15Test1D1Ev(%"struct.test1::Test1"* @_ZN5test12t1E)
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: declare i32 @atexit(void ()*)
+
+// CHECK: define internal void @__finalize__ZN5test12t1E() [[ATTR:#[0-9]+]] {
+// CHECK: entry:
+// CHECK:   %0 = call i32 @unatexit(void ()* @__dtor__ZN5test12t1E)
+// CHECK:   %needs_destruct = icmp eq i32 %0, 0
+// CHECK:   br i1 %needs_destruct, label %destruct.call, label %destruct.end
+
+// CHECK: destruct.call:
+// CHECK:   call void @__dtor__ZN5test12t1E()
+// CHECK:   br label %destruct.end
+
+// CHECK: destruct.end:
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: declare i32 @unatexit(void ()*)
+
+// CHECK: define internal void @__cxx_global_var_init.1() [[ATTR:#[0-9]+]] {
+// CHECK: entry:
+// CHECK:   call void @_ZN5test15Test1C1Ev(%"struct.test1::Test1"* @_ZN5test12t2E)
+// CHECK:   %0 = call i32 @atexit(void ()* @__dtor__ZN5test12t2E)
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: define internal void @__dtor__ZN5test12t2E() [[ATTR:#[0-9]+]] {
+// CHECK: entry:
+// CHECK:   call void @_ZN5test15Test1D1Ev(%"struct.test1::Test1"* @_ZN5test12t2E)
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: define internal void @__finalize__ZN5test12t2E() [[ATTR:#[0-9]+]] {
+// CHECK: entry:
+// CHECK:   %0 = call i32 @unatexit(void ()* @__dtor__ZN5test12t2E)
+// CHECK:   %needs_destruct = icmp eq i32 %0, 0
+// CHECK:   br i1 %needs_destruct, label %destruct.call, label %destruct.end
+
+// CHECK: destruct.call:
+// CHECK:   call void @__dtor__ZN5test12t2E()
+// CHECK:   br label %destruct.end
+
+// CHECK: destruct.end:
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: define internal void @__cxx_global_var_init.2() [[ATTR:#[0-9]+]] {
+// CHECK: entry:
+// CHECK32: %call = call i32 @_ZN5test23fooEv()
+// CHECK64: %call = call signext i32 @_ZN5test23fooEv()
+// CHECK:   store i32 %call, i32* @_ZN5test21xE
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: define internal void @__cxx_global_var_init.3() [[ATTR:#[0-9]+]] {
+// CHECK: entry:
+// CHECK:   %0 = call i32 @atexit(void ()* @__dtor__ZN5test31tE)
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: define internal void @__dtor__ZN5test31tE() [[ATTR:#[0-9]+]] {
+// CHECK: entry:
+// CHECK:   call void 

[PATCH] D74166: [AIX][Frontend] Static init implementation for AIX considering no priority

[Jason Liu via Phabricator via cfe-commits]

jasonliu added inline comments.



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:345
+// rarely.
+Weights = nullptr;
+  } else if (Kind == GuardKind::VariableGuard && !D->isLocalVarDecl()) {

Do we need to change/complicate the interface for this function, just to do a 
call to Builder.CreateCondBr()?
Could we call that function directly from where it's needed?



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:648
 
-  // Include the filename in the symbol name. Including "sub_" matches gcc and
-  // makes sure these symbols appear lexicographically behind the symbols with
-  // priority emitted above.
-  SmallString<128> FileName = llvm::sys::path::filename(getModule().getName());
-  if (FileName.empty())
-FileName = "";
+  // Create our global initialization function.
+  if (UseSinitAndSterm && CXXGlobalInits.empty())

This comment does not apply well on top of this early return for some platform. 
I think you could move it to current line 665?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74166/new/

https://reviews.llvm.org/D74166



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D74166: [AIX][Frontend] Static init implementation for AIX considering no priority

[Xiangling Liao via Phabricator via cfe-commits]

Xiangling_L updated this revision to Diff 271668.
Xiangling_L added a comment.

Fix the previous bad version;


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74166/new/

https://reviews.llvm.org/D74166

Files:
  clang/include/clang/AST/Mangle.h
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/CodeGen/CGCXXABI.h
  clang/lib/CodeGen/CGDeclCXX.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/CodeGen/MicrosoftCXXABI.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGen/aix-constructor-attribute.cpp
  clang/test/CodeGen/aix-destructor-attribute.cpp
  clang/test/CodeGen/aix-init-priority-attribute.cpp
  clang/test/CodeGen/static-init.cpp
  clang/test/CodeGenCXX/aix-static-init.cpp

Index: clang/test/CodeGenCXX/aix-static-init.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/aix-static-init.cpp
@@ -0,0 +1,193 @@
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -S -emit-llvm -x c++ \
+// RUN: -fno-use-cxa-atexit -std=c++2a < %s | \
+// RUN:   FileCheck --check-prefixes=CHECK,CHECK32 %s
+
+// RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff -S -emit-llvm -x c++ \
+// RUN: -fno-use-cxa-atexit -std=c++2a < %s | \
+// RUN:   FileCheck --check-prefixes=CHECK,CHECK64 %s
+
+namespace test1 {
+  struct Test1 {
+Test1();
+~Test1();
+  } t1, t2;
+} // namespace test1
+
+namespace test2 {
+  int foo() { return 3; }
+  int x = foo();
+} // namespace test2
+
+namespace test3 {
+  struct Test3 {
+constexpr Test3() {};
+~Test3() {};
+  };
+
+  constinit Test3 t;
+} // namespace test3
+
+namespace test4 {
+  struct Test4 {
+Test4();
+~Test4();
+  };
+
+  void f() {
+static Test4 staticLocal;
+  }
+} // namespace test4
+
+// CHECK: @llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @__sinit8000_clang_1145401da454a7baad10bfe313c46638, i8* null }]
+// CHECK: @llvm.global_dtors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @__sterm8000_clang_1145401da454a7baad10bfe313c46638, i8* null }]
+
+// CHECK: define internal void @__cxx_global_var_init() [[ATTR:#[0-9]+]] {
+// CHECK: entry:
+// CHECK:   call void @_ZN5test15Test1C1Ev(%"struct.test1::Test1"* @_ZN5test12t1E)
+// CHECK:   %0 = call i32 @atexit(void ()* @__dtor__ZN5test12t1E)
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: define internal void @__dtor__ZN5test12t1E() [[ATTR:#[0-9]+]] {
+// CHECK: entry:
+// CHECK:   call void @_ZN5test15Test1D1Ev(%"struct.test1::Test1"* @_ZN5test12t1E)
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: declare i32 @atexit(void ()*)
+
+// CHECK: define internal void @__finalize__ZN5test12t1E() [[ATTR:#[0-9]+]] {
+// CHECK: entry:
+// CHECK:   %0 = call i32 @unatexit(void ()* @__dtor__ZN5test12t1E)
+// CHECK:   %needs_destruct = icmp eq i32 %0, 0
+// CHECK:   br i1 %needs_destruct, label %destruct.call, label %destruct.end
+
+// CHECK: destruct.call:
+// CHECK:   call void @__dtor__ZN5test12t1E()
+// CHECK:   br label %destruct.end
+
+// CHECK: destruct.end:
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: declare i32 @unatexit(void ()*)
+
+// CHECK: define internal void @__cxx_global_var_init.1() [[ATTR:#[0-9]+]] {
+// CHECK: entry:
+// CHECK:   call void @_ZN5test15Test1C1Ev(%"struct.test1::Test1"* @_ZN5test12t2E)
+// CHECK:   %0 = call i32 @atexit(void ()* @__dtor__ZN5test12t2E)
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: define internal void @__dtor__ZN5test12t2E() [[ATTR:#[0-9]+]] {
+// CHECK: entry:
+// CHECK:   call void @_ZN5test15Test1D1Ev(%"struct.test1::Test1"* @_ZN5test12t2E)
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: define internal void @__finalize__ZN5test12t2E() [[ATTR:#[0-9]+]] {
+// CHECK: entry:
+// CHECK:   %0 = call i32 @unatexit(void ()* @__dtor__ZN5test12t2E)
+// CHECK:   %needs_destruct = icmp eq i32 %0, 0
+// CHECK:   br i1 %needs_destruct, label %destruct.call, label %destruct.end
+
+// CHECK: destruct.call:
+// CHECK:   call void @__dtor__ZN5test12t2E()
+// CHECK:   br label %destruct.end
+
+// CHECK: destruct.end:
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: define internal void @__cxx_global_var_init.2() [[ATTR:#[0-9]+]] {
+// CHECK: entry:
+// CHECK32: %call = call i32 @_ZN5test23fooEv()
+// CHECK64: %call = call signext i32 @_ZN5test23fooEv()
+// CHECK:   store i32 %call, i32* @_ZN5test21xE
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: define internal void @__cxx_global_var_init.3() [[ATTR:#[0-9]+]] {
+// CHECK: entry:
+// CHECK:   %0 = call i32 @atexit(void ()* @__dtor__ZN5test31tE)
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: define internal void @__dtor__ZN5test31tE() [[ATTR:#[0-9]+]] {
+// CHECK: entry:
+// CHECK:   call void @_ZN5test35Test3D1Ev(%"struct.test3::Test3"* @_ZN5test31tE)
+// CHECK:   ret void
+// CHECK: }
+
+// 

[PATCH] D74166: [AIX][Frontend] Static init implementation for AIX considering no priority

[Xiangling Liao via Phabricator via cfe-commits]

Xiangling_L updated this revision to Diff 271664.
Xiangling_L marked an inline comment as done.
Xiangling_L added a comment.

Adjust the patch corresponding to D81972 ;


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74166/new/

https://reviews.llvm.org/D74166

Files:
  clang/include/clang/AST/Mangle.h
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/CodeGen/CGCXXABI.h
  clang/lib/CodeGen/CGDeclCXX.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/CodeGen/MicrosoftCXXABI.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGen/aix-constructor-attribute.cpp
  clang/test/CodeGen/aix-destructor-attribute.cpp
  clang/test/CodeGen/aix-init-priority-attribute.cpp
  clang/test/CodeGen/static-init.cpp
  clang/test/CodeGenCXX/aix-static-init.cpp

Index: clang/test/CodeGenCXX/aix-static-init.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/aix-static-init.cpp
@@ -0,0 +1,193 @@
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -S -emit-llvm -x c++ \
+// RUN: -fno-use-cxa-atexit -std=c++2a < %s | \
+// RUN:   FileCheck --check-prefixes=CHECK,CHECK32 %s
+
+// RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff -S -emit-llvm -x c++ \
+// RUN: -fno-use-cxa-atexit -std=c++2a < %s | \
+// RUN:   FileCheck --check-prefixes=CHECK,CHECK64 %s
+
+namespace test1 {
+  struct Test1 {
+Test1();
+~Test1();
+  } t1, t2;
+} // namespace test1
+
+namespace test2 {
+  int foo() { return 3; }
+  int x = foo();
+} // namespace test2
+
+namespace test3 {
+  struct Test3 {
+constexpr Test3() {};
+~Test3() {};
+  };
+
+  constinit Test3 t;
+} // namespace test3
+
+namespace test4 {
+  struct Test4 {
+Test4();
+~Test4();
+  };
+
+  void f() {
+static Test4 staticLocal;
+  }
+} // namespace test4
+
+// CHECK: @llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @__sinit8000_clang_1145401da454a7baad10bfe313c46638, i8* null }]
+// CHECK: @llvm.global_dtors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @__sterm8000_clang_1145401da454a7baad10bfe313c46638, i8* null }]
+
+// CHECK: define internal void @__cxx_global_var_init() [[ATTR:#[0-9]+]] {
+// CHECK: entry:
+// CHECK:   call void @_ZN5test15Test1C1Ev(%"struct.test1::Test1"* @_ZN5test12t1E)
+// CHECK:   %0 = call i32 @atexit(void ()* @__dtor__ZN5test12t1E)
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: define internal void @__dtor__ZN5test12t1E() [[ATTR:#[0-9]+]] {
+// CHECK: entry:
+// CHECK:   call void @_ZN5test15Test1D1Ev(%"struct.test1::Test1"* @_ZN5test12t1E)
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: declare i32 @atexit(void ()*)
+
+// CHECK: define internal void @__finalize__ZN5test12t1E() [[ATTR:#[0-9]+]] {
+// CHECK: entry:
+// CHECK:   %0 = call i32 @unatexit(void ()* @__dtor__ZN5test12t1E)
+// CHECK:   %needs_destruct = icmp eq i32 %0, 0
+// CHECK:   br i1 %needs_destruct, label %destruct.call, label %destruct.end
+
+// CHECK: destruct.call:
+// CHECK:   call void @__dtor__ZN5test12t1E()
+// CHECK:   br label %destruct.end
+
+// CHECK: destruct.end:
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: declare i32 @unatexit(void ()*)
+
+// CHECK: define internal void @__cxx_global_var_init.1() [[ATTR:#[0-9]+]] {
+// CHECK: entry:
+// CHECK:   call void @_ZN5test15Test1C1Ev(%"struct.test1::Test1"* @_ZN5test12t2E)
+// CHECK:   %0 = call i32 @atexit(void ()* @__dtor__ZN5test12t2E)
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: define internal void @__dtor__ZN5test12t2E() [[ATTR:#[0-9]+]] {
+// CHECK: entry:
+// CHECK:   call void @_ZN5test15Test1D1Ev(%"struct.test1::Test1"* @_ZN5test12t2E)
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: define internal void @__finalize__ZN5test12t2E() [[ATTR:#[0-9]+]] {
+// CHECK: entry:
+// CHECK:   %0 = call i32 @unatexit(void ()* @__dtor__ZN5test12t2E)
+// CHECK:   %needs_destruct = icmp eq i32 %0, 0
+// CHECK:   br i1 %needs_destruct, label %destruct.call, label %destruct.end
+
+// CHECK: destruct.call:
+// CHECK:   call void @__dtor__ZN5test12t2E()
+// CHECK:   br label %destruct.end
+
+// CHECK: destruct.end:
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: define internal void @__cxx_global_var_init.2() [[ATTR:#[0-9]+]] {
+// CHECK: entry:
+// CHECK32: %call = call i32 @_ZN5test23fooEv()
+// CHECK64: %call = call signext i32 @_ZN5test23fooEv()
+// CHECK:   store i32 %call, i32* @_ZN5test21xE
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: define internal void @__cxx_global_var_init.3() [[ATTR:#[0-9]+]] {
+// CHECK: entry:
+// CHECK:   %0 = call i32 @atexit(void ()* @__dtor__ZN5test31tE)
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: define internal void @__dtor__ZN5test31tE() [[ATTR:#[0-9]+]] {
+// CHECK: entry:
+// CHECK:   call void 

[PATCH] D74166: [AIX][Frontend] Static init implementation for AIX considering no priority

[Xiangling Liao via Phabricator via cfe-commits]

Xiangling_L added inline comments.



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:708
+" based on strong external symbols");
+  GlobalUniqueModuleId = GlobalUniqueModuleId.substr(1);
+}

jasonliu wrote:
> Correct me if I'm wrong...
> `GlobalUniqueModuleId` will always get “initialized" in 
> `EmitCXXGlobalInitFunc`. And `EmitCXXGlobalInitFunc` will always run before 
> `EmitCXXGlobalCleanUpFunc` in `CodeGenModule::Release()`. So in theory, we do 
> not need to initialize it again in here. 
> If we could not `assert (!GlobalUniqueModuleId.empty())` here, that tells me 
> in `EmitCXXGlobalInitFunc` we effectively get an empty string after 
> `GlobalUniqueModuleId = GlobalUniqueModuleId.substr(1);`. So something is not 
> right?
Actually `GlobalUniqueModuleId` will not always get “initialized" in 
`EmitCXXGlobalInitFunc`. In some cases, only __sterm will be generated for the 
var decl.
e.g.
The static local variable won't be inited before main() but when called at the 
first time.
```
struct Test4 {
  Test4();
~Test4();
  };

  void f() {
static Test4 staticLocal;
  }

```



Comment at: clang/test/CodeGen/static-init.cpp:142
+
+// CHECK: ; Function Attrs: noinline nounwind
+// CHECK: define internal void @__finalize__ZZN5test41fEvE11staticLocal() #0 {

jasonliu wrote:
> Is checking this Attrs necessary?
I missed to delete this line. Thanks for pointing it out.



Comment at: clang/test/CodeGen/static-init.cpp:157
+
+// CHECK: define void 
@__sinit8000_clang_1145401da454a7baad10bfe313c46638() #5 {
+// CHECK: entry:

jasonliu wrote:
> I think we used to have dso_local marked on sinit and sterm function in 
> previous diff. Now they are gone. What's the reason for removing them?
`dso_local` affects calls to the function in terms of how the compiler will do 
the optimization on the call. Since we won't be generating any calls to the 
function (the linker arranges for the calls to happen indirectly), marking them 
dso_local does not help any. So I think it doesn't really matter if we put 
`dso_local` here.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74166/new/

https://reviews.llvm.org/D74166



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D74166: [AIX][Frontend] Static init implementation for AIX considering no priority

[Xiangling Liao via Phabricator via cfe-commits]

Xiangling_L updated this revision to Diff 271550.
Xiangling_L marked 11 inline comments as done.
Xiangling_L edited the summary of this revision.
Xiangling_L added a comment.
Herald added a subscriber: jfb.

Remove trailing spaces;
Update the testcase;
Adjust the EmitGuardedInitBranch function;


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74166/new/

https://reviews.llvm.org/D74166

Files:
  clang/include/clang/AST/Mangle.h
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/CodeGen/CGCXXABI.h
  clang/lib/CodeGen/CGDeclCXX.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/CodeGen/MicrosoftCXXABI.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGen/aix-constructor-attribute.cpp
  clang/test/CodeGen/aix-destructor-attribute.cpp
  clang/test/CodeGen/aix-init-priority-attribute.cpp
  clang/test/CodeGen/static-init.cpp
  clang/test/CodeGenCXX/aix-static-init.cpp

Index: clang/test/CodeGenCXX/aix-static-init.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/aix-static-init.cpp
@@ -0,0 +1,193 @@
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -S -emit-llvm -x c++ \
+// RUN: -fno-use-cxa-atexit -std=c++2a < %s | \
+// RUN:   FileCheck --check-prefixes=CHECK,CHECK32 %s
+
+// RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff -S -emit-llvm -x c++ \
+// RUN: -fno-use-cxa-atexit -std=c++2a < %s | \
+// RUN:   FileCheck --check-prefixes=CHECK,CHECK64 %s
+
+namespace test1 {
+  struct Test1 {
+Test1();
+~Test1();
+  } t1, t2;
+} // namespace test1
+
+namespace test2 {
+  int foo() { return 3; }
+  int x = foo();
+} // namespace test2
+
+namespace test3 {
+  struct Test3 {
+constexpr Test3() {};
+~Test3() {};
+  };
+
+  constinit Test3 t;
+} // namespace test3
+
+namespace test4 {
+  struct Test4 {
+Test4();
+~Test4();
+  };
+
+  void f() {
+static Test4 staticLocal;
+  }
+} // namespace test4
+
+// CHECK: @llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @__sinit8000_clang_1145401da454a7baad10bfe313c46638, i8* null }]
+// CHECK: @llvm.global_dtors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @__sterm8000_clang_1145401da454a7baad10bfe313c46638, i8* null }]
+
+// CHECK: define internal void @__cxx_global_var_init() [[ATTR:#[0-9]+]] {
+// CHECK: entry:
+// CHECK:   call void @_ZN5test15Test1C1Ev(%"struct.test1::Test1"* @_ZN5test12t1E)
+// CHECK:   %0 = call i32 @atexit(void ()* @__dtor__ZN5test12t1E)
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: define internal void @__dtor__ZN5test12t1E() [[ATTR:#[0-9]+]] {
+// CHECK: entry:
+// CHECK:   call void @_ZN5test15Test1D1Ev(%"struct.test1::Test1"* @_ZN5test12t1E)
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: declare i32 @atexit(void ()*)
+
+// CHECK: define internal void @__finalize__ZN5test12t1E() [[ATTR:#[0-9]+]] {
+// CHECK: entry:
+// CHECK:   %0 = call i32 @unatexit(void ()* @__dtor__ZN5test12t1E)
+// CHECK:   %needs_destruct = icmp eq i32 %0, 0
+// CHECK:   br i1 %needs_destruct, label %destruct.call, label %destruct.end
+
+// CHECK: destruct.call:
+// CHECK:   call void @__dtor__ZN5test12t1E()
+// CHECK:   br label %destruct.end
+
+// CHECK: destruct.end:
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: declare i32 @unatexit(void ()*)
+
+// CHECK: define internal void @__cxx_global_var_init.1() [[ATTR:#[0-9]+]] {
+// CHECK: entry:
+// CHECK:   call void @_ZN5test15Test1C1Ev(%"struct.test1::Test1"* @_ZN5test12t2E)
+// CHECK:   %0 = call i32 @atexit(void ()* @__dtor__ZN5test12t2E)
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: define internal void @__dtor__ZN5test12t2E() [[ATTR:#[0-9]+]] {
+// CHECK: entry:
+// CHECK:   call void @_ZN5test15Test1D1Ev(%"struct.test1::Test1"* @_ZN5test12t2E)
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: define internal void @__finalize__ZN5test12t2E() [[ATTR:#[0-9]+]] {
+// CHECK: entry:
+// CHECK:   %0 = call i32 @unatexit(void ()* @__dtor__ZN5test12t2E)
+// CHECK:   %needs_destruct = icmp eq i32 %0, 0
+// CHECK:   br i1 %needs_destruct, label %destruct.call, label %destruct.end
+
+// CHECK: destruct.call:
+// CHECK:   call void @__dtor__ZN5test12t2E()
+// CHECK:   br label %destruct.end
+
+// CHECK: destruct.end:
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: define internal void @__cxx_global_var_init.2() [[ATTR:#[0-9]+]] {
+// CHECK: entry:
+// CHECK32: %call = call i32 @_ZN5test23fooEv()
+// CHECK64: %call = call signext i32 @_ZN5test23fooEv()
+// CHECK:   store i32 %call, i32* @_ZN5test21xE
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: define internal void @__cxx_global_var_init.3() [[ATTR:#[0-9]+]] {
+// CHECK: entry:
+// CHECK:   %0 = call i32 @atexit(void ()* @__dtor__ZN5test31tE)
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: define internal 

[PATCH] D74166: [AIX][Frontend] Static init implementation for AIX considering no priority

[Jason Liu via Phabricator via cfe-commits]

jasonliu added inline comments.



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:835
+
+  llvm::CallInst *CI = nullptr; 
+  if (Arg == nullptr) {

nit: trailing whitespace



Comment at: clang/test/CodeGen/static-init.cpp:31
+namespace test4 {
+  struct Test4 { 
+Test4();

nit: trailing whitespace in this line.
struct Test4 { 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74166/new/

https://reviews.llvm.org/D74166



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D74166: [AIX][Frontend] Static init implementation for AIX considering no priority

[Jason Liu via Phabricator via cfe-commits]

jasonliu added inline comments.



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:708
+" based on strong external symbols");
+  GlobalUniqueModuleId = GlobalUniqueModuleId.substr(1);
+}

Correct me if I'm wrong...
`GlobalUniqueModuleId` will always get “initialized" in 
`EmitCXXGlobalInitFunc`. And `EmitCXXGlobalInitFunc` will always run before 
`EmitCXXGlobalCleanUpFunc` in `CodeGenModule::Release()`. So in theory, we do 
not need to initialize it again in here. 
If we could not `assert (!GlobalUniqueModuleId.empty())` here, that tells me in 
`EmitCXXGlobalInitFunc` we effectively get an empty string after 
`GlobalUniqueModuleId = GlobalUniqueModuleId.substr(1);`. So something is not 
right?



Comment at: clang/lib/CodeGen/CodeGenModule.h:471
 
-  /// Global destructor functions and arguments that need to run on 
termination.
+  /// Global destructor functions and arguments that need to run on 
termination;
+  /// When UseSinitAndSterm is set, it instead contains sterm finalizer

Word after `;` should not be Capitalize. We should use small case, or change 
this to `.`.
I would prefer changing it to `.`.



Comment at: clang/test/CodeGen/static-init.cpp:142
+
+// CHECK: ; Function Attrs: noinline nounwind
+// CHECK: define internal void @__finalize__ZZN5test41fEvE11staticLocal() #0 {

Is checking this Attrs necessary?



Comment at: clang/test/CodeGen/static-init.cpp:157
+
+// CHECK: define void 
@__sinit8000_clang_1145401da454a7baad10bfe313c46638() #5 {
+// CHECK: entry:

I think we used to have dso_local marked on sinit and sterm function in 
previous diff. Now they are gone. What's the reason for removing them?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74166/new/

https://reviews.llvm.org/D74166



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D74166: [AIX][Frontend] Static init implementation for AIX considering no priority

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:4489
+  // DestructCallBlock, otherwise jump to EndBlock directly.
+  CGF.EmitCXXGuardedInitBranch(NeedsDestruct, DestructCallBlock, EndBlock,
+   CodeGenFunction::GuardKind::VariableGuard, );

Xiangling_L wrote:
> hubert.reinterpretcast wrote:
> > Please add a test for a static local. Note the `branch_weights`. I do not 
> > believe the `branch_weights` associated with guarded initialization for a 
> > static local (what the called function is meant to deal with) is 
> > necessarily appropriate for a branch associated with finalization-on-unload.
> Thank you for pointing this out. I will adjust the function name to 
> `EmitCXXGuardedInitOrDestructBranch` later to match our usage.
> 
> In terms of `branch_weights`,  this is something I am not familiar with. So 
> please correct me if I am wrong. Some of my superficial thoughts would be are 
> we able to apply `branch_weights` on a branch associated with 
> finalization-on-unload? Should it be set as `nullptr`,  because we would not 
> know how many times we will call `__sterm`(and each sterm finalizer 
> contained)?
> 
> BTW, I will add a testcase for a static local and we can update the 
> `branch_weights` part later along with the reviews.
I don't think we want to generate `branch_weights`. We already expect `__sterm` 
to be called rarely. Just changing the function name is not going to help 
though. A caller would need to indicate which of "init" or "destruct" is 
intended.



Comment at: clang/test/CodeGen/static-init.cpp:8
+// RUN:   FileCheck %s
 
 struct test {

Xiangling_L wrote:
> hubert.reinterpretcast wrote:
> > Xiangling_L wrote:
> > > jasonliu wrote:
> > > > Looks like the non-inline comments are easier to get ignored and 
> > > > missed, so I will copy paste the non-inlined comment I previously had:
> > > > ```
> > > > -fregister_global_dtors_with_atexit does not seem to work properly in 
> > > > current implementation.
> > > > We should consider somehow disabling/report_fatal_error it instead of 
> > > > letting it generate invalid code on AIX.
> > > > ```
> > > Thanks for doing so!
> > > 
> > > The semantic of `global_dtors` here on AIX is `__sterm` function, which 
> > > we are never gonna register with `__atexit`. I am thinking we can disable 
> > > it in a follow-up driver patch with the handling of `-fno-use-cxa-atexit`.
> > The semantic of `global_dtors` is not limited to the `__sterm` functions 
> > associated with C++ cleanup actions. With respect to user-declared 
> > `__attribute__((__destructor__))` functions, the option could improve the 
> > interleaving of those cleanup actions with cleanup actions registered by 
> > user-declared `__attribute__((__constructor__))` functions.
> > 
> > This provides that rationale for separating the `__sterm` functions 
> > associated with the C++ cleanup actions from the other "destructor" 
> > functions.
> Yeah, I agree that the semantic of `global_dtors` should contain both 
> `__sterm` and `__attribute__((__destructor__))` dtors. And we do need some 
> rationale to separate `__sterm` and those other `destructor` functions out 
> for AIX.
> 
> However, I doubt the `-fregister_global_dtors_with_atexit` option is 
> something we should use. Cuz dtors with `__attribute__((__destructor__))` 
> actually are equivalent to `__dtor` functions in AIX's semantic which need to 
> be registered with `__atexit`. However, we shouldn't register `__sterm` with 
> `__atexit`. So it seems this option does not work well for AIX either we set 
> it to true or false, and we need to figure something else out when we start 
> supporting `__attribute__((__destructor__))` and 
> `__attribute__((__constructor__))`?
Yes, we should look further into this when `__attribute__((__destructor__))` is 
supported.

The difference between `atexit`-registered functions and `__sterm` functions is 
in whether or not they are run when unloading a module (which termination also 
does).

Thanks for your observation re: the similarity between 
`__attribute__((__destructor__))` functions and the `__dtor` functions. 
Thinking about it a bit, the level of similarity is rather tied to the option 
in question. If we choose to register using `atexit`, we need to consider 
introducing the corresponding `unatexit`. This means that even when the option 
in question is active, we will produce entries into the `global_dtors` array at 
the IR level.



Comment at: clang/test/CodeGen/static-init.cpp:31
+// CHECK: @llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] 
[{ i32, void ()*, i8* } { i32 65535, void ()* 
@__sinit8000_clang_510e898aa8d263cac999dd03eeed5b51, i8* null }]
+// CHECK: @llvm.global_dtors = appending global [1 x { i32, void ()*, i8* }] 
[{ i32, void ()*, i8* } { i32 65535, void ()* 

[PATCH] D74166: [AIX][Frontend] Static init implementation for AIX considering no priority

[Xiangling Liao via Phabricator via cfe-commits]

Xiangling_L added inline comments.



Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:4489
+  // DestructCallBlock, otherwise jump to EndBlock directly.
+  CGF.EmitCXXGuardedInitBranch(NeedsDestruct, DestructCallBlock, EndBlock,
+   CodeGenFunction::GuardKind::VariableGuard, );

hubert.reinterpretcast wrote:
> Please add a test for a static local. Note the `branch_weights`. I do not 
> believe the `branch_weights` associated with guarded initialization for a 
> static local (what the called function is meant to deal with) is necessarily 
> appropriate for a branch associated with finalization-on-unload.
Thank you for pointing this out. I will adjust the function name to 
`EmitCXXGuardedInitOrDestructBranch` later to match our usage.

In terms of `branch_weights`,  this is something I am not familiar with. So 
please correct me if I am wrong. Some of my superficial thoughts would be are 
we able to apply `branch_weights` on a branch associated with 
finalization-on-unload? Should it be set as `nullptr`,  because we would not 
know how many times we will call `__sterm`(and each sterm finalizer contained)?

BTW, I will add a testcase for a static local and we can update the 
`branch_weights` part later along with the reviews.



Comment at: clang/test/CodeGen/static-init.cpp:8
+// RUN:   FileCheck %s
 
 struct test {

hubert.reinterpretcast wrote:
> Xiangling_L wrote:
> > jasonliu wrote:
> > > Looks like the non-inline comments are easier to get ignored and missed, 
> > > so I will copy paste the non-inlined comment I previously had:
> > > ```
> > > -fregister_global_dtors_with_atexit does not seem to work properly in 
> > > current implementation.
> > > We should consider somehow disabling/report_fatal_error it instead of 
> > > letting it generate invalid code on AIX.
> > > ```
> > Thanks for doing so!
> > 
> > The semantic of `global_dtors` here on AIX is `__sterm` function, which we 
> > are never gonna register with `__atexit`. I am thinking we can disable it 
> > in a follow-up driver patch with the handling of `-fno-use-cxa-atexit`.
> The semantic of `global_dtors` is not limited to the `__sterm` functions 
> associated with C++ cleanup actions. With respect to user-declared 
> `__attribute__((__destructor__))` functions, the option could improve the 
> interleaving of those cleanup actions with cleanup actions registered by 
> user-declared `__attribute__((__constructor__))` functions.
> 
> This provides that rationale for separating the `__sterm` functions 
> associated with the C++ cleanup actions from the other "destructor" functions.
Yeah, I agree that the semantic of `global_dtors` should contain both `__sterm` 
and `__attribute__((__destructor__))` dtors. And we do need some rationale to 
separate `__sterm` and those other `destructor` functions out for AIX.

However, I doubt the `-fregister_global_dtors_with_atexit` option is something 
we should use. Cuz dtors with `__attribute__((__destructor__))` actually are 
equivalent to `__dtor` functions in AIX's semantic which need to be registered 
with `__atexit`. However, we shouldn't register `__sterm` with `__atexit`. So 
it seems this option does not work well for AIX either we set it to true or 
false, and we need to figure something else out when we start supporting 
`__attribute__((__destructor__))` and `__attribute__((__constructor__))`?



Comment at: clang/test/CodeGen/static-init.cpp:31
+// CHECK: @llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] 
[{ i32, void ()*, i8* } { i32 65535, void ()* 
@__sinit8000_clang_510e898aa8d263cac999dd03eeed5b51, i8* null }]
+// CHECK: @llvm.global_dtors = appending global [1 x { i32, void ()*, i8* }] 
[{ i32, void ()*, i8* } { i32 65535, void ()* 
@__sterm8000_clang_510e898aa8d263cac999dd03eeed5b51, i8* null }]
+

hubert.reinterpretcast wrote:
> hubert.reinterpretcast wrote:
> > Okay, the restricted scope of this patch seems to landed in a place where 
> > how the `llvm.global_dtors` array will be handled is not indicated...
> > 
> > The backend should implement the semantics of the IR construct. When 
> > handling said array in the IR, it seems the method to handle the requisite 
> > semantics would be to generate an alias (with the appropriate linkage for 
> > the linker to pick it up) that is named using the `__sinit`/`__sterm` 
> > convention. Which is to say that at least some part of the naming should 
> > eventually move into the LLVM side and out of Clang.
> > 
> > Please update the description of this patch to indicate that:
> > 
> >   - The `llvm.global_ctors` and `llvm.global_dtors` functionality has not 
> > yet been implemented on AIX.
> >   - This patch temporarily side-steps the need to implement that 
> > functionality.
> >   - The apparent uses of that functionality in this patch (with respect to 
> > the name of the functions involved) are 

[PATCH] D74166: [AIX][Frontend] Static init implementation for AIX considering no priority

[Xiangling Liao via Phabricator via cfe-commits]

Xiangling_L updated this revision to Diff 271156.
Xiangling_L marked 35 inline comments as done.
Xiangling_L added a comment.

Renamed some functions;
Add one more test;
etc.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74166/new/

https://reviews.llvm.org/D74166

Files:
  clang/include/clang/AST/Mangle.h
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/CodeGen/CGCXXABI.h
  clang/lib/CodeGen/CGDeclCXX.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/CodeGen/MicrosoftCXXABI.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGen/aix-constructor-attribute.cpp
  clang/test/CodeGen/aix-destructor-attribute.cpp
  clang/test/CodeGen/aix-init-priority-attribute.cpp
  clang/test/CodeGen/static-init.cpp

Index: clang/test/CodeGen/static-init.cpp
===
--- clang/test/CodeGen/static-init.cpp
+++ clang/test/CodeGen/static-init.cpp
@@ -1,12 +1,175 @@
-// RUN: not %clang_cc1 -triple powerpc-ibm-aix-xcoff -S -emit-llvm -x c++ %s \
-// RUN: -o /dev/null 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -S -emit-llvm -x c++ \
+// RUN: -fno-use-cxa-atexit -std=c++2a < %s | \
+// RUN:   FileCheck --check-prefixes=CHECK,CHECK32 %s
 
-// RUN: not %clang_cc1 -triple powerpc64-ibm-aix-xcoff -S -emit-llvm -x c++ %s \
-// RUN: -o /dev/null 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff -S -emit-llvm -x c++ \
+// RUN: -fno-use-cxa-atexit -std=c++2a < %s | \
+// RUN:   FileCheck --check-prefixes=CHECK,CHECK64 %s
 
-struct test {
-  test();
-  ~test();
-} t;
+namespace test1 {
+  struct Test1 {
+Test1();
+~Test1();
+  } t1, t2;
+} // namespace test1
 
-// CHECK: error in backend: Static initialization has not been implemented on XL ABI yet.
+namespace test2 {
+  int foo() { return 3; }
+  int x = foo();
+} // namespace test2
+
+namespace test3 {
+  struct Test3 {
+constexpr Test3() {};
+~Test3() {};
+  };
+
+  constinit Test3 t;
+} // namespace test3
+
+namespace test4 {
+  struct Test4 { 
+Test4();
+~Test4();
+  };
+
+  void f() {
+static Test4 staticLocal;
+  }
+} // namespace test4
+
+// CHECK: @llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @__sinit8000_clang_1145401da454a7baad10bfe313c46638, i8* null }]
+// CHECK: @llvm.global_dtors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @__sterm8000_clang_1145401da454a7baad10bfe313c46638, i8* null }]
+
+// CHECK: define internal void @__cxx_global_var_init() #0 {
+// CHECK: entry:
+// CHECK:   call void @_ZN5test15Test1C1Ev(%"struct.test1::Test1"* @_ZN5test12t1E)
+// CHECK:   %0 = call i32 @atexit(void ()* @__dtor__ZN5test12t1E) #3
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: define internal void @__dtor__ZN5test12t1E() #0 {
+// CHECK: entry:
+// CHECK:   call void @_ZN5test15Test1D1Ev(%"struct.test1::Test1"* @_ZN5test12t1E)
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: declare i32 @atexit(void ()*)
+
+// CHECK: define internal void @__finalize__ZN5test12t1E() #0 {
+// CHECK: entry:
+// CHECK:   %0 = call i32 @unatexit(void ()* @__dtor__ZN5test12t1E) #3
+// CHECK:   %needs_destruct = icmp eq i32 %0, 0
+// CHECK:   br i1 %needs_destruct, label %destruct.call, label %destruct.end
+
+// CHECK: destruct.call:
+// CHECK:   call void @__dtor__ZN5test12t1E()
+// CHECK:   br label %destruct.end
+
+// CHECK: destruct.end:
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: declare i32 @unatexit(void ()*)
+
+// CHECK: define internal void @__cxx_global_var_init.1() #0 {
+// CHECK: entry:
+// CHECK:   call void @_ZN5test15Test1C1Ev(%"struct.test1::Test1"* @_ZN5test12t2E)
+// CHECK:   %0 = call i32 @atexit(void ()* @__dtor__ZN5test12t2E)
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: define internal void @__dtor__ZN5test12t2E() #0 {
+// CHECK: entry:
+// CHECK:   call void @_ZN5test15Test1D1Ev(%"struct.test1::Test1"* @_ZN5test12t2E)
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: define internal void @__finalize__ZN5test12t2E() #0 {
+// CHECK: entry:
+// CHECK:   %0 = call i32 @unatexit(void ()* @__dtor__ZN5test12t2E)
+// CHECK:   %needs_destruct = icmp eq i32 %0, 0
+// CHECK:   br i1 %needs_destruct, label %destruct.call, label %destruct.end
+
+// CHECK: destruct.call:
+// CHECK:   call void @__dtor__ZN5test12t2E()
+// CHECK:   br label %destruct.end
+
+// CHECK: destruct.end:
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: define internal void @__cxx_global_var_init.2() #0 {
+// CHECK: entry:
+// CHECK32: %call = call i32 @_ZN5test23fooEv()
+// CHECK64: %call = call signext i32 @_ZN5test23fooEv()
+// CHECK:   store i32 %call, i32* @_ZN5test21xE
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: define internal void 

[PATCH] D74166: [AIX][Frontend] Static init implementation for AIX considering no priority

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/test/CodeGen/static-init.cpp:31
+// CHECK: @llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] 
[{ i32, void ()*, i8* } { i32 65535, void ()* 
@__sinit8000_clang_510e898aa8d263cac999dd03eeed5b51, i8* null }]
+// CHECK: @llvm.global_dtors = appending global [1 x { i32, void ()*, i8* }] 
[{ i32, void ()*, i8* } { i32 65535, void ()* 
@__sterm8000_clang_510e898aa8d263cac999dd03eeed5b51, i8* null }]
+

hubert.reinterpretcast wrote:
> Okay, the restricted scope of this patch seems to landed in a place where how 
> the `llvm.global_dtors` array will be handled is not indicated...
> 
> The backend should implement the semantics of the IR construct. When handling 
> said array in the IR, it seems the method to handle the requisite semantics 
> would be to generate an alias (with the appropriate linkage for the linker to 
> pick it up) that is named using the `__sinit`/`__sterm` convention. Which is 
> to say that at least some part of the naming should eventually move into the 
> LLVM side and out of Clang.
> 
> Please update the description of this patch to indicate that:
> 
>   - The `llvm.global_ctors` and `llvm.global_dtors` functionality has not yet 
> been implemented on AIX.
>   - This patch temporarily side-steps the need to implement that 
> functionality.
>   - The apparent uses of that functionality in this patch (with respect to 
> the name of the functions involved) are not representative of how the 
> functionality will be used once implemented.
> 
I think the `llvm.global_ctors` usage is also more extensive here (and not 
symmetric on the finalization side):
```
struct C {
  C(int) {}
  ~C() {}
};

C c0 = 42;
template  C c = 42;
inline C c1 = 42;

int main(void) {
  (void) ;
}
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74166/new/

https://reviews.llvm.org/D74166



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D74166: [AIX][Frontend] Static init implementation for AIX considering no priority

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:4482
+  llvm::Value *NeedsDestruct =
+  CGF.Builder.CreateIsNull(V, "needsDestruct");
+

There are uses of `CreateIsNull` with `snake_case`; this is the only 
`camelCase` instance.



Comment at: clang/test/CodeGen/static-init.cpp:14
+  } t1, t2;
+}; // namespace test1
 

Minor nit: Don't use a semicolon after a namespace definition. Apply throughout.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74166/new/

https://reviews.llvm.org/D74166



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D74166: [AIX][Frontend] Static init implementation for AIX considering no priority

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:277
+ llvm::FunctionType::get(CGM.VoidTy, false) &&
+ "atexit has wrong parameter type.");
+

s/atexit has wrong parameter type/Argument to atexit has a wrong type/;



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:302
+ llvm::FunctionType::get(CGM.VoidTy, false) &&
+ "unatexit has wrong parameter type.");
+

s/unatexit has wrong parameter type/Argument to unatexit has a wrong type/;



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:649
+FuncName = llvm::Twine("__sinit8000_clang_", GlobalUniqueModuleId)
+   .toNullTerminatedStringRef(FuncName);
+  else {

Replace the call to `toNullTerminatedStringRef` and the assignment with a call 
to `toVector`.



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:657
+   getTransformedFileName(getModule(), Storage))
+   .toNullTerminatedStringRef(FuncName);
   }

Replace the call to `toNullTerminatedStringRef` and the assignment with a call 
to `toVector`.



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:664
+  if (getCXXABI().useSinitAndSterm())
+Fn->setLinkage(llvm::Function::ExternalLinkage);
 

`CreateGlobalInitOrDestructFunction` currently calls `Create` with 
`llvm::GlobalValue::InternalLinkage` and also calls 
`SetInternalFunctionAttributes`. Setting `ExternalLinkage` after-the-fact seems 
brittle.



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:690
 
-void CodeGenModule::EmitCXXGlobalDtorFunc() {
-  if (CXXGlobalDtors.empty())
+void CodeGenModule::EmitCXXGlobalDestructFunc() {
+  if (CXXGlobalDtorsOrStermFinalizers.empty())

I think "cleanup" works here too.



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:697
 
   // Create our global destructor function.
+  llvm::Function *Fn = nullptr;

s/Create our global destructor function/Create our global cleanup function/;



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:822
 
 // Emit the dtors, in reverse order from construction.
+for (unsigned i = 0, e = DtorsOrStermFinalizers.size(); i != e; ++i) {

s/dtors/cleanups/;



Comment at: clang/lib/CodeGen/CodeGenModule.h:472
   /// Global destructor functions and arguments that need to run on 
termination.
+  /// Global destructor functions and arguments that need to run on 
termination;
+  /// When UseSinitAndSterm is set, it contains sterm finalizers functions

Near duplicate comment line.



Comment at: clang/lib/CodeGen/CodeGenModule.h:473
+  /// Global destructor functions and arguments that need to run on 
termination;
+  /// When UseSinitAndSterm is set, it contains sterm finalizers functions
+  /// instead that need to run on unloading a shared library.

s/finalizers/finalizer/;
s/it contains/it instead contains/;



Comment at: clang/lib/CodeGen/CodeGenModule.h:474
+  /// When UseSinitAndSterm is set, it contains sterm finalizers functions
+  /// instead that need to run on unloading a shared library.
   std::vector<

s/instead that need to run/, which also run/;



Comment at: clang/lib/CodeGen/CodeGenModule.h:1059
+
+  /// Add a destructor to the C++ global destructor function.
+  void AddCXXDtorEntry(llvm::FunctionCallee DtorFn) {

s/a destructor/an sterm finalizer/;
s/global destructor/global cleanup/;



Comment at: clang/lib/CodeGen/CodeGenModule.h:1060
+  /// Add a destructor to the C++ global destructor function.
+  void AddCXXDtorEntry(llvm::FunctionCallee DtorFn) {
+CXXGlobalDtorsOrStermFinalizers.emplace_back(DtorFn.getFunctionType(),

s/Dtor/StermFinalizer/;



Comment at: clang/lib/CodeGen/CodeGenModule.h:1462
 
-  /// Emit the function that destroys C++ globals.
-  void EmitCXXGlobalDtorFunc();
+  /// Emit the function that destructs C++ globals.
+  void EmitCXXGlobalDestructFunc();

s/destructs/performs cleanup associated with/;



Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:4448
+  if (CGM.getCodeGenOpts().CXAAtExit)
+llvm::report_fatal_error("using __cxa_atexit unsupported on AIX.");
+  // Register above __dtor with atexit().

There should not be period at the end of a `report_fatal_error` message.



Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:4452
+
+  // Emit __finalize function to unregister __dtor and call __dtor.
+  emitCXXStermFinalizer(D, dtorStub, addr);

s/and/and (as appropriate)/;



Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:4477
+  // registered by the atexit subroutine. If the referenced function is 

[PATCH] D74166: [AIX][Frontend] Static init implementation for AIX considering no priority

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/test/CodeGen/static-init.cpp:8
+// RUN:   FileCheck %s
 
 struct test {

Xiangling_L wrote:
> jasonliu wrote:
> > Looks like the non-inline comments are easier to get ignored and missed, so 
> > I will copy paste the non-inlined comment I previously had:
> > ```
> > -fregister_global_dtors_with_atexit does not seem to work properly in 
> > current implementation.
> > We should consider somehow disabling/report_fatal_error it instead of 
> > letting it generate invalid code on AIX.
> > ```
> Thanks for doing so!
> 
> The semantic of `global_dtors` here on AIX is `__sterm` function, which we 
> are never gonna register with `__atexit`. I am thinking we can disable it in 
> a follow-up driver patch with the handling of `-fno-use-cxa-atexit`.
The semantic of `global_dtors` is not limited to the `__sterm` functions 
associated with C++ cleanup actions. With respect to user-declared 
`__attribute__((__destructor__))` functions, the option could improve the 
interleaving of those cleanup actions with cleanup actions registered by 
user-declared `__attribute__((__constructor__))` functions.

This provides that rationale for separating the `__sterm` functions associated 
with the C++ cleanup actions from the other "destructor" functions.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74166/new/

https://reviews.llvm.org/D74166



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D74166: [AIX][Frontend] Static init implementation for AIX considering no priority

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/test/CodeGen/static-init.cpp:31
+// CHECK: @llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] 
[{ i32, void ()*, i8* } { i32 65535, void ()* 
@__sinit8000_clang_510e898aa8d263cac999dd03eeed5b51, i8* null }]
+// CHECK: @llvm.global_dtors = appending global [1 x { i32, void ()*, i8* }] 
[{ i32, void ()*, i8* } { i32 65535, void ()* 
@__sterm8000_clang_510e898aa8d263cac999dd03eeed5b51, i8* null }]
+

Okay, the restricted scope of this patch seems to landed in a place where how 
the `llvm.global_dtors` array will be handled is not indicated...

The backend should implement the semantics of the IR construct. When handling 
said array in the IR, it seems the method to handle the requisite semantics 
would be to generate an alias (with the appropriate linkage for the linker to 
pick it up) that is named using the `__sinit`/`__sterm` convention. Which is to 
say that at least some part of the naming should eventually move into the LLVM 
side and out of Clang.

Please update the description of this patch to indicate that:

  - The `llvm.global_ctors` and `llvm.global_dtors` functionality has not yet 
been implemented on AIX.
  - This patch temporarily side-steps the need to implement that functionality.
  - The apparent uses of that functionality in this patch (with respect to the 
name of the functions involved) are not representative of how the functionality 
will be used once implemented.



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74166/new/

https://reviews.llvm.org/D74166



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D74166: [AIX][Frontend] Static init implementation for AIX considering no priority

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast marked an inline comment as done.
hubert.reinterpretcast added inline comments.



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:700
+
+Fn = CreateGlobalInitOrDestructFunction(
+FTy, llvm::Twine("__sterm8000_clang_") + GlobalUniqueModuleId, FI,

Xiangling_L wrote:
> hubert.reinterpretcast wrote:
> > The called function is to be renamed.
> Any suggestions here about how to represent the functionality of `__sterm` 
> and `_GLOBAL__D_a` into one word? Cannot think of a good name...
> 
> Actually I am wondering do we need to rename the `Destruct` part? The 
> `__sterm` and `_GLOBAL__D_a` do destruct the object by calling sterm 
> finalizers and object destructors?
Being clear in the naming of the function helps to signal its purpose to future 
maintainers. The sterm finalizers are unlikely to be understood from `Destruct` 
(it implies plain destruction a bit too much). Maybe "cleanup"?



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:807
 
 void CodeGenFunction::GenerateCXXGlobalDtorsFunc(
 llvm::Function *Fn,

Xiangling_L wrote:
> hubert.reinterpretcast wrote:
> > This function is to be renamed.
>  I will try `GenerateCXXGlobalDestructFunc` based on the thoughts as I also 
> mentioned elsewhere that the `__sterm` and `_GLOBAL__D_` function do destruct 
> the object by calling sterm finalizers and object destructors.
> 
> Any suggestions or concerns about this renaming? Or do we really need to 
> rename this one?
I think "cleanup" works here too.



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:639
+  if (CXXGlobalInits.empty())
+return;
 

Xiangling_L wrote:
> hubert.reinterpretcast wrote:
> > Can this part be committed in a separate patch? It does not appear to have 
> > dependencies on other parts of this patch and has the appearance of being a 
> > possible change for other platforms.
> Sure. I will create a NFC patch for this part and try to commit it first. But 
> so far, I think I can keep this part in this patch just for review purpose to 
> make the patch look nicer?
Sure; you'd need the new patch to exist before rebasing on it anyway. We can 
leave the rebasing to the commit or later in the review.



Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:4467
+
+  // Create a variable destruction function.
+  const CGFunctionInfo  = CGM.getTypes().arrangeNullaryFunction();

Xiangling_L wrote:
> hubert.reinterpretcast wrote:
> > Suggestion:
> > Create the finalization action associated with a variable.
> I don't get your point, can you elaborate on this a little bit?
This is my suggestion for the comment (to replace "Create a variable 
destruction function").


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74166/new/

https://reviews.llvm.org/D74166



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D74166: [AIX][Frontend] Static init implementation for AIX considering no priority

[Xiangling Liao via Phabricator via cfe-commits]

Xiangling_L updated this revision to Diff 270548.
Xiangling_L marked 35 inline comments as done.
Xiangling_L edited the summary of this revision.
Xiangling_L added a comment.

Address another round of reviews;


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74166/new/

https://reviews.llvm.org/D74166

Files:
  clang/include/clang/AST/Mangle.h
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/CodeGen/CGCXXABI.h
  clang/lib/CodeGen/CGDeclCXX.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGen/aix-constructor-attribute.cpp
  clang/test/CodeGen/aix-destructor-attribute.cpp
  clang/test/CodeGen/aix-init-priority-attribute.cpp
  clang/test/CodeGen/static-init.cpp

Index: clang/test/CodeGen/static-init.cpp
===
--- clang/test/CodeGen/static-init.cpp
+++ clang/test/CodeGen/static-init.cpp
@@ -1,12 +1,139 @@
-// RUN: not %clang_cc1 -triple powerpc-ibm-aix-xcoff -S -emit-llvm -x c++ %s \
-// RUN: -o /dev/null 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -S -emit-llvm -x c++ \
+// RUN: -fno-use-cxa-atexit -std=c++2a < %s | \
+// RUN:   FileCheck --check-prefixes=CHECK,CHECK32 %s
 
-// RUN: not %clang_cc1 -triple powerpc64-ibm-aix-xcoff -S -emit-llvm -x c++ %s \
-// RUN: -o /dev/null 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff -S -emit-llvm -x c++ \
+// RUN: -fno-use-cxa-atexit -std=c++2a < %s | \
+// RUN:   FileCheck --check-prefixes=CHECK,CHECK64 %s
 
-struct test {
-  test();
-  ~test();
-} t;
+namespace test1 {
+  struct Test1 {
+Test1();
+~Test1();
+  } t1, t2;
+}; // namespace test1
 
-// CHECK: error in backend: Static initialization has not been implemented on XL ABI yet.
+namespace test2 {
+  int foo() { return 3; }
+  int x = foo();
+}; // namespace test2
+
+namespace test3 {
+  struct Test {
+constexpr Test() {};
+~Test() {};
+  };
+
+  constinit Test t;
+}; // namespace test3
+
+// CHECK: @llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @__sinit8000_clang_510e898aa8d263cac999dd03eeed5b51, i8* null }]
+// CHECK: @llvm.global_dtors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @__sterm8000_clang_510e898aa8d263cac999dd03eeed5b51, i8* null }]
+
+// CHECK: define internal void @__cxx_global_var_init() #0 {
+// CHECK: entry: 
+// CHECK:   call void @_ZN5test15Test1C1Ev(%"struct.test1::Test1"* @_ZN5test12t1E)
+// CHECK:   %0 = call i32 @atexit(void ()* @__dtor__ZN5test12t1E)
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: define internal void @__dtor__ZN5test12t1E() #0 {
+// CHECK: entry:
+// CHECK:   call void @_ZN5test15Test1D1Ev(%"struct.test1::Test1"* @_ZN5test12t1E)
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: declare i32 @atexit(void ()*)
+
+// CHECK: define internal void @__finalize__ZN5test12t1E() #0 {
+// CHECK: entry:
+// CHECK:   %0 = call i32 @unatexit(void ()* @__dtor__ZN5test12t1E)
+// CHECK:   %needsDestruct = icmp eq i32 %0, 0
+// CHECK:   br i1 %needsDestruct, label %destruct.call, label %destruct.end
+
+// CHECK: destruct.call:
+// CHECK:   call void @__dtor__ZN5test12t1E()
+// CHECK:   br label %destruct.end
+
+// CHECK: destruct.end:
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: declare i32 @unatexit(void ()*)
+
+// CHECK: define internal void @__cxx_global_var_init.1() #0 {
+// CHECK: entry:
+// CHECK:   call void @_ZN5test15Test1C1Ev(%"struct.test1::Test1"* @_ZN5test12t2E)
+// CHECK:   %0 = call i32 @atexit(void ()* @__dtor__ZN5test12t2E)
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: define internal void @__dtor__ZN5test12t2E() #0 {
+// CHECK: entry:
+// CHECK:   call void @_ZN5test15Test1D1Ev(%"struct.test1::Test1"* @_ZN5test12t2E)
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: define internal void @__finalize__ZN5test12t2E() #0 {
+// CHECK: entry:
+// CHECK:   %0 = call i32 @unatexit(void ()* @__dtor__ZN5test12t2E)
+// CHECK:   %needsDestruct = icmp eq i32 %0, 0
+// CHECK:   br i1 %needsDestruct, label %destruct.call, label %destruct.end
+
+// CHECK: destruct.call:
+// CHECK:   call void @__dtor__ZN5test12t2E()
+// CHECK:   br label %destruct.end
+
+// CHECK: destruct.end:
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: define internal void @__cxx_global_var_init.2() #0 {
+// CHECK: entry:
+// CHECK32: %call = call i32 @_ZN5test23fooEv()
+// CHECK64: %call = call signext i32 @_ZN5test23fooEv()
+// CHECK:   store i32 %call, i32* @_ZN5test21xE
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: define internal void @__cxx_global_var_init.3() #0 {
+// CHECK: entry:
+// CHECK:   %0 = call i32 @atexit(void ()* @__dtor__ZN5test31tE)
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: define internal void @__dtor__ZN5test31tE() #0 {
+// CHECK: 

[PATCH] D74166: [AIX][Frontend] Static init implementation for AIX considering no priority

[Xiangling Liao via Phabricator via cfe-commits]

Xiangling_L added inline comments.



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:639
+  if (CXXGlobalInits.empty())
+return;
 

hubert.reinterpretcast wrote:
> Can this part be committed in a separate patch? It does not appear to have 
> dependencies on other parts of this patch and has the appearance of being a 
> possible change for other platforms.
Sure. I will create a NFC patch for this part and try to commit it first. But 
so far, I think I can keep this part in this patch just for review purpose to 
make the patch look nicer?



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:704
   AddGlobalDtor(Fn);
+  CXXGlobalDtors.clear();
 }

hubert.reinterpretcast wrote:
> If this is another drive-by fix, can we put it in a separate patch?
Sure, I will put it in the above mentioned clean-up NFC patch as well.



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:700
+
+Fn = CreateGlobalInitOrDestructFunction(
+FTy, llvm::Twine("__sterm8000_clang_") + GlobalUniqueModuleId, FI,

hubert.reinterpretcast wrote:
> The called function is to be renamed.
Any suggestions here about how to represent the functionality of `__sterm` and 
`_GLOBAL__D_a` into one word? Cannot think of a good name...

Actually I am wondering do we need to rename the `Destruct` part? The `__sterm` 
and `_GLOBAL__D_a` do destruct the object by calling sterm finalizers and 
object destructors?



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:807
 
 void CodeGenFunction::GenerateCXXGlobalDtorsFunc(
 llvm::Function *Fn,

hubert.reinterpretcast wrote:
> This function is to be renamed.
 I will try `GenerateCXXGlobalDestructFunc` based on the thoughts as I also 
mentioned elsewhere that the `__sterm` and `_GLOBAL__D_` function do destruct 
the object by calling sterm finalizers and object destructors.

Any suggestions or concerns about this renaming? Or do we really need to rename 
this one?



Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:4467
+
+  // Create a variable destruction function.
+  const CGFunctionInfo  = CGM.getTypes().arrangeNullaryFunction();

hubert.reinterpretcast wrote:
> Suggestion:
> Create the finalization action associated with a variable.
I don't get your point, can you elaborate on this a little bit?



Comment at: clang/test/CodeGen/static-init.cpp:8
+// RUN:   FileCheck %s
 
 struct test {

jasonliu wrote:
> Looks like the non-inline comments are easier to get ignored and missed, so I 
> will copy paste the non-inlined comment I previously had:
> ```
> -fregister_global_dtors_with_atexit does not seem to work properly in current 
> implementation.
> We should consider somehow disabling/report_fatal_error it instead of letting 
> it generate invalid code on AIX.
> ```
Thanks for doing so!

The semantic of `global_dtors` here on AIX is `__sterm` function, which we are 
never gonna register with `__atexit`. I am thinking we can disable it in a 
follow-up driver patch with the handling of `-fno-use-cxa-atexit`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74166/new/

https://reviews.llvm.org/D74166



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D74166: [AIX][Frontend] Static init implementation for AIX considering no priority

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:807
 
 void CodeGenFunction::GenerateCXXGlobalDtorsFunc(
 llvm::Function *Fn,

This function is to be renamed.



Comment at: clang/lib/CodeGen/CodeGenModule.h:401
+  /// A unique trailing identifier as a part of sinit/sterm function when
+  /// UseSinitAndSterm of CXXABI set as true.
+  std::string GlobalUniqueModuleId;

Minor nit: s/set/is set/;


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74166/new/

https://reviews.llvm.org/D74166



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D74166: [AIX][Frontend] Static init implementation for AIX considering no priority

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/test/CodeGen/static-init.cpp:12
   ~test();
-} t;
+} t1, t2;
 

I suggest adding also one each of the following:

  - a dynamic initialization of a non-local variable of type `int`
  - a `constinit` initialization of a non-local variable with non-constant 
destruction



CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74166/new/

https://reviews.llvm.org/D74166



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D74166: [AIX][Frontend] Static init implementation for AIX considering no priority

[Jason Liu via Phabricator via cfe-commits]

jasonliu added inline comments.



Comment at: clang/test/CodeGen/static-init.cpp:8
+// RUN:   FileCheck %s
 
 struct test {

Looks like the non-inline comments are easier to get ignored and missed, so I 
will copy paste the non-inlined comment I previously had:
```
-fregister_global_dtors_with_atexit does not seem to work properly in current 
implementation.
We should consider somehow disabling/report_fatal_error it instead of letting 
it generate invalid code on AIX.
```


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74166/new/

https://reviews.llvm.org/D74166



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D74166: [AIX][Frontend] Static init implementation for AIX considering no priority

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:682
 void CodeGenModule::EmitCXXGlobalDtorFunc() {
   if (CXXGlobalDtors.empty())
 return;

hubert.reinterpretcast wrote:
> Following from my previous comments on `CXXGlobalDtors`, I am not convinced 
> that `__sterm` functions match the role.
This function is to be renamed.



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:700
+
+Fn = CreateGlobalInitOrDestructFunction(
+FTy, llvm::Twine("__sterm8000_clang_") + GlobalUniqueModuleId, FI,

The called function is to be renamed.



Comment at: clang/lib/CodeGen/CodeGenModule.h:1057
+  /// Add a destructor to the C++ global destructor function.
+  void AddCXXDtorEntry(llvm::FunctionCallee DtorFn) {
+CXXGlobalDtors.emplace_back(DtorFn.getFunctionType(), DtorFn.getCallee(),

This function is to be renamed.



Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:4458
+
+void XLCXXABI::emitCXXGlobalVarDeclDestructFunc(const VarDecl ,
+llvm::Function *dtorStub,

This function is to be renamed.



Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:4470
+  const CGFunctionInfo  = CGM.getTypes().arrangeNullaryFunction();
+  llvm::Function *GlobalVDTermFn = CGM.CreateGlobalInitOrDestructFunction(
+  FTy, FnName.str(), FI, D.getLocation());

`Term`, being a short form for "termination", the variable name does not match 
the purpose based on the terminology within the Clang context.



Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:4487
+
+  // Check if the guard variable is zero.
+  CGF.EmitCXXGuardedInitBranch(NeedsDestruct, DestructCallBlock, EndBlock,

This comment is not helpful. It does not say which action is associated with 
which state.



Comment at: clang/test/CodeGen/aix-constructor-attribute.cpp:2
+// RUN: not %clang_cc1 -triple powerpc-ibm-aix-xcoff -x c++ -emit-llvm < %s \
+// RUN: 2>&1 | FileCheck %s
+// RUN: not %clang_cc1 -triple powerpc64-ibm-aix-xcoff -x c++ -emit-llvm < %s \

Please split on the `|` with the new line beginning with `FileCheck` indented 
two spaces in relation to the previous line. Apply throughout.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74166/new/

https://reviews.llvm.org/D74166



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D74166: [AIX][Frontend] Static init implementation for AIX considering no priority

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:824
   std::tie(CalleeTy, Callee, Arg) = DtorsAndObjects[e - i - 1];
-  llvm::CallInst *CI = Builder.CreateCall(CalleeTy, Callee, Arg);
+  llvm::CallInst *CI = (Arg == nullptr)
+   ? Builder.CreateCall(CalleeTy, Callee)

Please assert that `Arg == nullptr` iff we're dealing with sinit/sterm.



Comment at: clang/lib/CodeGen/CodeGenModule.h:1058
+  void AddCXXDtorEntry(llvm::FunctionCallee DtorFn) {
+CXXGlobalDtors.emplace_back(DtorFn.getFunctionType(), DtorFn.getCallee(),
+nullptr);

Xiangling_L wrote:
> hubert.reinterpretcast wrote:
> > The description of `CXXGlobalDtors` is
> > > Global destructor functions and arguments that need to run on termination.
> > 
> Currently, only Apple Kernal extension will use `AddCXXDtorEntry` to add 
> variable destructor to CXXGlobalDtors.
> An example is:
> 
> ```
> $cat test.cpp
> class test {
>int a;
> public:
> test(int c) {a = c;}
> ~test() {a = 0;}
> } t(1);
> 
> 
> $clang --driver-mode=g++ -target x86_64-apple-darwin10 -fapple-kext -flto -S 
> -o - test.cpp
> 
> ...
> define internal void @_GLOBAL__D_a() #0 {
> entry:
>   call void @_ZN4testD1Ev(%class.test* @t)   #~test() {a = 0;}
>   ret void
> }
> ```
> 
> Since the usage as you said below does not match with our sterm finalizer 
> function `__cxx_global_var_destruct`, I am thinking we can either modify the 
> name and the description of `CXXGlobalDtors` to make it also fit AIX or we 
> can define a brand new `CXXStermFinalizers` vector and also related facility 
> to `GenerateCXXStermFinalizerFunc` instead. 
> 
> An example of modification I have in mind is:
> `CXXGlobalDtorsOrStermFinalizers`
> 
> 
> > Global destructor functions and arguments that need to run on termination;
> > When UseSinitAndSterm is set, it contains sterm finalizer functions instead 
> > that need to run on unloading a shared library.
> 
> 
> 
> I would prefer the modification way to save a lot effort. Any thoughts on it?
I'm fine with renaming but it should include renaming the associated fuctions.



Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:4447
+
+  // Create __dtor function for the var decl.
+  llvm::Function *dtorStub = CGF.createAtExitStub(D, dtor, addr);

Xiangling_L wrote:
> hubert.reinterpretcast wrote:
> > We should probably report_fatal_error if `CXAAtExit` (the option) is true 
> > before this line. This would imply driver changes to default AIX to using 
> > `-fno-use-cxa-atexit`. An associated test file is 
> > https://github.com/llvm/llvm-project/blame/master/clang/test/Driver/cxa-atexit.cpp.
> Maybe we should add `-fno-use-cxa-atexit` to driver in a follow-up patch?
I'm okay with that.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74166/new/

https://reviews.llvm.org/D74166



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D74166: [AIX][Frontend] Static init implementation for AIX considering no priority

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/lib/AST/ItaniumMangle.cpp:5231
+  CXXNameMangler Mangler(*this, Out);
+  Mangler.getStream() << "__cxx_global_var_destruct_";
+  if (shouldMangleDeclName(D))

I believe these are actually paired with the `__dtor_` functions. The prefix 
can be `__finalize_`.



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:562
 
+static StringRef getTransformedFileName(llvm::Module , SmallString<128> 
FileName) {
+  FileName = llvm::sys::path::filename(M.getName());

`FileName` should be a reference?



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:643
+  // When not using sinit and sterm functions, include the filename in the
+  // symbol name. Including "sub_" matches gcc and makes sure these symbols
+  // appear lexicographically behind the symbols with priority emitted above.

The binding of the second sentence in relation to "not using sinit and sterm" 
is not clear in this new version. I still recommend introducing a "block" with 
a colon.



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:645
+  // appear lexicographically behind the symbols with priority emitted above.
+  StringRef FuncName;
+  SmallString<128> Storage;

Use a `SmallString` for `FuncName`.



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:646
+  StringRef FuncName;
+  SmallString<128> Storage;
+  if (UseSinitAndSterm)

Move `Storage` into the `else` block and use it only for calling 
`getTransformedFileName`.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74166/new/

https://reviews.llvm.org/D74166



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D74166: [AIX][Frontend] Static init implementation for AIX considering no priority

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:293
+CodeGenFunction::unregisterGlobalDtorWithUnAtExit(llvm::Function *dtorStub) {
+  // extern "C" int unatexit(void (*f)(void));
+  assert(dtorStub->getFunctionType() ==

Please add a comment explaining the meaning of the zero and non-zero return 
values.



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:603
   if (!PrioritizedCXXGlobalInits.empty()) {
+assert(!UseSinitAndSterm && "Prioritized Sinit and Sterm functions are not"
+" supported yet.");

I don't think we should capitalize "sinit" and "sterm" in comments. Indeed, 
there's a comment below where they are not capitalized.



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:639
+  if (CXXGlobalInits.empty())
+return;
 

Can this part be committed in a separate patch? It does not appear to have 
dependencies on other parts of this patch and has the appearance of being a 
possible change for other platforms.



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:641
 
-  for (size_t i = 0; i < FileName.size(); ++i) {
-// Replace everything that's not [a-zA-Z0-9._] with a _. This set happens
-// to be the set of C preprocessing numbers.
-if (!isPreprocessingNumberBody(FileName[i]))
-  FileName[i] = '_';
-  }
+  // Include the filename in the symbol name. When not using sinit and sterm
+  // functions, including "sub_" matches gcc and makes sure these symbols

The first sentence does not apply unconditionally in the status quo of this 
patch. Perhaps keep the original comment completely with something like:
```
// When not using sinit and sterm functions:
// ...
```



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:682
 void CodeGenModule::EmitCXXGlobalDtorFunc() {
   if (CXXGlobalDtors.empty())
 return;

Following from my previous comments on `CXXGlobalDtors`, I am not convinced 
that `__sterm` functions match the role.



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:704
   AddGlobalDtor(Fn);
+  CXXGlobalDtors.clear();
 }

If this is another drive-by fix, can we put it in a separate patch?



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:817
   std::tie(CalleeTy, Callee, Arg) = DtorsAndObjects[e - i - 1];
-  llvm::CallInst *CI = Builder.CreateCall(CalleeTy, Callee, Arg);
+  llvm::CallInst *CI = (Arg == nullptr)
+   ? Builder.CreateCall(CalleeTy, Callee)

This change would not belong in this function if its scope remains the 
generation of a "GlobalDtorsFunc" (based on my understanding of the latter).



Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:4445
+  if (D.getTLSKind() != VarDecl::TLS_None)
+llvm::report_fatal_error("thread local storage unimplemented on AIX yet");
+

s/unimplemented on AIX yet/not yet implemented on AIX/;



Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:4467
+
+  // Create a variable destruction function.
+  const CGFunctionInfo  = CGM.getTypes().arrangeNullaryFunction();

Suggestion:
Create the finalization action associated with a variable.



Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:4480
+
+  llvm::Value *NeedsDestruct = CGF.Builder.CreateIsNull(V, "guard.hasDtor");
+

I don't think this is a real "guard variable". I think the comments need to 
explain this (including scare quotes for "guard variable".


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74166/new/

https://reviews.llvm.org/D74166



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D74166: [AIX][Frontend] Static init implementation for AIX considering no priority

[Xiangling Liao via Phabricator via cfe-commits]

Xiangling_L added inline comments.



Comment at: clang/include/clang/AST/Mangle.h:178
 
+  virtual void mangleDynamicDestructor(const VarDecl *D, raw_ostream ) = 0;
+

hubert.reinterpretcast wrote:
> I am not sure "destructor" is the right term here. This seems to be an 
> analogue to the functions named using `mangleDynamicAtExitDestructor`, except 
> that those rather directly perform destruction and are registered with 
> `atexit` during initialization whereas these perform finalization and are 
> "registered" by being called from an "sterm" function. What are the thoughts 
> on `mangleDynamicStermFinalizer`?
`mangleDynamicDestructor` is an analogue to `mangleDynamicInitializer` actually.

From this perspective, I think `mangleDynamicStermFinalizer` is good.



Comment at: clang/lib/CodeGen/CodeGenModule.h:1058
+  void AddCXXDtorEntry(llvm::FunctionCallee DtorFn) {
+CXXGlobalDtors.emplace_back(DtorFn.getFunctionType(), DtorFn.getCallee(),
+nullptr);

hubert.reinterpretcast wrote:
> The description of `CXXGlobalDtors` is
> > Global destructor functions and arguments that need to run on termination.
> 
Currently, only Apple Kernal extension will use `AddCXXDtorEntry` to add 
variable destructor to CXXGlobalDtors.
An example is:

```
$cat test.cpp
class test {
   int a;
public:
test(int c) {a = c;}
~test() {a = 0;}
} t(1);


$clang --driver-mode=g++ -target x86_64-apple-darwin10 -fapple-kext -flto -S -o 
- test.cpp

...
define internal void @_GLOBAL__D_a() #0 {
entry:
  call void @_ZN4testD1Ev(%class.test* @t)   #~test() {a = 0;}
  ret void
}
```

Since the usage as you said below does not match with our sterm finalizer 
function `__cxx_global_var_destruct`, I am thinking we can either modify the 
name and the description of `CXXGlobalDtors` to make it also fit AIX or we can 
define a brand new `CXXStermFinalizers` vector and also related facility to 
`GenerateCXXStermFinalizerFunc` instead. 

An example of modification I have in mind is:
`CXXGlobalDtorsOrStermFinalizers`


> Global destructor functions and arguments that need to run on termination;
> When UseSinitAndSterm is set, it contains sterm finalizer functions instead 
> that need to run on unloading a shared library.



I would prefer the modification way to save a lot effort. Any thoughts on it?



Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:4447
+
+  // Create __dtor function for the var decl.
+  llvm::Function *dtorStub = CGF.createAtExitStub(D, dtor, addr);

hubert.reinterpretcast wrote:
> We should probably report_fatal_error if `CXAAtExit` (the option) is true 
> before this line. This would imply driver changes to default AIX to using 
> `-fno-use-cxa-atexit`. An associated test file is 
> https://github.com/llvm/llvm-project/blame/master/clang/test/Driver/cxa-atexit.cpp.
Maybe we should add `-fno-use-cxa-atexit` to driver in a follow-up patch?



Comment at: clang/test/CodeGen/static-init.cpp:15
+
+// CHECK: define internal void @__cxx_global_var_init() #0 {
+// CHECK: entry:

jasonliu wrote:
> #0 could be removed.
Why I chose to keep this `#0` is because to remove it, we would either remove 
the following `{` which I kinda feel makes the function look not nice or we 
need to match `#0` with regex which I think is redundant. 


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74166/new/

https://reviews.llvm.org/D74166



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D74166: [AIX][Frontend] Static init implementation for AIX considering no priority

[Xiangling Liao via Phabricator via cfe-commits]

Xiangling_L updated this revision to Diff 270267.
Xiangling_L marked 21 inline comments as done.
Xiangling_L added a comment.

Address another round of reviews;


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74166/new/

https://reviews.llvm.org/D74166

Files:
  clang/include/clang/AST/Mangle.h
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/CodeGen/CGCXXABI.h
  clang/lib/CodeGen/CGDeclCXX.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGen/aix-constructor-attribute.cpp
  clang/test/CodeGen/aix-destructor-attribute.cpp
  clang/test/CodeGen/aix-init-priority-attribute.cpp
  clang/test/CodeGen/static-init.cpp

Index: clang/test/CodeGen/static-init.cpp
===
--- clang/test/CodeGen/static-init.cpp
+++ clang/test/CodeGen/static-init.cpp
@@ -1,12 +1,87 @@
-// RUN: not %clang_cc1 -triple powerpc-ibm-aix-xcoff -S -emit-llvm -x c++ %s \
-// RUN: -o /dev/null 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -S -emit-llvm -x c++ \
+// RUN: -fno-use-cxa-atexit < %s | \
+// RUN:   FileCheck %s
 
-// RUN: not %clang_cc1 -triple powerpc64-ibm-aix-xcoff -S -emit-llvm -x c++ %s \
-// RUN: -o /dev/null 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff -S -emit-llvm -x c++ \
+// RUN: -fno-use-cxa-atexit < %s | \
+// RUN:   FileCheck %s
 
 struct test {
   test();
   ~test();
-} t;
+} t1, t2;
 
-// CHECK: error in backend: Static initialization has not been implemented on XL ABI yet.
+// CHECK: @llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @__sinit8000_clang_66d40ba2f9a26b497582a82a8a262181, i8* null }]
+// CHECK: @llvm.global_dtors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @__sterm8000_clang_66d40ba2f9a26b497582a82a8a262181, i8* null }]
+
+// CHECK: define internal void @__cxx_global_var_init() #0 {
+// CHECK: entry:
+// CHECK:   call void @_ZN4testC1Ev(%struct.test* @t1)
+// CHECK:   %0 = call i32 @atexit(void ()* @__dtor_t1)
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: define internal void @__dtor_t1() #0 {
+// CHECK: entry:
+// CHECK:   call void @_ZN4testD1Ev(%struct.test* @t1)
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: declare i32 @atexit(void ()*)
+
+// CHECK: define internal void @__cxx_global_var_destruct_t1() #0 {
+// CHECK: entry:
+// CHECK:   %0 = call i32 @unatexit(void ()* @__dtor_t1)
+// CHECK:   %guard.needsDestruct = icmp eq i32 %0, 0
+// CHECK:   br i1 %guard.needsDestruct, label %destruct.call, label %destruct.end
+
+// CHECK: destruct.call:
+// CHECK:   call void @__dtor_t1()
+// CHECK:   br label %destruct.end
+
+// CHECK: destruct.end:
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: declare i32 @unatexit(void ()*) #3
+
+// CHECK: define internal void @__cxx_global_var_init.1() #0 {
+// CHECK: entry:
+// CHECK:   call void @_ZN4testC1Ev(%struct.test* @t2)
+// CHECK:   %0 = call i32 @atexit(void ()* @__dtor_t2)
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: define internal void @__dtor_t2() #0 {
+// CHECK: entry:
+// CHECK:   call void @_ZN4testD1Ev(%struct.test* @t2)
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: define internal void @__cxx_global_var_destruct_t2() #0 {
+// CHECK: entry:
+// CHECK:   %0 = call i32 @unatexit(void ()* @__dtor_t2)
+// CHECK:   %guard.needsDestruct = icmp eq i32 %0, 0
+// CHECK:   br i1 %guard.needsDestruct, label %destruct.call, label %destruct.end
+
+// CHECK: destruct.call:
+// CHECK:   call void @__dtor_t2()
+// CHECK:   br label %destruct.end
+
+// CHECK: destruct.end:
+// CHECK:  ret void
+// CHECK: }
+
+// CHECK: define dso_local void @__sinit8000_clang_66d40ba2f9a26b497582a82a8a262181() #0 {
+// CHECK: entry:
+// CHECK:   call void @__cxx_global_var_init()
+// CHECK:   call void @__cxx_global_var_init.1()
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: define dso_local void @__sterm8000_clang_66d40ba2f9a26b497582a82a8a262181() #0 {
+// CHECK: entry:
+// CHECK:   call void @__cxx_global_var_destruct_t2()
+// CHECK:   call void @__cxx_global_var_destruct_t1()
+// CHECK:   ret void
+// CHECK: }
Index: clang/test/CodeGen/aix-init-priority-attribute.cpp
===
--- /dev/null
+++ clang/test/CodeGen/aix-init-priority-attribute.cpp
@@ -0,0 +1,17 @@
+// RUN: not %clang_cc1 -triple powerpc-ibm-aix-xcoff -x c++ -emit-llvm < %s \
+// RUN: 2>&1 | FileCheck %s
+// RUN: not %clang_cc1 -triple powerpc64-ibm-aix-xcoff -x c++ -emit-llvm < %s \
+// RUN: 2>&1 | FileCheck %s
+
+class test {
+  int a;
+
+public:
+  test(int c) { a = c; }
+  ~test() { a = 0; }
+};
+
+__attribute__((init_priority(2000)))
+test t(1);
+
+// CHECK: fatal error: error in backend: 'init_priority' attribute unsupported on AIX yet
Index: 

[PATCH] D74166: [AIX][Frontend] Static init implementation for AIX considering no priority

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:305
+  if (llvm::Function *unatexitFn =
+  dyn_cast(unatexit.getCallee()))
+unatexitFn->setDoesNotThrow();

Xiangling_L wrote:
> jasonliu wrote:
> > Is there a valid case that unatexit.getCallee() returns a type which could 
> > not be cast to llvm::Function?
> > i.e. Could we use cast instead of dyn_cast?
> I used `cast` instead of `dyn_cast` before Diff 9 actually, and then I 
> noticed that `clang-tidy` gave error and asked me to use `dyn_cast` instead. 
> Cannot recall what the error says though...
If we expect `cast` to be okay, then we should use it without the `if`.

In Diff 8, an `if` is used with `cast` (thus a `clang-tidy` error make sense):
```
  if (llvm::Function *unatexitFn = cast(unatexit.getCallee()))
```



Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74166/new/

https://reviews.llvm.org/D74166



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D74166: [AIX][Frontend] Static init implementation for AIX considering no priority

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/include/clang/AST/Mangle.h:178
 
+  virtual void mangleDynamicDestructor(const VarDecl *D, raw_ostream ) = 0;
+

I am not sure "destructor" is the right term here. This seems to be an analogue 
to the functions named using `mangleDynamicAtExitDestructor`, except that those 
rather directly perform destruction and are registered with `atexit` during 
initialization whereas these perform finalization and are "registered" by being 
called from an "sterm" function. What are the thoughts on 
`mangleDynamicStermFinalizer`?



Comment at: clang/lib/AST/ItaniumMangle.cpp:5217
+  else
+Mangler.getStream() << D->getName();
+}

Xiangling_L wrote:
> jasonliu wrote:
> > If I understand correctly, this function will come in pair with 
> > `__cxx_global_var_init`.
> > `__cxx_global_var_init` use number as suffix in the end to avoid 
> > duplication when there is more than one of those, but we are using the 
> > variable name as suffix here instead.
> > Do we want to use number as suffix here to match what 
> > `__cxx_global_var_init` does? It would help people to recognize the pairs 
> > and make them more symmetric. 
> This is somewhere I am kinda on the fence. Personally, I think embed decl 
> name in the __cxx_global_var_destruct_ / __cxx_global_vat_init_ as 
> `mangleDynamicAtExitDestructor` does is more helpful for the user to debug 
> the static init.
> I am not sure if we want to give up that benefit and be consistent with 
> current `__cxx_global_vat_init_ ` using number suffix or do we want to 
> replace number suffix by decl name for `__cxx_global_vat_init_ ` as well.
Not every dynamically initialized non-local variable of static storage duration 
requires non-trivial destruction. These don't actually pair with 
`__cxx_global_var_init`; rather, they pair with the calls to `atexit` and the 
functions whose addresses are passed to `atexit`.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74166/new/

https://reviews.llvm.org/D74166



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D74166: [AIX][Frontend] Static init implementation for AIX considering no priority

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:562
 
+static std::string getTransformedFileName(llvm::Module ) {
+  SmallString<128> FileName = llvm::sys::path::filename(M.getName());

Consider having the `SmallString<128>` buffer passed in and returning a 
`StringRef` backed by that buffer.



Comment at: clang/lib/CodeGen/CodeGenModule.h:1056
 
+  /// Add a destructor to the C++ global destructor function.
+  void AddCXXDtorEntry(llvm::FunctionCallee DtorFn) {

What is "the C++ global destructor function"? Based on the comment on 
`CXXGlobalDtors` it has something to do with termination of the program. The 
existing usage is limited and consistent: This is a facility used when even 
`atexit` is not available.



Comment at: clang/lib/CodeGen/CodeGenModule.h:1058
+  void AddCXXDtorEntry(llvm::FunctionCallee DtorFn) {
+CXXGlobalDtors.emplace_back(DtorFn.getFunctionType(), DtorFn.getCallee(),
+nullptr);

The description of `CXXGlobalDtors` is
> Global destructor functions and arguments that need to run on termination.




Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:4501
+
+  CGM.AddCXXDtorEntry(StermFn);
 }

The purpose of the specific interface does not appear to match its usage here 
(see my other comment). This function is meant to be called on unloading a 
shared library. The usual termination path calls destructors via `atexit`.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74166/new/

https://reviews.llvm.org/D74166



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D74166: [AIX][Frontend] Static init implementation for AIX considering no priority

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:4447
+
+  // Create __dtor function for the var decl.
+  llvm::Function *dtorStub = CGF.createAtExitStub(D, dtor, addr);

We should probably report_fatal_error if `CXAAtExit` (the option) is true 
before this line. This would imply driver changes to default AIX to using 
`-fno-use-cxa-atexit`. An associated test file is 
https://github.com/llvm/llvm-project/blame/master/clang/test/Driver/cxa-atexit.cpp.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74166/new/

https://reviews.llvm.org/D74166



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D74166: [AIX][Frontend] Static init implementation for AIX considering no priority

[Jason Liu via Phabricator via cfe-commits]

jasonliu added inline comments.



Comment at: clang/lib/CodeGen/CGCXXABI.h:113
+
+  virtual bool isCXXGlobalInitAndDtorFuncInternal() const { return true; }
+

Xiangling_L wrote:
> jasonliu wrote:
> > Do we need this isCXXGlobalInitAndDtorFuncInternal? Looks like it's always 
> > going to return ! useSinitAndSterm() result.
> AFAIK, not all platforms which use sinit and sterm have external linkage 
> sinit/sterm functions. And also since for 7.2 AIX we are considering change 
> sinit/sterm to internal linkage symbols, I seperate this query out.
I would prefer to create the query in the patch when we actually need it. With 
what we have right now, it seems redundant. 



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:650
+  FTy,
+  (UseSinitAndSterm ? "__sinit8000_clang_" : "_GLOBAL__sub_I_") +
+  FuncSuffix,

It looks like one of the UseSinitAndSterm is redundant. We could have something 
like: 
```
UseSinitAndSterm ? llvm::twine("__sinit8000_clang_") + GlobalUniqueModuleId 
: llvm::twine("_GLOBAL__sub_I_") + getTransformedFileName(getModule())
```
which minimize the use of std::string?
If this is too long, then we could separate them into if statement just like 
what we have in EmitCXXGlobalDtorFunc, or use a lambda.



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:692
+  if (UseSinitAndSterm) {
+Fn = CreateGlobalInitOrDestructFunction(
+FTy, llvm::Twine("__sterm8000_clang_") + GlobalUniqueModuleId, FI,

Could we assert !GlobalUniqueModuleId.empty() here?
Right now, it solely relies on `EmitCXXGlobalInitFunc` to run before 
`EmitCXXGlobalDtorFunc` to get `GlobalUniqueModuleId` initialized properly. I 
think it makes sense to put an assert here to make sure it stays in that case 
in the future.  



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:699
+
+  if (!getCXXABI().isCXXGlobalInitAndDtorFuncInternal())
+Fn->setLinkage(llvm::Function::ExternalLinkage);

This could go inside of the `if (UseSinitAndSterm)`.



Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:4479
+
+  llvm::Value *NeedsDestruct = CGF.Builder.CreateIsNull(V, "guard.hasSrterm");
+

jasonliu wrote:
> Srterm is not used in this implementation.
> Suggestion:
> guard.hasDtorCalled
"guard.hasDtor" does not reflect what the meaning. Maybe matching with the 
variable name would make sense: "guard.needsDestruct" or just "needsDestruct"?



Comment at: clang/test/CodeGen/static-init.cpp:15
+
+// CHECK: define internal void @__cxx_global_var_init() #0 {
+// CHECK: entry:

#0 could be removed.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74166/new/

https://reviews.llvm.org/D74166



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D74166: [AIX][Frontend] Static init implementation for AIX considering no priority

[Xiangling Liao via Phabricator via cfe-commits]

Xiangling_L added inline comments.



Comment at: clang/lib/AST/ItaniumMangle.cpp:5217
+  else
+Mangler.getStream() << D->getName();
+}

jasonliu wrote:
> If I understand correctly, this function will come in pair with 
> `__cxx_global_var_init`.
> `__cxx_global_var_init` use number as suffix in the end to avoid duplication 
> when there is more than one of those, but we are using the variable name as 
> suffix here instead.
> Do we want to use number as suffix here to match what `__cxx_global_var_init` 
> does? It would help people to recognize the pairs and make them more 
> symmetric. 
This is somewhere I am kinda on the fence. Personally, I think embed decl name 
in the __cxx_global_var_destruct_ / __cxx_global_vat_init_ as 
`mangleDynamicAtExitDestructor` does is more helpful for the user to debug the 
static init.
I am not sure if we want to give up that benefit and be consistent with current 
`__cxx_global_vat_init_ ` using number suffix or do we want to replace number 
suffix by decl name for `__cxx_global_vat_init_ ` as well.



Comment at: clang/lib/CodeGen/CGCXXABI.h:113
+
+  virtual bool isCXXGlobalInitAndDtorFuncInternal() const { return true; }
+

jasonliu wrote:
> Do we need this isCXXGlobalInitAndDtorFuncInternal? Looks like it's always 
> going to return ! useSinitAndSterm() result.
AFAIK, not all platforms which use sinit and sterm have external linkage 
sinit/sterm functions. And also since for 7.2 AIX we are considering change 
sinit/sterm to internal linkage symbols, I seperate this query out.



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:305
+  if (llvm::Function *unatexitFn =
+  dyn_cast(unatexit.getCallee()))
+unatexitFn->setDoesNotThrow();

jasonliu wrote:
> Is there a valid case that unatexit.getCallee() returns a type which could 
> not be cast to llvm::Function?
> i.e. Could we use cast instead of dyn_cast?
I used `cast` instead of `dyn_cast` before Diff 9 actually, and then I noticed 
that `clang-tidy` gave error and asked me to use `dyn_cast` instead. Cannot 
recall what the error says though...



Comment at: clang/lib/CodeGen/CodeGenModule.h:20
 #include "SanitizerMetadata.h"
+#include "clang/AST/Attr.h"
 #include "clang/AST/DeclCXX.h"

jasonliu wrote:
> Is this Attr.h needed somewhere in this file?
Oops..this is something I forgot to remove. Good catch!



Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:4481
+
+  llvm::BasicBlock *DestructCheckBlock = 
CGF.createBasicBlock("destruct.check");
+  llvm::BasicBlock *EndBlock = CGF.createBasicBlock("destruct.end");

jasonliu wrote:
> I think we need a better naming for this and make it consistent with the end 
> block below.
> As it is for now, `destruct.check` is confusing, as we are not checking 
> anything here and we are just going to call destructor directly in this 
> block. 
Thanks, I will try `destruct.call` instead.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74166/new/

https://reviews.llvm.org/D74166



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D74166: [AIX][Frontend] Static init implementation for AIX considering no priority

[Xiangling Liao via Phabricator via cfe-commits]

Xiangling_L updated this revision to Diff 269900.
Xiangling_L marked 17 inline comments as done.
Xiangling_L added a comment.

Address the comments


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74166/new/

https://reviews.llvm.org/D74166

Files:
  clang/include/clang/AST/Mangle.h
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/CodeGen/CGCXXABI.h
  clang/lib/CodeGen/CGDeclCXX.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGen/aix-constructor-attribute.cpp
  clang/test/CodeGen/aix-destructor-attribute.cpp
  clang/test/CodeGen/aix-init-priority-attribute.cpp
  clang/test/CodeGen/static-init.cpp

Index: clang/test/CodeGen/static-init.cpp
===
--- clang/test/CodeGen/static-init.cpp
+++ clang/test/CodeGen/static-init.cpp
@@ -1,12 +1,85 @@
-// RUN: not %clang_cc1 -triple powerpc-ibm-aix-xcoff -S -emit-llvm -x c++ %s \
-// RUN: -o /dev/null 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -S -emit-llvm -x c++ < %s \
+// RUN: | FileCheck %s
 
-// RUN: not %clang_cc1 -triple powerpc64-ibm-aix-xcoff -S -emit-llvm -x c++ %s \
-// RUN: -o /dev/null 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff -S -emit-llvm -x c++ < %s \
+// RUN: | FileCheck %s
 
 struct test {
   test();
   ~test();
-} t;
+} t1, t2;
 
-// CHECK: error in backend: Static initialization has not been implemented on XL ABI yet.
+// CHECK: @llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @__sinit8000_clang_66d40ba2f9a26b497582a82a8a262181, i8* null }]
+// CHECK: @llvm.global_dtors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @__sterm8000_clang_66d40ba2f9a26b497582a82a8a262181, i8* null }]
+
+// CHECK: define internal void @__cxx_global_var_init() #0 {
+// CHECK: entry:
+// CHECK:   call void @_ZN4testC1Ev(%struct.test* @t1)
+// CHECK:   %0 = call i32 @atexit(void ()* @__dtor_t1)
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: define internal void @__dtor_t1() #0 {
+// CHECK: entry:
+// CHECK:   call void @_ZN4testD1Ev(%struct.test* @t1)
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: declare i32 @atexit(void ()*)
+
+// CHECK: define internal void @__cxx_global_var_destruct_t1() #0 {
+// CHECK: entry:
+// CHECK:   %0 = call i32 @unatexit(void ()* @__dtor_t1)
+// CHECK:   %guard.hasDtor = icmp eq i32 %0, 0
+// CHECK:   br i1 %guard.hasDtor, label %destruct.call, label %destruct.end
+
+// CHECK: destruct.call:
+// CHECK:   call void @__dtor_t1()
+// CHECK:   br label %destruct.end
+
+// CHECK: destruct.end:
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: declare i32 @unatexit(void ()*) #3
+
+// CHECK: define internal void @__cxx_global_var_init.1() #0 {
+// CHECK: entry:
+// CHECK:   call void @_ZN4testC1Ev(%struct.test* @t2)
+// CHECK:   %0 = call i32 @atexit(void ()* @__dtor_t2)
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: define internal void @__dtor_t2() #0 {
+// CHECK: entry:
+// CHECK:   call void @_ZN4testD1Ev(%struct.test* @t2)
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: define internal void @__cxx_global_var_destruct_t2() #0 {
+// CHECK: entry:
+// CHECK:   %0 = call i32 @unatexit(void ()* @__dtor_t2)
+// CHECK:   %guard.hasDtor = icmp eq i32 %0, 0
+// CHECK:   br i1 %guard.hasDtor, label %destruct.call, label %destruct.end
+
+// CHECK: destruct.call:
+// CHECK:   call void @__dtor_t2()
+// CHECK:   br label %destruct.end
+
+// CHECK: destruct.end:
+// CHECK:  ret void
+// CHECK: }
+
+// CHECK: define dso_local void @__sinit8000_clang_66d40ba2f9a26b497582a82a8a262181() #0 {
+// CHECK: entry:
+// CHECK:   call void @__cxx_global_var_init()
+// CHECK:   call void @__cxx_global_var_init.1()
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: define dso_local void @__sterm8000_clang_66d40ba2f9a26b497582a82a8a262181() #0 {
+// CHECK: entry:
+// CHECK:   call void @__cxx_global_var_destruct_t2()
+// CHECK:   call void @__cxx_global_var_destruct_t1()
+// CHECK:   ret void
+// CHECK: }
Index: clang/test/CodeGen/aix-init-priority-attribute.cpp
===
--- /dev/null
+++ clang/test/CodeGen/aix-init-priority-attribute.cpp
@@ -0,0 +1,17 @@
+// RUN: not %clang_cc1 -triple powerpc-ibm-aix-xcoff -x c++ -emit-llvm < %s \
+// RUN: 2>&1 | FileCheck %s
+// RUN: not %clang_cc1 -triple powerpc64-ibm-aix-xcoff -x c++ -emit-llvm < %s \
+// RUN: 2>&1 | FileCheck %s
+
+class test {
+  int a;
+
+public:
+  test(int c) { a = c; }
+  ~test() { a = 0; }
+};
+
+__attribute__((init_priority(2000)))
+test t(1);
+
+// CHECK: fatal error: error in backend: 'init_priority' attribute unsupported on AIX yet
Index: clang/test/CodeGen/aix-destructor-attribute.cpp
===
--- /dev/null

[PATCH] D74166: [AIX][Frontend] Static init implementation for AIX considering no priority

[Jason Liu via Phabricator via cfe-commits]

jasonliu added a comment.

-fregister_global_dtors_with_atexit does not seem to work properly in current 
implementation. 
We should consider somehow disabling/report_fatal_error it instead of letting 
it generate invalid code on AIX.




Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:594
+   "Cannot produce a unique identifier for this module based on strong 
"
+   "external symbols.");
+GlobalUniqueModuleId = GlobalUniqueModuleId.substr(1);

I think report_fatal_error is more appropriate here? Since we know some case 
will trigger this, and we do not want those case to silently pass in 
non-assertion mode?



Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:4452
+
+  // Emit __sterm function to unregister __srterm and call __srterm.
+  emitCXXGlobalVarDeclDestructFunc(D, dtorStub, addr);

This comment is confusing.
This is not emitting `__sterm` function, this is emitting 
`__cxx_global_var_destruct` functions. And again, we do not have `__srterm` 
function in this implementation. 



Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:4468
+  const CGFunctionInfo  = CGM.getTypes().arrangeNullaryFunction();
+  llvm::Function *StermFn = CGM.CreateGlobalInitOrDestructFunction(
+  FTy, FnName.str(), FI, D.getLocation());

nit: this is not sterm function.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74166/new/

https://reviews.llvm.org/D74166



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D74166: [AIX][Frontend] Static init implementation for AIX considering no priority

[Jason Liu via Phabricator via cfe-commits]

jasonliu added inline comments.



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:305
+  if (llvm::Function *unatexitFn =
+  dyn_cast(unatexit.getCallee()))
+unatexitFn->setDoesNotThrow();

Is there a valid case that unatexit.getCallee() returns a type which could not 
be cast to llvm::Function?
i.e. Could we use cast instead of dyn_cast?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74166/new/

https://reviews.llvm.org/D74166



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D74166: [AIX][Frontend] Static init implementation for AIX considering no priority

[Jason Liu via Phabricator via cfe-commits]

jasonliu added inline comments.



Comment at: clang/lib/AST/ItaniumMangle.cpp:5217
+  else
+Mangler.getStream() << D->getName();
+}

If I understand correctly, this function will come in pair with 
`__cxx_global_var_init`.
`__cxx_global_var_init` use number as suffix in the end to avoid duplication 
when there is more than one of those, but we are using the variable name as 
suffix here instead.
Do we want to use number as suffix here to match what `__cxx_global_var_init` 
does? It would help people to recognize the pairs and make them more symmetric. 



Comment at: clang/lib/CodeGen/CGCXXABI.h:113
+
+  virtual bool isCXXGlobalInitAndDtorFuncInternal() const { return true; }
+

Do we need this isCXXGlobalInitAndDtorFuncInternal? Looks like it's always 
going to return ! useSinitAndSterm() result.



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:586
 
+  bool UseSinitAndSterm = getCXXABI().useSinitAndSterm();
+  if (UseSinitAndSterm) {

`const` for UseSinitAndSterm variable?



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:638
+  // Create our global initialization function.
+  if (!CXXGlobalInits.empty()) {
+// Include the filename in the symbol name. When not using sinit and sterm

flip this for an early return?



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:691
+  if (UseSinitAndSterm) {
+std::string FuncSuffix = GlobalUniqueModuleId;
+Fn = CreateGlobalInitOrDestructFunction(

We might want to avoid this string copy construction if possible. 



Comment at: clang/lib/CodeGen/CodeGenModule.h:20
 #include "SanitizerMetadata.h"
+#include "clang/AST/Attr.h"
 #include "clang/AST/DeclCXX.h"

Is this Attr.h needed somewhere in this file?



Comment at: clang/lib/CodeGen/CodeGenModule.h:401
+  /// A unique trailing identifier as a part of sinit/sterm function when
+  /// UserSinitAndSterm set as true.
+  std::string GlobalUniqueModuleId;

nit: UserSinitAndSterm -> UseSinitAndSterm?
and we do not have that property here.
Suggestion:
when getCXXABI().useSinitAndSterm() return true.



Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:4446
+
+  // Create __srterm function for the var decl.
+  llvm::Function *dtorStub = CGF.createAtExitStub(D, dtor, addr);

In this implementation we do not seem to have `__srterm` functions. I think we 
should refer to `__dtor` instead.



Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:4479
+
+  llvm::Value *NeedsDestruct = CGF.Builder.CreateIsNull(V, "guard.hasSrterm");
+

Srterm is not used in this implementation.
Suggestion:
guard.hasDtorCalled



Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:4481
+
+  llvm::BasicBlock *DestructCheckBlock = 
CGF.createBasicBlock("destruct.check");
+  llvm::BasicBlock *EndBlock = CGF.createBasicBlock("destruct.end");

I think we need a better naming for this and make it consistent with the end 
block below.
As it is for now, `destruct.check` is confusing, as we are not checking 
anything here and we are just going to call destructor directly in this block. 



Comment at: clang/test/CodeGen/static-init.cpp:10
   ~test();
 } t;
 

Would it be better to test static init for at least two global variable?
Testing only one global variable does not show the whole picture of AIX static 
init.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74166/new/

https://reviews.llvm.org/D74166



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D74166: [AIX][Frontend] Static init implementation for AIX considering no priority

[Xiangling Liao via Phabricator via cfe-commits]

Xiangling_L updated this revision to Diff 266087.
Xiangling_L added a comment.

Adjust  `mangleDynamicDestructor`;
Add assertion and FIXME for getUniqueModuleId


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74166/new/

https://reviews.llvm.org/D74166

Files:
  clang/include/clang/AST/Mangle.h
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/CodeGen/CGCXXABI.h
  clang/lib/CodeGen/CGDeclCXX.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGen/aix-constructor-attribute.cpp
  clang/test/CodeGen/aix-destructor-attribute.cpp
  clang/test/CodeGen/aix-init-priority-attribute.cpp
  clang/test/CodeGen/static-init.cpp

Index: clang/test/CodeGen/static-init.cpp
===
--- clang/test/CodeGen/static-init.cpp
+++ clang/test/CodeGen/static-init.cpp
@@ -1,12 +1,55 @@
-// RUN: not %clang_cc1 -triple powerpc-ibm-aix-xcoff -S -emit-llvm -x c++ %s \
-// RUN: -o /dev/null 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -S -emit-llvm -x c++ < %s \
+// RUN: | FileCheck %s
 
-// RUN: not %clang_cc1 -triple powerpc64-ibm-aix-xcoff -S -emit-llvm -x c++ %s \
-// RUN: -o /dev/null 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff -S -emit-llvm -x c++ < %s \
+// RUN: | FileCheck %s
 
 struct test {
   test();
   ~test();
 } t;
 
-// CHECK: error in backend: Static initialization has not been implemented on XL ABI yet.
+// CHECK: @llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @__sinit8000_clang_6a64b8be19fb12e74feab8a7a858f83b, i8* null }]
+// CHECK: @llvm.global_dtors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @__sterm8000_clang_6a64b8be19fb12e74feab8a7a858f83b, i8* null }]
+// CHECK: define internal void @__cxx_global_var_init() #0 {
+// CHECK: entry:
+// CHECK:   call void @_ZN4testC1Ev(%struct.test* @t)
+// CHECK:   %0 = call i32 @atexit(void ()* @__dtor_t)
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: define internal void @__dtor_t() #0 {
+// CHECK: entry:
+// CHECK:   call void @_ZN4testD1Ev(%struct.test* @t)
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: declare i32 @atexit(void ()*)
+
+// CHECK: define internal void @__cxx_global_var_destruct_t() #0 {
+// CHECK: entry:
+// CHECK:   %0 = call i32 @unatexit(void ()* @__dtor_t)
+// CHECK:   %guard.hasSrterm = icmp eq i32 %0, 0
+// CHECK:   br i1 %guard.hasSrterm, label %destruct.check, label %destruct.end
+
+// CHECK: destruct.check:
+// CHECK:   call void @__dtor_t()
+// CHECK:   br label %destruct.end
+
+// CHECK: destruct.end:
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: declare i32 @unatexit(void ()*)
+
+// CHECK: define dso_local void @__sinit8000_clang_6a64b8be19fb12e74feab8a7a858f83b() #0 {
+// CHECK: entry:
+// CHECK:   call void @__cxx_global_var_init()
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: define dso_local void @__sterm8000_clang_6a64b8be19fb12e74feab8a7a858f83b() #0 {
+// CHECK: entry:
+// CHECK:   call void @__cxx_global_var_destruct_t()
+// CHECK:   ret void
+// CHECK: }
Index: clang/test/CodeGen/aix-init-priority-attribute.cpp
===
--- /dev/null
+++ clang/test/CodeGen/aix-init-priority-attribute.cpp
@@ -0,0 +1,17 @@
+// RUN: not %clang_cc1 -triple powerpc-ibm-aix-xcoff -x c++ -emit-llvm < %s \
+// RUN: 2>&1 | FileCheck %s
+// RUN: not %clang_cc1 -triple powerpc64-ibm-aix-xcoff -x c++ -emit-llvm < %s \
+// RUN: 2>&1 | FileCheck %s
+
+class test {
+  int a;
+
+public:
+  test(int c) { a = c; }
+  ~test() { a = 0; }
+};
+
+__attribute__((init_priority(2000)))
+test t(1);
+
+// CHECK: fatal error: error in backend: 'init_priority' attribute unsupported on AIX yet
Index: clang/test/CodeGen/aix-destructor-attribute.cpp
===
--- /dev/null
+++ clang/test/CodeGen/aix-destructor-attribute.cpp
@@ -0,0 +1,18 @@
+// RUN: not %clang_cc1 -triple powerpc-ibm-aix-xcoff -x c++ -emit-llvm < %s \
+// RUN: 2>&1 | FileCheck %s
+// RUN: not %clang_cc1 -triple powerpc64-ibm-aix-xcoff -x c++ -emit-llvm < %s \
+// RUN: 2>&1 | FileCheck %s
+
+int bar() __attribute__((destructor(180)));
+
+class test {
+  int a;
+
+public:
+  test(int c) { a = c; }
+  ~test() { a = 0; }
+};
+
+test t(1);
+
+// CHECK: fatal error: error in backend: 'destructor' attribute unsupported on AIX yet
Index: clang/test/CodeGen/aix-constructor-attribute.cpp
===
--- /dev/null
+++ clang/test/CodeGen/aix-constructor-attribute.cpp
@@ -0,0 +1,18 @@
+// RUN: not %clang_cc1 -triple powerpc-ibm-aix-xcoff -x c++ -emit-llvm < %s \
+// RUN: 2>&1 | FileCheck %s
+// RUN: not %clang_cc1 -triple powerpc64-ibm-aix-xcoff -x c++ -emit-llvm < 

[PATCH] D74166: [AIX][Frontend] Static init implementation for AIX considering no priority

[Xiangling Liao via Phabricator via cfe-commits]

Xiangling_L updated this revision to Diff 264926.
Xiangling_L marked an inline comment as done.
Xiangling_L added a comment.

Fix the linkage types;
Adjust the formatting;
Update the testcase;


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74166/new/

https://reviews.llvm.org/D74166

Files:
  clang/include/clang/AST/Mangle.h
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/CodeGen/CGCXXABI.h
  clang/lib/CodeGen/CGDeclCXX.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGen/aix-constructor-attribute.cpp
  clang/test/CodeGen/aix-destructor-attribute.cpp
  clang/test/CodeGen/aix-init-priority-attribute.cpp
  clang/test/CodeGen/static-init.cpp

Index: clang/test/CodeGen/static-init.cpp
===
--- clang/test/CodeGen/static-init.cpp
+++ clang/test/CodeGen/static-init.cpp
@@ -1,12 +1,55 @@
-// RUN: not %clang_cc1 -triple powerpc-ibm-aix-xcoff -S -emit-llvm -x c++ %s \
-// RUN: -o /dev/null 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -S -emit-llvm -x c++ < %s \
+// RUN: | FileCheck %s
 
-// RUN: not %clang_cc1 -triple powerpc64-ibm-aix-xcoff -S -emit-llvm -x c++ %s \
-// RUN: -o /dev/null 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff -S -emit-llvm -x c++ < %s \
+// RUN: | FileCheck %s
 
 struct test {
   test();
   ~test();
 } t;
 
-// CHECK: error in backend: Static initialization has not been implemented on XL ABI yet.
+// CHECK: @llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @__sinit8000_clang_6a64b8be19fb12e74feab8a7a858f83b, i8* null }]
+// CHECK: @llvm.global_dtors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @__sterm8000_clang_6a64b8be19fb12e74feab8a7a858f83b, i8* null }]
+// CHECK: define internal void @__cxx_global_var_init() #0 {
+// CHECK: entry:
+// CHECK:   call void @_ZN4testC1Ev(%struct.test* @t)
+// CHECK:   %0 = call i32 @atexit(void ()* @__dtor_t)
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: define internal void @__dtor_t() #0 {
+// CHECK: entry:
+// CHECK:   call void @_ZN4testD1Ev(%struct.test* @t)
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: declare i32 @atexit(void ()*)
+
+// CHECK: define internal void @__cxx_global_var_destruct_t() #0 {
+// CHECK: entry:
+// CHECK:   %0 = call i32 @unatexit(void ()* @__dtor_t)
+// CHECK:   %guard.hasSrterm = icmp eq i32 %0, 0
+// CHECK:   br i1 %guard.hasSrterm, label %destruct.check, label %destruct.end
+
+// CHECK: destruct.check:
+// CHECK:   call void @__dtor_t()
+// CHECK:   br label %destruct.end
+
+// CHECK: destruct.end:
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: declare i32 @unatexit(void ()*)
+
+// CHECK: define dso_local void @__sinit8000_clang_6a64b8be19fb12e74feab8a7a858f83b() #0 {
+// CHECK: entry:
+// CHECK:   call void @__cxx_global_var_init()
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: define dso_local void @__sterm8000_clang_6a64b8be19fb12e74feab8a7a858f83b() #0 {
+// CHECK: entry:
+// CHECK:   call void @__cxx_global_var_destruct_t()
+// CHECK:   ret void
+// CHECK: }
Index: clang/test/CodeGen/aix-init-priority-attribute.cpp
===
--- /dev/null
+++ clang/test/CodeGen/aix-init-priority-attribute.cpp
@@ -0,0 +1,17 @@
+// RUN: not %clang_cc1 -triple powerpc-ibm-aix-xcoff -x c++ -emit-llvm < %s \
+// RUN: 2>&1 | FileCheck %s
+// RUN: not %clang_cc1 -triple powerpc64-ibm-aix-xcoff -x c++ -emit-llvm < %s \
+// RUN: 2>&1 | FileCheck %s
+
+class test {
+  int a;
+
+public:
+  test(int c) { a = c; }
+  ~test() { a = 0; }
+};
+
+__attribute__((init_priority(2000)))
+test t(1);
+
+// CHECK: fatal error: error in backend: 'init_priority' attribute unsupported on AIX yet
Index: clang/test/CodeGen/aix-destructor-attribute.cpp
===
--- /dev/null
+++ clang/test/CodeGen/aix-destructor-attribute.cpp
@@ -0,0 +1,18 @@
+// RUN: not %clang_cc1 -triple powerpc-ibm-aix-xcoff -x c++ -emit-llvm < %s \
+// RUN: 2>&1 | FileCheck %s
+// RUN: not %clang_cc1 -triple powerpc64-ibm-aix-xcoff -x c++ -emit-llvm < %s \
+// RUN: 2>&1 | FileCheck %s
+
+int bar() __attribute__((destructor(180)));
+
+class test {
+  int a;
+
+public:
+  test(int c) { a = c; }
+  ~test() { a = 0; }
+};
+
+test t(1);
+
+// CHECK: fatal error: error in backend: 'destructor' attribute unsupported on AIX yet
Index: clang/test/CodeGen/aix-constructor-attribute.cpp
===
--- /dev/null
+++ clang/test/CodeGen/aix-constructor-attribute.cpp
@@ -0,0 +1,18 @@
+// RUN: not %clang_cc1 -triple powerpc-ibm-aix-xcoff -x c++ -emit-llvm < %s \
+// RUN: 2>&1 | FileCheck %s
+// RUN: not %clang_cc1 -triple 

[PATCH] D74166: [AIX][Frontend] Static init implementation for AIX considering no priority

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/test/CodeGen/static-init.cpp:14
+// CHECK: @llvm.global_dtors = appending global [1 x { i32, void ()*, i8* }] 
[{ i32, void ()*, i8* } { i32 65535, void ()* 
@__sterm8000_clang_b2e4830f1c9d2d063e5ea946868f3bfd, i8* null }]
+// CHECK: define dso_local void @__cxx_global_var_init() #0 {
+// CHECK: entry:

Shouldn't this be `internal`?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74166/new/

https://reviews.llvm.org/D74166



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D74166: [AIX][Frontend] Static init implementation for AIX considering no priority

[Xiangling Liao via Phabricator via cfe-commits]

Xiangling_L updated this revision to Diff 264009.
Xiangling_L added a comment.

Clean `clang-tidy` warnings and `clang-format` errors


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74166/new/

https://reviews.llvm.org/D74166

Files:
  clang/include/clang/AST/Mangle.h
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/CodeGen/CGCXXABI.h
  clang/lib/CodeGen/CGDeclCXX.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGen/aix-constructor-attribute.cpp
  clang/test/CodeGen/aix-destructor-attribute.cpp
  clang/test/CodeGen/aix-init-priority-attribute.cpp
  clang/test/CodeGen/static-init.cpp

Index: clang/test/CodeGen/static-init.cpp
===
--- clang/test/CodeGen/static-init.cpp
+++ clang/test/CodeGen/static-init.cpp
@@ -1,12 +1,55 @@
-// RUN: not %clang_cc1 -triple powerpc-ibm-aix-xcoff -S -emit-llvm -x c++ %s \
-// RUN: -o /dev/null 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -S -emit-llvm -x c++ < %s \
+// RUN: | FileCheck %s
 
-// RUN: not %clang_cc1 -triple powerpc64-ibm-aix-xcoff -S -emit-llvm -x c++ %s \
-// RUN: -o /dev/null 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff -S -emit-llvm -x c++ < %s \
+// RUN: | FileCheck %s
 
 struct test {
   test();
   ~test();
 } t;
 
-// CHECK: error in backend: Static initialization has not been implemented on XL ABI yet.
+// CHECK: @llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @__sinit8000_clang_b2e4830f1c9d2d063e5ea946868f3bfd, i8* null }]
+// CHECK: @llvm.global_dtors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @__sterm8000_clang_b2e4830f1c9d2d063e5ea946868f3bfd, i8* null }]
+// CHECK: define dso_local void @__cxx_global_var_init() #0 {
+// CHECK: entry:
+// CHECK:   call void @_ZN4testC1Ev(%struct.test* @t)
+// CHECK:   %0 = call i32 @atexit(void ()* @__dtor_t)
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: define dso_local void @__dtor_t() #0 {
+// CHECK: entry:
+// CHECK:   call void @_ZN4testD1Ev(%struct.test* @t)
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: declare i32 @atexit(void ()*)
+
+// CHECK: define dso_local void @__cxx_global_var_destruct_t() #0 {
+// CHECK: entry:
+// CHECK:   %0 = call i32 @unatexit(void ()* @__dtor_t)
+// CHECK:   %guard.hasSrterm = icmp eq i32 %0, 0
+// CHECK:   br i1 %guard.hasSrterm, label %destruct.check, label %destruct.end
+
+// CHECK: destruct.check:
+// CHECK:   call void @__dtor_t()
+// CHECK:   br label %destruct.end
+
+// CHECK: destruct.end:
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: declare i32 @unatexit(void ()*)
+
+// CHECK: define dso_local void @__sinit8000_clang_b2e4830f1c9d2d063e5ea946868f3bfd() #0 {
+// CHECK: entry:
+// CHECK:   call void @__cxx_global_var_init()
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: define dso_local void @__sterm8000_clang_b2e4830f1c9d2d063e5ea946868f3bfd() #0 {
+// CHECK: entry:
+// CHECK:   call void @__cxx_global_var_destruct_t()
+// CHECK:   ret void
+// CHECK: }
Index: clang/test/CodeGen/aix-init-priority-attribute.cpp
===
--- /dev/null
+++ clang/test/CodeGen/aix-init-priority-attribute.cpp
@@ -0,0 +1,16 @@
+// RUN: not %clang_cc1 -triple powerpc-ibm-aix-xcoff -x c++ -emit-llvm < %s \
+// RUN: 2>&1 | FileCheck %s
+// RUN: not %clang_cc1 -triple powerpc64-ibm-aix-xcoff -x c++ -emit-llvm < %s \
+// RUN: 2>&1 | FileCheck %s
+
+class test {
+  int a;
+public:
+  test(int c) {a = c;}
+  ~test() {a = 0;}
+};
+
+__attribute__((init_priority(2000)))
+test t(1);
+
+// CHECK: fatal error: error in backend: 'init_priority' attribute unsupported on AIX yet
Index: clang/test/CodeGen/aix-destructor-attribute.cpp
===
--- /dev/null
+++ clang/test/CodeGen/aix-destructor-attribute.cpp
@@ -0,0 +1,17 @@
+// RUN: not %clang_cc1 -triple powerpc-ibm-aix-xcoff -x c++ -emit-llvm < %s \
+// RUN: 2>&1 | FileCheck %s
+// RUN: not %clang_cc1 -triple powerpc64-ibm-aix-xcoff -x c++ -emit-llvm < %s \
+// RUN: 2>&1 | FileCheck %s
+
+int bar() __attribute__((destructor(180)));
+
+class test {
+  int a;
+public:
+  test(int c) {a = c;}
+  ~test() {a = 0;}
+};
+
+test t(1);
+
+// CHECK: fatal error: error in backend: 'destructor' attribute unsupported on AIX yet
Index: clang/test/CodeGen/aix-constructor-attribute.cpp
===
--- /dev/null
+++ clang/test/CodeGen/aix-constructor-attribute.cpp
@@ -0,0 +1,17 @@
+// RUN: not %clang_cc1 -triple powerpc-ibm-aix-xcoff -x c++ -emit-llvm < %s \
+// RUN: 2>&1 | FileCheck %s
+// RUN: not %clang_cc1 -triple powerpc64-ibm-aix-xcoff -x c++ -emit-llvm < %s \
+// RUN: 2>&1 | FileCheck %s
+

[PATCH] D74166: [AIX][Frontend] Static init implementation for AIX considering no priority

[Xiangling Liao via Phabricator via cfe-commits]

Xiangling_L updated this revision to Diff 263843.
Xiangling_L added a comment.

Fix a minor issue in the testcase


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74166/new/

https://reviews.llvm.org/D74166

Files:
  clang/include/clang/AST/Mangle.h
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/CodeGen/CGCXXABI.h
  clang/lib/CodeGen/CGDeclCXX.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGen/aix-constructor-attribute.cpp
  clang/test/CodeGen/aix-destructor-attribute.cpp
  clang/test/CodeGen/aix-init-priority-attribute.cpp
  clang/test/CodeGen/static-init.cpp

Index: clang/test/CodeGen/static-init.cpp
===
--- clang/test/CodeGen/static-init.cpp
+++ clang/test/CodeGen/static-init.cpp
@@ -1,12 +1,55 @@
-// RUN: not %clang_cc1 -triple powerpc-ibm-aix-xcoff -S -emit-llvm -x c++ %s \
-// RUN: -o /dev/null 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -S -emit-llvm -x c++ < %s \
+// RUN: | FileCheck %s
 
-// RUN: not %clang_cc1 -triple powerpc64-ibm-aix-xcoff -S -emit-llvm -x c++ %s \
-// RUN: -o /dev/null 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff -S -emit-llvm -x c++ < %s \
+// RUN: | FileCheck %s
 
 struct test {
   test();
   ~test();
 } t;
 
-// CHECK: error in backend: Static initialization has not been implemented on XL ABI yet.
+// CHECK: @llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @__sinit8000_clang_b2e4830f1c9d2d063e5ea946868f3bfd, i8* null }]
+// CHECK: @llvm.global_dtors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @__sterm8000_clang_b2e4830f1c9d2d063e5ea946868f3bfd, i8* null }]
+// CHECK: define dso_local void @__cxx_global_var_init() #0 {
+// CHECK: entry:
+// CHECK:   call void @_ZN4testC1Ev(%struct.test* @t)
+// CHECK:   %0 = call i32 @atexit(void ()* @__dtor_t)
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: define dso_local void @__dtor_t() #0 {
+// CHECK: entry:
+// CHECK:   call void @_ZN4testD1Ev(%struct.test* @t)
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: declare i32 @atexit(void ()*)
+
+// CHECK: define dso_local void @__cxx_global_var_destruct_t() #0 {
+// CHECK: entry:
+// CHECK:   %0 = call i32 @unatexit(void ()* @__dtor_t)
+// CHECK:   %guard.hasSrterm = icmp eq i32 %0, 0
+// CHECK:   br i1 %guard.hasSrterm, label %destruct.check, label %destruct.end
+
+// CHECK: destruct.check:
+// CHECK:   call void @__dtor_t()
+// CHECK:   br label %destruct.end
+
+// CHECK: destruct.end:
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: declare i32 @unatexit(void ()*)
+
+// CHECK: define dso_local void @__sinit8000_clang_b2e4830f1c9d2d063e5ea946868f3bfd() #0 {
+// CHECK: entry:
+// CHECK:   call void @__cxx_global_var_init()
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: define dso_local void @__sterm8000_clang_b2e4830f1c9d2d063e5ea946868f3bfd() #0 {
+// CHECK: entry:
+// CHECK:   call void @__cxx_global_var_destruct_t()
+// CHECK:   ret void
+// CHECK: }
Index: clang/test/CodeGen/aix-init-priority-attribute.cpp
===
--- /dev/null
+++ clang/test/CodeGen/aix-init-priority-attribute.cpp
@@ -0,0 +1,16 @@
+// RUN: not %clang_cc1 -triple powerpc-ibm-aix-xcoff -x c++ -emit-llvm < %s \
+// RUN: 2>&1 | FileCheck %s
+// RUN: not %clang_cc1 -triple powerpc64-ibm-aix-xcoff -x c++ -emit-llvm < %s \
+// RUN: 2>&1 | FileCheck %s
+
+class test {
+   int a;
+public:
+test(int c) {a = c;}
+~test() {a = 0;}
+};
+
+__attribute__ ((init_priority (2000)))
+test t(1);
+
+// CHECK: fatal error: error in backend: 'init_priority' attribute unsupported on AIX yet
Index: clang/test/CodeGen/aix-destructor-attribute.cpp
===
--- /dev/null
+++ clang/test/CodeGen/aix-destructor-attribute.cpp
@@ -0,0 +1,17 @@
+// RUN: not %clang_cc1 -triple powerpc-ibm-aix-xcoff -x c++ -emit-llvm < %s \
+// RUN: 2>&1 | FileCheck %s
+// RUN: not %clang_cc1 -triple powerpc64-ibm-aix-xcoff -x c++ -emit-llvm < %s \
+// RUN: 2>&1 | FileCheck %s
+
+int bar() __attribute__((destructor(180)));
+
+class test {
+   int a;
+public:
+test(int c) {a = c;}
+~test() {a = 0;}
+};
+
+test t(1);
+
+// CHECK: fatal error: error in backend: 'destructor' attribute unsupported on AIX yet
Index: clang/test/CodeGen/aix-constructor-attribute.cpp
===
--- /dev/null
+++ clang/test/CodeGen/aix-constructor-attribute.cpp
@@ -0,0 +1,17 @@
+// RUN: not %clang_cc1 -triple powerpc-ibm-aix-xcoff -x c++ -emit-llvm < %s \
+// RUN: 2>&1 | FileCheck %s
+// RUN: not %clang_cc1 -triple powerpc64-ibm-aix-xcoff -x c++ -emit-llvm < %s \
+// RUN: 2>&1 | FileCheck %s
+
+int 

[PATCH] D74166: [AIX][Frontend] Static init implementation for AIX considering no priority

[Xiangling Liao via Phabricator via cfe-commits]

Xiangling_L updated this revision to Diff 263499.
Xiangling_L marked 3 inline comments as done.
Xiangling_L added a comment.

Updated the warnings to `report_fatal_error`;
Update the testcases;


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74166/new/

https://reviews.llvm.org/D74166

Files:
  clang/include/clang/AST/Mangle.h
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/CodeGen/CGCXXABI.h
  clang/lib/CodeGen/CGDeclCXX.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGen/aix-constructor-attribute.cpp
  clang/test/CodeGen/aix-destructor-attribute.cpp
  clang/test/CodeGen/aix-init-priority-attribute.cpp
  clang/test/CodeGen/static-init.cpp

Index: clang/test/CodeGen/static-init.cpp
===
--- clang/test/CodeGen/static-init.cpp
+++ clang/test/CodeGen/static-init.cpp
@@ -1,12 +1,55 @@
-// RUN: not %clang_cc1 -triple powerpc-ibm-aix-xcoff -S -emit-llvm -x c++ %s \
-// RUN: -o /dev/null 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -S -emit-llvm -x c++ < %s \
+// RUN: | FileCheck %s
 
-// RUN: not %clang_cc1 -triple powerpc64-ibm-aix-xcoff -S -emit-llvm -x c++ %s \
-// RUN: -o /dev/null 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff -S -emit-llvm -x c++ < %s \
+// RUN: | FileCheck %s
 
 struct test {
   test();
   ~test();
 } t;
 
-// CHECK: error in backend: Static initialization has not been implemented on XL ABI yet.
+// CHECK: @llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @__sinit8000_clang_b2e4830f1c9d2d063e5ea946868f3bfd, i8* null }]
+// CHECK: @llvm.global_dtors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @__sterm8000_clang_b2e4830f1c9d2d063e5ea946868f3bfd, i8* null }]
+// CHECK: define dso_local void @__cxx_global_var_init() {
+// CHECK: entry:
+// CHECK:   call void @_ZN4testC1Ev(%struct.test* @t)
+// CHECK:   %0 = call i32 @atexit(void ()* @__dtor_t)
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: define dso_local void @__dtor_t() {
+// CHECK: entry:
+// CHECK:   call void @_ZN4testD1Ev(%struct.test* @t)
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: declare i32 @atexit(void ()*)
+
+// CHECK: define dso_local void @__cxx_global_var_destruct_t() {
+// CHECK: entry:
+// CHECK:   %0 = call i32 @unatexit(void ()* @__dtor_t)
+// CHECK:   %guard.hasSrterm = icmp eq i32 %0, 0
+// CHECK:   br i1 %guard.hasSrterm, label %destruct.check, label %destruct.end
+
+// CHECK: destruct.check:
+// CHECK:   call void @__dtor_t()
+// CHECK:   br label %destruct.end
+
+// CHECK: destruct.end:
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: declare i32 @unatexit(void ()*)
+
+// CHECK: define dso_local void @__sinit8000_clang_b2e4830f1c9d2d063e5ea946868f3bfd() {
+// CHECK: entry:
+// CHECK:   call void @__cxx_global_var_init()
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: define dso_local void @__sterm8000_clang_b2e4830f1c9d2d063e5ea946868f3bfd() {
+// CHECK: entry:
+// CHECK:   call void @__cxx_global_var_destruct_t()
+// CHECK:   ret void
+// CHECK: }
Index: clang/test/CodeGen/aix-init-priority-attribute.cpp
===
--- /dev/null
+++ clang/test/CodeGen/aix-init-priority-attribute.cpp
@@ -0,0 +1,16 @@
+// RUN: not %clang_cc1 -triple powerpc-ibm-aix-xcoff -x c++ -emit-llvm < %s \
+// RUN: 2>&1 | FileCheck %s
+// RUN: not %clang_cc1 -triple powerpc64-ibm-aix-xcoff -x c++ -emit-llvm < %s \
+// RUN: 2>&1 | FileCheck %s
+
+class test {
+   int a;
+public:
+test(int c) {a = c;}
+~test() {a = 0;}
+};
+
+__attribute__ ((init_priority (2000)))
+test t(1);
+
+// CHECK: fatal error: error in backend: 'init_priority' attribute unsupported on AIX yet
Index: clang/test/CodeGen/aix-destructor-attribute.cpp
===
--- /dev/null
+++ clang/test/CodeGen/aix-destructor-attribute.cpp
@@ -0,0 +1,17 @@
+// RUN: not %clang_cc1 -triple powerpc-ibm-aix-xcoff -x c++ -emit-llvm < %s \
+// RUN: 2>&1 | FileCheck %s
+// RUN: not %clang_cc1 -triple powerpc64-ibm-aix-xcoff -x c++ -emit-llvm < %s \
+// RUN: 2>&1 | FileCheck %s
+
+int bar() __attribute__((destructor(180)));
+
+class test {
+   int a;
+public:
+test(int c) {a = c;}
+~test() {a = 0;}
+};
+
+test t(1);
+
+// CHECK: fatal error: error in backend: 'destructor' attribute unsupported on AIX yet
Index: clang/test/CodeGen/aix-constructor-attribute.cpp
===
--- /dev/null
+++ clang/test/CodeGen/aix-constructor-attribute.cpp
@@ -0,0 +1,17 @@
+// RUN: not %clang_cc1 -triple powerpc-ibm-aix-xcoff -x c++ -emit-llvm < %s \
+// RUN: 2>&1 | FileCheck %s
+// RUN: not %clang_cc1 -triple powerpc64-ibm-aix-xcoff -x c++ 

[PATCH] D74166: [AIX][Frontend] Static init implementation for AIX considering no priority

[Xiangling Liao via Phabricator via cfe-commits]

Xiangling_L marked 7 inline comments as done.
Xiangling_L added inline comments.



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:691
   AddGlobalDtor(Fn);
+  CXXGlobalDtors.clear();
 }

ZarkoCA wrote:
> I may be missing something but why do we need this now, as opposed to not 
> needing it before? Why didn't we need to clear the CXXGlobalDtors after 
> emitting the function function before?
No, you didn't miss anything here. I just followed what 
`EmitCXXGlobalInitFunc()` does, which is to clear the std::vecotr once we are 
certain it's useless.



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:6898
   case ParsedAttr::AT_Constructor:
-handleConstructorAttr(S, D, AL);
+if (S.Context.getTargetInfo().getTriple().isOSAIX())
+  S.Diag(AL.getLoc(), diag::warn_attribute_type_not_supported) << AL << "";

aaron.ballman wrote:
> This change (and the others like it) should be done within Attr.td and not 
> exposed here. You should make these attributes target-specific.
> 
> You should also update AttrDocs.td for these attributes to document that 
> they're not supported on AIX.
Thanks for your comments. As I mentioned in the below testcase, those three 
attributes actually will be supported by the follow-up patches for AIX.  I will 
update them to `report_fatal_error` instead.



Comment at: clang/test/CodeGen/aix-priority-attribute.cpp:1-4
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -x c++ -emit-llvm < %s 2>&1 | 
\
+// RUN: FileCheck %s
+// RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff -x c++ -emit-llvm < %s 2>&1 
| \
+// RUN: FileCheck %s

aaron.ballman wrote:
> I think this test file should live in SemaCXX instead, as this is not testing 
> the codegen behavior, but testing the semantic checking behavior.
Actually we will support those three attributes in the future, so the warning 
are placeholders waiting for the future upgrade where we do want to check the 
codegen results. 

I agree the warnings here are confusing, I will update them with 
report_fatal_error.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74166/new/

https://reviews.llvm.org/D74166



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D74166: [AIX][Frontend] Static init implementation for AIX considering no priority

[Zarko Todorovski via Phabricator via cfe-commits]

ZarkoCA added inline comments.



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:691
   AddGlobalDtor(Fn);
+  CXXGlobalDtors.clear();
 }

I may be missing something but why do we need this now, as opposed to not 
needing it before? Why didn't we need to clear the CXXGlobalDtors after 
emitting the function function before?



Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:4442
+  if (D.getTLSKind() != VarDecl::TLS_None)
+llvm::report_fatal_error("Thread local storage unimplemented on AIX yet.");
+

Style guide now says no capitalization and no punctuation at the end for 
`report_fatal_error` messages.  


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74166/new/

https://reviews.llvm.org/D74166



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D74166: [AIX][Frontend] Static init implementation for AIX considering no priority

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:6898
   case ParsedAttr::AT_Constructor:
-handleConstructorAttr(S, D, AL);
+if (S.Context.getTargetInfo().getTriple().isOSAIX())
+  S.Diag(AL.getLoc(), diag::warn_attribute_type_not_supported) << AL << "";

This change (and the others like it) should be done within Attr.td and not 
exposed here. You should make these attributes target-specific.

You should also update AttrDocs.td for these attributes to document that 
they're not supported on AIX.



Comment at: clang/test/CodeGen/aix-priority-attribute.cpp:1-4
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -x c++ -emit-llvm < %s 2>&1 | 
\
+// RUN: FileCheck %s
+// RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff -x c++ -emit-llvm < %s 2>&1 
| \
+// RUN: FileCheck %s

I think this test file should live in SemaCXX instead, as this is not testing 
the codegen behavior, but testing the semantic checking behavior.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74166/new/

https://reviews.llvm.org/D74166



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D74166: [AIX][Frontend] Static init implementation for AIX considering no priority

[Xiangling Liao via Phabricator via cfe-commits]

Xiangling_L updated this revision to Diff 261602.
Xiangling_L added a comment.
Herald added a reviewer: aaron.ballman.

Fix a minor issue


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74166/new/

https://reviews.llvm.org/D74166

Files:
  clang/include/clang/AST/Mangle.h
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/CodeGen/CGCXXABI.h
  clang/lib/CodeGen/CGDeclCXX.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGen/aix-priority-attribute.cpp
  clang/test/CodeGen/static-init.cpp

Index: clang/test/CodeGen/static-init.cpp
===
--- clang/test/CodeGen/static-init.cpp
+++ clang/test/CodeGen/static-init.cpp
@@ -1,12 +1,55 @@
-// RUN: not %clang_cc1 -triple powerpc-ibm-aix-xcoff -S -emit-llvm -x c++ %s \
-// RUN: -o /dev/null 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -S -emit-llvm -x c++ < %s \
+// RUN: | FileCheck %s
 
-// RUN: not %clang_cc1 -triple powerpc64-ibm-aix-xcoff -S -emit-llvm -x c++ %s \
-// RUN: -o /dev/null 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff -S -emit-llvm -x c++ < %s \
+// RUN: | FileCheck %s
 
 struct test {
   test();
   ~test();
 } t;
 
-// CHECK: error in backend: Static initialization has not been implemented on XL ABI yet.
+// CHECK: @llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @__sinit8000_clang_b2e4830f1c9d2d063e5ea946868f3bfd, i8* null }]
+// CHECK: @llvm.global_dtors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @__sterm8000_clang_b2e4830f1c9d2d063e5ea946868f3bfd, i8* null }]
+// CHECK: define dso_local void @__cxx_global_var_init() #0 {
+// CHECK: entry:
+// CHECK:   call void @_ZN4testC1Ev(%struct.test* @t)
+// CHECK:   %0 = call i32 @atexit(void ()* @__dtor_t)
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: define dso_local void @__dtor_t() #0 {
+// CHECK: entry:
+// CHECK:   call void @_ZN4testD1Ev(%struct.test* @t)
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: declare i32 @atexit(void ()*) #3
+
+// CHECK: define dso_local void @__cxx_global_var_destruct_t() #0 {
+// CHECK: entry:
+// CHECK:   %0 = call i32 @unatexit(void ()* @__dtor_t)
+// CHECK:   %guard.hasSrterm = icmp eq i32 %0, 0
+// CHECK:   br i1 %guard.hasSrterm, label %destruct.check, label %destruct.end
+
+// CHECK: destruct.check:
+// CHECK:   call void @__dtor_t()
+// CHECK:   br label %destruct.end
+
+// CHECK: destruct.end:
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: declare i32 @unatexit(void ()*) #3
+
+// CHECK: define dso_local void @__sinit8000_clang_b2e4830f1c9d2d063e5ea946868f3bfd() #0 {
+// CHECK: entry:
+// CHECK:   call void @__cxx_global_var_init()
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: define dso_local void @__sterm8000_clang_b2e4830f1c9d2d063e5ea946868f3bfd() #0 {
+// CHECK: entry:
+// CHECK:   call void @__cxx_global_var_destruct_t()
+// CHECK:   ret void
+// CHECK: }
Index: clang/test/CodeGen/aix-priority-attribute.cpp
===
--- /dev/null
+++ clang/test/CodeGen/aix-priority-attribute.cpp
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -x c++ -emit-llvm < %s 2>&1 | \
+// RUN: FileCheck %s
+// RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff -x c++ -emit-llvm < %s 2>&1 | \
+// RUN: FileCheck %s
+
+int foo() __attribute__((constructor(180)));
+int bar() __attribute__((destructor(180)));
+
+class test {
+   int a;
+public:
+test(int c) {a = c;}
+~test() {a = 0;}
+};
+
+__attribute__ ((init_priority (2000)))
+test t(1);
+
+// CHECK: warning: 'constructor' attribute argument not supported:
+// CHECK: int foo() __attribute__((constructor(180)));
+
+// CHECK: warning: 'destructor' attribute argument not supported:
+// check: int bar() __attribute__((destructor(180)));
+
+// CHECK: warning: 'init_priority' attribute argument not supported:
+// CHECK: __attribute__ ((init_priority (2000)))
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -6895,13 +6895,19 @@
 handlePassObjectSizeAttr(S, D, AL);
 break;
   case ParsedAttr::AT_Constructor:
-handleConstructorAttr(S, D, AL);
+if (S.Context.getTargetInfo().getTriple().isOSAIX())
+  S.Diag(AL.getLoc(), diag::warn_attribute_type_not_supported) << AL << "";
+else
+  handleConstructorAttr(S, D, AL);
 break;
   case ParsedAttr::AT_Deprecated:
 handleDeprecatedAttr(S, D, AL);
 break;
   case ParsedAttr::AT_Destructor:
-handleDestructorAttr(S, D, AL);
+if (S.Context.getTargetInfo().getTriple().isOSAIX())
+  S.Diag(AL.getLoc(), diag::warn_attribute_type_not_supported) << AL << 

[PATCH] D74166: [AIX][Frontend] Static init implementation for AIX considering no priority

[David Tenty via Phabricator via cfe-commits]

daltenty added inline comments.



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:583
+  if (UseSinitAndSterm)
+GlobalUniqueModuleId = getUniqueModuleId(()).substr(1);
+

if `getUniqueModuleId()` returns an empy string, which it does for modules 
which don't export any symbols, the `substr()` will throw an `OutOfRange` 
exception.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74166/new/

https://reviews.llvm.org/D74166



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D74166: [AIX][Frontend] Static init implementation for AIX considering no priority

[Xiangling Liao via Phabricator via cfe-commits]

Xiangling_L updated this revision to Diff 255456.
Xiangling_L added a comment.

Rebase on the latest master branch;


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74166/new/

https://reviews.llvm.org/D74166

Files:
  clang/include/clang/AST/Mangle.h
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/CodeGen/CGCXXABI.h
  clang/lib/CodeGen/CGDeclCXX.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGen/aix-priority-attribute.cpp
  clang/test/CodeGen/static-init.cpp

Index: clang/test/CodeGen/static-init.cpp
===
--- clang/test/CodeGen/static-init.cpp
+++ clang/test/CodeGen/static-init.cpp
@@ -1,12 +1,55 @@
-// RUN: not %clang_cc1 -triple powerpc-ibm-aix-xcoff -S -emit-llvm -x c++ %s \
-// RUN: -o /dev/null 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -S -emit-llvm -x c++ < %s \
+// RUN: | FileCheck %s
 
-// RUN: not %clang_cc1 -triple powerpc64-ibm-aix-xcoff -S -emit-llvm -x c++ %s \
-// RUN: -o /dev/null 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff -S -emit-llvm -x c++ < %s \
+// RUN: | FileCheck %s
 
 struct test {
   test();
   ~test();
 } t;
 
-// CHECK: error in backend: Static initialization has not been implemented on XL ABI yet.
+// CHECK: @llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @__sinit8000_clang_b2e4830f1c9d2d063e5ea946868f3bfd, i8* null }]
+// CHECK: @llvm.global_dtors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @__sterm8000_clang_b2e4830f1c9d2d063e5ea946868f3bfd, i8* null }]
+// CHECK: define dso_local void @__cxx_global_var_init() #0 {
+// CHECK: entry:
+// CHECK:   call void @_ZN4testC1Ev(%struct.test* @t)
+// CHECK:   %0 = call i32 @atexit(void ()* @__dtor_t)
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: define dso_local void @__dtor_t() #0 {
+// CHECK: entry:
+// CHECK:   call void @_ZN4testD1Ev(%struct.test* @t)
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: declare i32 @atexit(void ()*) #3
+
+// CHECK: define dso_local void @__cxx_global_var_destruct_t() #0 {
+// CHECK: entry:
+// CHECK:   %0 = call i32 @unatexit(void ()* @__dtor_t)
+// CHECK:   %guard.hasSrterm = icmp eq i32 %0, 0
+// CHECK:   br i1 %guard.hasSrterm, label %destruct.check, label %destruct.end
+
+// CHECK: destruct.check:
+// CHECK:   call void @__dtor_t()
+// CHECK:   br label %destruct.end
+
+// CHECK: destruct.end:
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: declare i32 @unatexit(void ()*) #3
+
+// CHECK: define dso_local void @__sinit8000_clang_b2e4830f1c9d2d063e5ea946868f3bfd() #0 {
+// CHECK: entry:
+// CHECK:   call void @__cxx_global_var_init()
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: define dso_local void @__sterm8000_clang_b2e4830f1c9d2d063e5ea946868f3bfd() #0 {
+// CHECK: entry:
+// CHECK:   call void @__cxx_global_var_destruct_t()
+// CHECK:   ret void
+// CHECK: }
Index: clang/test/CodeGen/aix-priority-attribute.cpp
===
--- /dev/null
+++ clang/test/CodeGen/aix-priority-attribute.cpp
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -x c++ -emit-llvm < %s 2>&1 | \
+// RUN: FileCheck %s
+// RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff -x c++ -emit-llvm < %s 2>&1 | \
+// RUN: FileCheck %s
+
+int foo() __attribute__((constructor(180)));
+int bar() __attribute__((destructor(180)));
+
+class test {
+   int a;
+public:
+test(int c) {a = c;}
+~test() {a = 0;}
+};
+
+__attribute__ ((init_priority (2000)))
+test t(1);
+
+// CHECK: warning: 'constructor' attribute argument not supported:
+// CHECK: int foo() __attribute__((constructor(180)));
+
+// CHECK: warning: 'destructor' attribute argument not supported:
+// check: int bar() __attribute__((destructor(180)));
+
+// CHECK: warning: 'init_priority' attribute argument not supported:
+// CHECK: __attribute__ ((init_priority (2000)))
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -6876,13 +6876,19 @@
 handlePassObjectSizeAttr(S, D, AL);
 break;
   case ParsedAttr::AT_Constructor:
-handleConstructorAttr(S, D, AL);
+if (S.Context.getTargetInfo().getTriple().isOSAIX())
+  S.Diag(AL.getLoc(), diag::warn_attribute_type_not_supported) << AL << "";
+else
+  handleConstructorAttr(S, D, AL);
 break;
   case ParsedAttr::AT_Deprecated:
 handleDeprecatedAttr(S, D, AL);
 break;
   case ParsedAttr::AT_Destructor:
-handleDestructorAttr(S, D, AL);
+if (S.Context.getTargetInfo().getTriple().isOSAIX())
+  S.Diag(AL.getLoc(), diag::warn_attribute_type_not_supported) << AL << "";
+else
+  

[PATCH] D74166: [AIX][Frontend] Static init implementation for AIX considering no priority

[Xiangling Liao via Phabricator via cfe-commits]

Xiangling_L updated this revision to Diff 248571.
Xiangling_L marked 2 inline comments as done.
Xiangling_L added a comment.

Fix the formatting issue;
Address the 1st round reviews;


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74166/new/

https://reviews.llvm.org/D74166

Files:
  clang/include/clang/AST/Mangle.h
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/CodeGen/CGCXXABI.h
  clang/lib/CodeGen/CGDeclCXX.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGen/aix-priority-attribute.cpp
  clang/test/CodeGen/static-init.cpp

Index: clang/test/CodeGen/static-init.cpp
===
--- clang/test/CodeGen/static-init.cpp
+++ clang/test/CodeGen/static-init.cpp
@@ -1,12 +1,55 @@
-// RUN: not %clang_cc1 -triple powerpc-ibm-aix-xcoff -S -emit-llvm -x c++ %s \
-// RUN: -o /dev/null 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -S -emit-llvm -x c++ < %s \
+// RUN: | FileCheck %s
 
-// RUN: not %clang_cc1 -triple powerpc64-ibm-aix-xcoff -S -emit-llvm -x c++ %s \
-// RUN: -o /dev/null 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff -S -emit-llvm -x c++ < %s \
+// RUN: | FileCheck %s
 
 struct test {
   test();
   ~test();
 } t;
 
-// CHECK: error in backend: Static initialization has not been implemented on XL ABI yet.
+// CHECK: @llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @__sinit8000_clang_b2e4830f1c9d2d063e5ea946868f3bfd, i8* null }]
+// CHECK: @llvm.global_dtors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @__sterm8000_clang_b2e4830f1c9d2d063e5ea946868f3bfd, i8* null }]
+// CHECK: define dso_local void @__cxx_global_var_init() #0 {
+// CHECK: entry:
+// CHECK:   call void @_ZN4testC1Ev(%struct.test* @t)
+// CHECK:   %0 = call i32 @atexit(void ()* @__dtor_t)
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: define dso_local void @__dtor_t() #0 {
+// CHECK: entry:
+// CHECK:   call void @_ZN4testD1Ev(%struct.test* @t)
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: declare i32 @atexit(void ()*) #3
+
+// CHECK: define dso_local void @__cxx_global_var_destruct_t() #0 {
+// CHECK: entry:
+// CHECK:   %0 = call i32 @unatexit(void ()* @__dtor_t)
+// CHECK:   %guard.hasSrterm = icmp eq i32 %0, 0
+// CHECK:   br i1 %guard.hasSrterm, label %destruct.check, label %destruct.end
+
+// CHECK: destruct.check:
+// CHECK:   call void @__dtor_t()
+// CHECK:   br label %destruct.end
+
+// CHECK: destruct.end:
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: declare i32 @unatexit(void ()*) #3
+
+// CHECK: define dso_local void @__sinit8000_clang_b2e4830f1c9d2d063e5ea946868f3bfd() #0 {
+// CHECK: entry:
+// CHECK:   call void @__cxx_global_var_init()
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: define dso_local void @__sterm8000_clang_b2e4830f1c9d2d063e5ea946868f3bfd() #0 {
+// CHECK: entry:
+// CHECK:   call void @__cxx_global_var_destruct_t()
+// CHECK:   ret void
+// CHECK: }
Index: clang/test/CodeGen/aix-priority-attribute.cpp
===
--- /dev/null
+++ clang/test/CodeGen/aix-priority-attribute.cpp
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -x c++ -emit-llvm < %s 2>&1 | \
+// RUN: FileCheck %s
+// RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff -x c++ -emit-llvm < %s 2>&1 | \
+// RUN: FileCheck %s
+
+int foo() __attribute__((constructor(180)));
+int bar() __attribute__((destructor(180)));
+
+class test {
+   int a;
+public:
+test(int c) {a = c;}
+~test() {a = 0;}
+};
+
+__attribute__ ((init_priority (2000)))
+test t(1);
+
+// CHECK: warning: 'constructor' attribute argument not supported:
+// CHECK: int foo() __attribute__((constructor(180)));
+
+// CHECK: warning: 'destructor' attribute argument not supported:
+// check: int bar() __attribute__((destructor(180)));
+
+// CHECK: warning: 'init_priority' attribute argument not supported:
+// CHECK: __attribute__ ((init_priority (2000)))
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -6821,7 +6821,10 @@
 handlePassObjectSizeAttr(S, D, AL);
 break;
   case ParsedAttr::AT_Constructor:
-handleConstructorAttr(S, D, AL);
+if (S.Context.getTargetInfo().getTriple().isOSAIX())
+  S.Diag(AL.getLoc(), diag::warn_attribute_type_not_supported) << AL << "";
+else
+  handleConstructorAttr(S, D, AL);
 break;
   case ParsedAttr::AT_CXX11NoReturn:
 handleSimpleAttribute(S, D, AL);
@@ -6830,7 +6833,10 @@
 handleDeprecatedAttr(S, D, AL);
 break;
   case ParsedAttr::AT_Destructor:
-handleDestructorAttr(S, D, AL);
+if (S.Context.getTargetInfo().getTriple().isOSAIX())
+  

[PATCH] D74166: [AIX][Frontend] Static init implementation for AIX considering no priority

[Xiangling Liao via Phabricator via cfe-commits]

Xiangling_L marked 4 inline comments as done.
Xiangling_L added inline comments.



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:283
   llvm::FunctionCallee atexit =
-  CGM.CreateRuntimeFunction(atexitTy, "atexit", llvm::AttributeList(),
-/*Local=*/true);
+  CGM.CreateRuntimeFunction(atexitTy, "atexit", llvm::AttributeList());
   if (llvm::Function *atexitFn = dyn_cast(atexit.getCallee()))

sfertile wrote:
> The default value for `Local` is false, was this change intentional? If so 
> why is it needed?
Thanks for pointing this out. I believe this is a bug. I was supposed to only 
let `Local` in `unregisterGlobalDtorWithUnAtExit` as default value `false`. 
Because it is only used in relation to Windows. 





Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:531
+
+  bool isCXXGlobalInitAndDtorFuncInternal() const override { return false; }
+

yusra.syeda wrote:
> Perhaps adding a check to see if the OS is AIX before setting linkage to 
> external will be useful here.
Since we are already under the context that `XLCXXABI` is an AIX C++ ABI, I 
kinda feel it's a duplication to add OS check.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74166/new/

https://reviews.llvm.org/D74166



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D74166: [AIX][Frontend] Static init implementation for AIX considering no priority

[Sean Fertile via Phabricator via cfe-commits]

sfertile added inline comments.



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:283
   llvm::FunctionCallee atexit =
-  CGM.CreateRuntimeFunction(atexitTy, "atexit", llvm::AttributeList(),
-/*Local=*/true);
+  CGM.CreateRuntimeFunction(atexitTy, "atexit", llvm::AttributeList());
   if (llvm::Function *atexitFn = dyn_cast(atexit.getCallee()))

The default value for `Local` is false, was this change intentional? If so why 
is it needed?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74166/new/

https://reviews.llvm.org/D74166



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D74166: [AIX][Frontend] Static init implementation for AIX considering no priority

[Sean Fertile via Phabricator via cfe-commits]

sfertile added a comment.

Please fix the formatting issues flagged by the pre-merge checks.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74166/new/

https://reviews.llvm.org/D74166



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D74166: [AIX][Frontend] Static init implementation for AIX considering no priority

[Yusra Syeda via Phabricator via cfe-commits]

yusra.syeda added a comment.

Overall the patch LGTM, aside from 1 change we may want to make.




Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:531
+
+  bool isCXXGlobalInitAndDtorFuncInternal() const override { return false; }
+

Perhaps adding a check to see if the OS is AIX before setting linkage to 
external will be useful here.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74166/new/

https://reviews.llvm.org/D74166



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D74166: [AIX][Frontend] Static init implementation for AIX considering no priority

[Xiangling Liao via Phabricator via cfe-commits]

Xiangling_L updated this revision to Diff 247360.
Xiangling_L added a comment.

Clean the formatting issues;


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74166/new/

https://reviews.llvm.org/D74166

Files:
  clang/include/clang/AST/Mangle.h
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/CodeGen/CGCXXABI.h
  clang/lib/CodeGen/CGDeclCXX.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGen/aix-priority-attribute.cpp
  clang/test/CodeGen/static-init.cpp

Index: clang/test/CodeGen/static-init.cpp
===
--- clang/test/CodeGen/static-init.cpp
+++ clang/test/CodeGen/static-init.cpp
@@ -1,12 +1,55 @@
-// RUN: not %clang_cc1 -triple powerpc-ibm-aix-xcoff -S -emit-llvm -x c++ %s \
-// RUN: -o /dev/null 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -S -emit-llvm -x c++ < %s \
+// RUN: | FileCheck %s
 
-// RUN: not %clang_cc1 -triple powerpc64-ibm-aix-xcoff -S -emit-llvm -x c++ %s \
-// RUN: -o /dev/null 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff -S -emit-llvm -x c++ < %s \
+// RUN: | FileCheck %s
 
 struct test {
   test();
   ~test();
 } t;
 
-// CHECK: error in backend: Static initialization has not been implemented on XL ABI yet.
+// CHECK: @llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @__sinit8000_clang_b2e4830f1c9d2d063e5ea946868f3bfd, i8* null }]
+// CHECK: @llvm.global_dtors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @__sterm8000_clang_b2e4830f1c9d2d063e5ea946868f3bfd, i8* null }]
+// CHECK: define dso_local void @__cxx_global_var_init() #0 {
+// CHECK: entry:
+// CHECK:   call void @_ZN4testC1Ev(%struct.test* @t)
+// CHECK:   %0 = call i32 @atexit(void ()* @__dtor_t)
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: define dso_local void @__dtor_t() #0 {
+// CHECK: entry:
+// CHECK:   call void @_ZN4testD1Ev(%struct.test* @t)
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: declare i32 @atexit(void ()*) #3
+
+// CHECK: define dso_local void @__cxx_global_var_destruct_t() #0 {
+// CHECK: entry:
+// CHECK:   %0 = call i32 @unatexit(void ()* @__dtor_t)
+// CHECK:   %guard.hasSrterm = icmp eq i32 %0, 0
+// CHECK:   br i1 %guard.hasSrterm, label %destruct.check, label %destruct.end
+
+// CHECK: destruct.check:
+// CHECK:   call void @__dtor_t()
+// CHECK:   br label %destruct.end
+
+// CHECK: destruct.end:
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: declare i32 @unatexit(void ()*) #3
+
+// CHECK: define dso_local void @__sinit8000_clang_b2e4830f1c9d2d063e5ea946868f3bfd() #0 {
+// CHECK: entry:
+// CHECK:   call void @__cxx_global_var_init()
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: define dso_local void @__sterm8000_clang_b2e4830f1c9d2d063e5ea946868f3bfd() #0 {
+// CHECK: entry:
+// CHECK:   call void @__cxx_global_var_destruct_t()
+// CHECK:   ret void
+// CHECK: }
Index: clang/test/CodeGen/aix-priority-attribute.cpp
===
--- /dev/null
+++ clang/test/CodeGen/aix-priority-attribute.cpp
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -x c++ -emit-llvm < %s 2>&1 | \
+// RUN: FileCheck %s
+// RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff -x c++ -emit-llvm < %s 2>&1 | \
+// RUN: FileCheck %s
+
+int foo() __attribute__((constructor(180)));
+int bar() __attribute__((destructor(180)));
+
+class test {
+   int a;
+public:
+test(int c) {a = c;}
+~test() {a = 0;}
+};
+
+__attribute__ ((init_priority (2000)))
+test t(1);
+
+// CHECK: warning: 'constructor' attribute argument not supported:
+// CHECK: int foo() __attribute__((constructor(180)));
+
+// CHECK: warning: 'destructor' attribute argument not supported:
+// check: int bar() __attribute__((destructor(180)));
+
+// CHECK: warning: 'init_priority' attribute argument not supported:
+// CHECK: __attribute__ ((init_priority (2000)))
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -6819,7 +6819,11 @@
 handlePassObjectSizeAttr(S, D, AL);
 break;
   case ParsedAttr::AT_Constructor:
-handleConstructorAttr(S, D, AL);
+if (S.Context.getTargetInfo().getTriple().isOSAIX())
+  S.Diag(AL.getLoc(), diag::warn_attribute_type_not_supported)
+  << AL << "";
+else
+  handleConstructorAttr(S, D, AL);
 break;
   case ParsedAttr::AT_CXX11NoReturn:
 handleSimpleAttribute(S, D, AL);
@@ -6828,7 +6832,11 @@
 handleDeprecatedAttr(S, D, AL);
 break;
   case ParsedAttr::AT_Destructor:
-handleDestructorAttr(S, D, AL);
+if (S.Context.getTargetInfo().getTriple().isOSAIX())
+  S.Diag(AL.getLoc(), 

[PATCH] D74166: [AIX][Frontend] Static init implementation for AIX considering no priority

[Xiangling Liao via Phabricator via cfe-commits]

Xiangling_L updated this revision to Diff 247355.
Xiangling_L added a comment.

Rebase on the latest master branch.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74166/new/

https://reviews.llvm.org/D74166

Files:
  clang/include/clang/AST/Mangle.h
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/CodeGen/CGCXXABI.h
  clang/lib/CodeGen/CGDeclCXX.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGen/aix-priority-attribute.cpp
  clang/test/CodeGen/static-init.cpp

Index: clang/test/CodeGen/static-init.cpp
===
--- clang/test/CodeGen/static-init.cpp
+++ clang/test/CodeGen/static-init.cpp
@@ -1,12 +1,55 @@
-// RUN: not %clang_cc1 -triple powerpc-ibm-aix-xcoff -S -emit-llvm -x c++ %s \
-// RUN: -o /dev/null 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -S -emit-llvm -x c++ < %s \
+// RUN: | FileCheck %s
 
-// RUN: not %clang_cc1 -triple powerpc64-ibm-aix-xcoff -S -emit-llvm -x c++ %s \
-// RUN: -o /dev/null 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff -S -emit-llvm -x c++ < %s \
+// RUN: | FileCheck %s
 
 struct test {
   test();
   ~test();
 } t;
 
-// CHECK: error in backend: Static initialization has not been implemented on XL ABI yet.
+// CHECK: @llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @__sinit8000_clang_b2e4830f1c9d2d063e5ea946868f3bfd, i8* null }]
+// CHECK: @llvm.global_dtors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @__sterm8000_clang_b2e4830f1c9d2d063e5ea946868f3bfd, i8* null }]
+// CHECK: define dso_local void @__cxx_global_var_init() #0 {
+// CHECK: entry:
+// CHECK:   call void @_ZN4testC1Ev(%struct.test* @t)
+// CHECK:   %0 = call i32 @atexit(void ()* @__dtor_t)
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: define dso_local void @__dtor_t() #0 {
+// CHECK: entry:
+// CHECK:   call void @_ZN4testD1Ev(%struct.test* @t)
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: declare i32 @atexit(void ()*) #3
+
+// CHECK: define dso_local void @__cxx_global_var_destruct_t() #0 {
+// CHECK: entry:
+// CHECK:   %0 = call i32 @unatexit(void ()* @__dtor_t)
+// CHECK:   %guard.hasSrterm = icmp eq i32 %0, 0
+// CHECK:   br i1 %guard.hasSrterm, label %destruct.check, label %destruct.end
+
+// CHECK: destruct.check:
+// CHECK:   call void @__dtor_t()
+// CHECK:   br label %destruct.end
+
+// CHECK: destruct.end:
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: declare i32 @unatexit(void ()*) #3
+
+// CHECK: define dso_local void @__sinit8000_clang_b2e4830f1c9d2d063e5ea946868f3bfd() #0 {
+// CHECK: entry:
+// CHECK:   call void @__cxx_global_var_init()
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: define dso_local void @__sterm8000_clang_b2e4830f1c9d2d063e5ea946868f3bfd() #0 {
+// CHECK: entry:
+// CHECK:   call void @__cxx_global_var_destruct_t()
+// CHECK:   ret void
+// CHECK: }
Index: clang/test/CodeGen/aix-priority-attribute.cpp
===
--- /dev/null
+++ clang/test/CodeGen/aix-priority-attribute.cpp
@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -x c++ -emit-llvm < %s 2>&1 | \
+// RUN: FileCheck %s
+// RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff -x c++ -emit-llvm < %s 2>&1 | \
+// RUN: FileCheck %s
+
+int foo() __attribute__((constructor(180)));
+int bar() __attribute__((destructor(180)));
+
+class test {
+   int a;
+public:
+test(int c) {a = c;}
+~test() {a = 0;}
+};
+
+__attribute__ ((init_priority (2000)))
+test t(1);
+
+// CHECK: warning: 'constructor' attribute argument not supported:
+// CHECK: int foo() __attribute__((constructor(180)));
+ 
+// CHECK: warning: 'destructor' attribute argument not supported:
+// check: int bar() __attribute__((destructor(180)));
+
+// CHECK: warning: 'init_priority' attribute argument not supported:
+// CHECK: __attribute__ ((init_priority (2000)))
+
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -6819,7 +6819,11 @@
 handlePassObjectSizeAttr(S, D, AL);
 break;
   case ParsedAttr::AT_Constructor:
-handleConstructorAttr(S, D, AL);
+if (S.Context.getTargetInfo().getTriple().isOSAIX())
+  S.Diag(AL.getLoc(), diag::warn_attribute_type_not_supported)
+  << AL << "";
+else
+  handleConstructorAttr(S, D, AL);
 break;
   case ParsedAttr::AT_CXX11NoReturn:
 handleSimpleAttribute(S, D, AL);
@@ -6828,7 +6832,11 @@
 handleDeprecatedAttr(S, D, AL);
 break;
   case ParsedAttr::AT_Destructor:
-handleDestructorAttr(S, D, AL);
+if (S.Context.getTargetInfo().getTriple().isOSAIX())
+  

[PATCH] D74166: [AIX][Frontend] Static init implementation for AIX considering no priority

[Xiangling Liao via Phabricator via cfe-commits]

Xiangling_L added a comment.

ping.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74166/new/

https://reviews.llvm.org/D74166



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D74166: [AIX][Frontend] Static init implementation for AIX considering no priority

[Xiangling Liao via Phabricator via cfe-commits]

Xiangling_L updated this revision to Diff 245929.
Xiangling_L edited the summary of this revision.
Xiangling_L added a comment.

Update the testcase;


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74166/new/

https://reviews.llvm.org/D74166

Files:
  clang/include/clang/AST/Mangle.h
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/CodeGen/CGCXXABI.h
  clang/lib/CodeGen/CGDeclCXX.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGen/aix-priority-attribute.cpp
  clang/test/CodeGen/static-init.cpp

Index: clang/test/CodeGen/static-init.cpp
===
--- clang/test/CodeGen/static-init.cpp
+++ clang/test/CodeGen/static-init.cpp
@@ -1,12 +1,55 @@
-// RUN: not %clang_cc1 -triple powerpc-ibm-aix-xcoff -S -emit-llvm -x c++ %s \
-// RUN: 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -S -emit-llvm -x c++ < %s \
+// RUN: | FileCheck %s
 
-// RUN: not %clang_cc1 -triple powerpc64-ibm-aix-xcoff -S -emit-llvm -x c++ %s \
-// RUN: 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff -S -emit-llvm -x c++ < %s \
+// RUN: | FileCheck %s
 
 struct test {
   test();
   ~test();
 } t;
 
-// CHECK: error in backend: Static initialization has not been implemented on XL ABI yet.
+// CHECK: @llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @__sinit8000_clang_b2e4830f1c9d2d063e5ea946868f3bfd, i8* null }]
+// CHECK: @llvm.global_dtors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @__sterm8000_clang_b2e4830f1c9d2d063e5ea946868f3bfd, i8* null }]
+// CHECK: define dso_local void @__cxx_global_var_init() #0 {
+// CHECK: entry:
+// CHECK:   call void @_ZN4testC1Ev(%struct.test* @t)
+// CHECK:   %0 = call i32 @atexit(void ()* @__dtor_t)
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: define dso_local void @__dtor_t() #0 {
+// CHECK: entry:
+// CHECK:   call void @_ZN4testD1Ev(%struct.test* @t)
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: declare i32 @atexit(void ()*) #3
+
+// CHECK: define dso_local void @__cxx_global_var_destruct_t() #0 {
+// CHECK: entry:
+// CHECK:   %0 = call i32 @unatexit(void ()* @__dtor_t)
+// CHECK:   %guard.hasSrterm = icmp eq i32 %0, 0
+// CHECK:   br i1 %guard.hasSrterm, label %destruct.check, label %destruct.end
+
+// CHECK: destruct.check:
+// CHECK:   call void @__dtor_t()
+// CHECK:   br label %destruct.end
+
+// CHECK: destruct.end:
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: declare i32 @unatexit(void ()*) #3
+
+// CHECK: define dso_local void @__sinit8000_clang_b2e4830f1c9d2d063e5ea946868f3bfd() #0 {
+// CHECK: entry:
+// CHECK:   call void @__cxx_global_var_init()
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: define dso_local void @__sterm8000_clang_b2e4830f1c9d2d063e5ea946868f3bfd() #0 {
+// CHECK: entry:
+// CHECK:   call void @__cxx_global_var_destruct_t()
+// CHECK:   ret void
+// CHECK: }
Index: clang/test/CodeGen/aix-priority-attribute.cpp
===
--- /dev/null
+++ clang/test/CodeGen/aix-priority-attribute.cpp
@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -x c++ -emit-llvm < %s 2>&1 | \
+// RUN: FileCheck %s
+// RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff -x c++ -emit-llvm < %s 2>&1 | \
+// RUN: FileCheck %s
+
+int foo() __attribute__((constructor(180)));
+int bar() __attribute__((destructor(180)));
+
+class test {
+   int a;
+public:
+test(int c) {a = c;}
+~test() {a = 0;}
+};
+
+__attribute__ ((init_priority (2000)))
+test t(1);
+
+// CHECK: warning: 'constructor' attribute argument not supported:
+// CHECK: int foo() __attribute__((constructor(180)));
+ 
+// CHECK: warning: 'destructor' attribute argument not supported:
+// check: int bar() __attribute__((destructor(180)));
+
+// CHECK: warning: 'init_priority' attribute argument not supported:
+// CHECK: __attribute__ ((init_priority (2000)))
+
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -6819,7 +6819,11 @@
 handlePassObjectSizeAttr(S, D, AL);
 break;
   case ParsedAttr::AT_Constructor:
-handleConstructorAttr(S, D, AL);
+if (S.Context.getTargetInfo().getTriple().isOSAIX())
+  S.Diag(AL.getLoc(), diag::warn_attribute_type_not_supported)
+  << AL << "";
+else
+  handleConstructorAttr(S, D, AL);
 break;
   case ParsedAttr::AT_CXX11NoReturn:
 handleSimpleAttribute(S, D, AL);
@@ -6828,7 +6832,11 @@
 handleDeprecatedAttr(S, D, AL);
 break;
   case ParsedAttr::AT_Destructor:
-handleDestructorAttr(S, D, AL);
+if (S.Context.getTargetInfo().getTriple().isOSAIX())
+  S.Diag(AL.getLoc(), 

[PATCH] D74166: [AIX][Frontend] Static init implementation for AIX considering no priority

[Xiangling Liao via Phabricator via cfe-commits]

Xiangling_L updated this revision to Diff 245879.
Xiangling_L added a comment.

Rebase to incorparate `XL` C++ ABI name && comdat changes;


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74166/new/

https://reviews.llvm.org/D74166

Files:
  clang/include/clang/AST/Mangle.h
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/CodeGen/CGCXXABI.h
  clang/lib/CodeGen/CGDeclCXX.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGen/aix-priority-attribute.cpp
  clang/test/CodeGen/static-init.cpp

Index: clang/test/CodeGen/static-init.cpp
===
--- clang/test/CodeGen/static-init.cpp
+++ clang/test/CodeGen/static-init.cpp
@@ -1,12 +1,55 @@
-// RUN: not %clang_cc1 -triple powerpc-ibm-aix-xcoff -S -emit-llvm -x c++ %s \
-// RUN: 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -S -emit-llvm -x c++ < %s \
+// RUN: | FileCheck %s
 
-// RUN: not %clang_cc1 -triple powerpc64-ibm-aix-xcoff -S -emit-llvm -x c++ %s \
-// RUN: 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff -S -emit-llvm -x c++ < %s \
+// RUN: | FileCheck %s
 
 struct test {
   test();
   ~test();
 } t;
 
-// CHECK: error in backend: Static initialization has not been implemented on XL ABI yet.
+// CHECK: @llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @__sinit8000_clang_b2e4830f1c9d2d063e5ea946868f3bfd, i8* null }]
+// CHECK: @llvm.global_dtors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @__sterm8000_clang_b2e4830f1c9d2d063e5ea946868f3bfd, i8* null }]
+// CHECK: define dso_local void @__cxx_global_var_init() #0 {
+// CHECK: entry:
+// CHECK:   call void @_ZN4testC1Ei(%class.test* @t, i32 1)
+// CHECK:   %0 = call i32 @atexit(void ()* @__dtor_t)
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: define dso_local void @__dtor_t() #0 {
+// CHECK: entry:
+// CHECK:   call void @_ZN4testD1Ev(%class.test* @t)
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: declare i32 @atexit(void ()*) #2
+
+// CHECK: define dso_local void @__cxx_global_var_destruct_t() #0 {
+// CHECK: entry:
+// CHECK:   %0 = call i32 @unatexit(void ()* @__dtor_t)
+// CHECK:   %guard.hasSrterm = icmp eq i32 %0, 0
+// CHECK:   br i1 %guard.hasSrterm, label %destruct.check, label %destruct.end
+
+// CHECK: destruct.check:
+// CHECK:   call void @__dtor_t()
+// CHECK:   br label %destruct.end
+
+// CHECK: destruct.end:
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: declare i32 @unatexit(void ()*) #2
+
+// CHECK: define dso_local void @__sinit8000_clang_b2e4830f1c9d2d063e5ea946868f3bfd() #0 {
+// CHECK: entry:
+// CHECK:   call void @__cxx_global_var_init()
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: define dso_local void @__sterm8000_clang_b2e4830f1c9d2d063e5ea946868f3bfd() #0 {
+// CHECK: entry:
+// CHECK:   call void @__cxx_global_var_destruct_t()
+// CHECK:   ret void
+// CHECK: }
Index: clang/test/CodeGen/aix-priority-attribute.cpp
===
--- /dev/null
+++ clang/test/CodeGen/aix-priority-attribute.cpp
@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -x c++ -emit-llvm < %s 2>&1 | \
+// RUN: FileCheck %s
+// RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff -x c++ -emit-llvm < %s 2>&1 | \
+// RUN: FileCheck %s
+
+int foo() __attribute__((constructor(180)));
+int bar() __attribute__((destructor(180)));
+
+class test {
+   int a;
+public:
+test(int c) {a = c;}
+~test() {a = 0;}
+};
+
+__attribute__ ((init_priority (2000)))
+test t(1);
+
+// CHECK: warning: 'constructor' attribute argument not supported:
+// CHECK: int foo() __attribute__((constructor(180)));
+ 
+// CHECK: warning: 'destructor' attribute argument not supported:
+// check: int bar() __attribute__((destructor(180)));
+
+// CHECK: warning: 'init_priority' attribute argument not supported:
+// CHECK: __attribute__ ((init_priority (2000)))
+
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -6819,7 +6819,11 @@
 handlePassObjectSizeAttr(S, D, AL);
 break;
   case ParsedAttr::AT_Constructor:
-handleConstructorAttr(S, D, AL);
+if (S.Context.getTargetInfo().getTriple().isOSAIX())
+  S.Diag(AL.getLoc(), diag::warn_attribute_type_not_supported)
+  << AL << "";
+else
+  handleConstructorAttr(S, D, AL);
 break;
   case ParsedAttr::AT_CXX11NoReturn:
 handleSimpleAttribute(S, D, AL);
@@ -6828,7 +6832,11 @@
 handleDeprecatedAttr(S, D, AL);
 break;
   case ParsedAttr::AT_Destructor:
-handleDestructorAttr(S, D, AL);
+if (S.Context.getTargetInfo().getTriple().isOSAIX())
+  

[PATCH] D74166: [AIX][Frontend] Static init implementation for AIX considering no priority

[Xiangling Liao via Phabricator via cfe-commits]

Xiangling_L created this revision.
Xiangling_L added reviewers: hubert.reinterpretcast, cebowleratibm, 
yusra.syeda, sfertile, jasonliu, xingxue, hfinkel.
Xiangling_L added a project: LLVM.
Herald added subscribers: llvm-commits, cfe-commits, dexonsmith.
Herald added a project: clang.
Xiangling_L added a parent revision: D74015: [AIX][Frontend] C++ ABI 
customizations for AIX boilerplate.
Xiangling_L planned changes to this revision.

Importantly, this patch provides context and shows where things are going of 
static init of AIX in LLVM. And hope it would help reviewers with its parent 
patch:  D74015 .

- Provides no piroirity supoort && disables/ignores three priority related 
attributes: init_priority, ctor attr, dtor attr;
  - '-qunique' in XLC compiler equivalent behavior of emitting sinit and sterm 
functions name using `getUniqueModuleId()` function in LLVM;
  - Add a simple testcase to emit IR sample with sinit8000, srterm, and 
sterm8000


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D74166

Files:
  clang/include/clang/AST/Mangle.h
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/CodeGen/CGCXXABI.h
  clang/lib/CodeGen/CGDeclCXX.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGen/aix-priority-attribute.cpp
  clang/test/CodeGen/static-init.cpp
  llvm/include/llvm/ADT/Triple.h

Index: llvm/include/llvm/ADT/Triple.h
===
--- llvm/include/llvm/ADT/Triple.h
+++ llvm/include/llvm/ADT/Triple.h
@@ -743,7 +743,7 @@
 
   /// Tests whether the target supports comdat
   bool supportsCOMDAT() const {
-return !isOSBinFormatMachO();
+return !isOSBinFormatMachO() && !isOSBinFormatXCOFF();
   }
 
   /// Tests whether the target uses emulated TLS as default.
Index: clang/test/CodeGen/static-init.cpp
===
--- clang/test/CodeGen/static-init.cpp
+++ clang/test/CodeGen/static-init.cpp
@@ -1,8 +1,9 @@
-// RUN: not %clang_cc1 -triple powerpc-ibm-aix-xcoff -S -emit-llvm -x c++ %s \
-// RUN: 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -S -emit-llvm -x c++ < %s \
+// RUN: | FileCheck %s
+
+// RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff -S -emit-llvm -x c++ < %s \
+// RUN: | FileCheck %s
 
-// RUN: not %clang_cc1 -triple powerpc64-ibm-aix-xcoff -S -emit-llvm -x c++ %s \
-// RUN: 2>&1 | FileCheck %s
 class test {
int a;
 public:
@@ -12,5 +13,47 @@
 
 test t(1);
 
+// CHECK: @llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @__sinit8000_clang_b2e4830f1c9d2d063e5ea946868f3bfd, i8* null }]
+// CHECK: @llvm.global_dtors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @__sterm8000_clang_b2e4830f1c9d2d063e5ea946868f3bfd, i8* null }]
+// CHECK: define dso_local void @__cxx_global_var_init() #0 {
+// CHECK: entry:
+// CHECK:   call void @_ZN4testC1Ei(%class.test* @t, i32 1)
+// CHECK:   %0 = call i32 @atexit(void ()* @__dtor_t)
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: define dso_local void @__dtor_t() #0 {
+// CHECK: entry:
+// CHECK:   call void @_ZN4testD1Ev(%class.test* @t)
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: declare i32 @atexit(void ()*) #2
+
+// CHECK: define dso_local void @__cxx_global_var_destruct_t() #0 {
+// CHECK: entry:
+// CHECK:   %0 = call i32 @unatexit(void ()* @__dtor_t)
+// CHECK:   %guard.hasSrterm = icmp eq i32 %0, 0
+// CHECK:   br i1 %guard.hasSrterm, label %destruct.check, label %destruct.end
+
+// CHECK: destruct.check:
+// CHECK:   call void @__dtor_t()
+// CHECK:   br label %destruct.end
+
+// CHECK: destruct.end:
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: declare i32 @unatexit(void ()*) #2
+
+// CHECK: define dso_local void @__sinit8000_clang_b2e4830f1c9d2d063e5ea946868f3bfd() #0 {
+// CHECK: entry:
+// CHECK:   call void @__cxx_global_var_init()
+// CHECK:   ret void
+// CHECK: }
 
-; CHECK: error in backend: Static initialization has not been fully implemented on XL_Clang ABI yet.
+// CHECK: define dso_local void @__sterm8000_clang_b2e4830f1c9d2d063e5ea946868f3bfd() #0 {
+// CHECK: entry:
+// CHECK:   call void @__cxx_global_var_destruct_t()
+// CHECK:   ret void
+// CHECK: }
Index: clang/test/CodeGen/aix-priority-attribute.cpp
===
--- /dev/null
+++ clang/test/CodeGen/aix-priority-attribute.cpp
@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -x c++ -emit-llvm < %s 2>&1 | \
+// RUN: FileCheck %s
+// RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff -x c++ -emit-llvm < %s 2>&1 | \
+// RUN: FileCheck %s
+
+int foo() __attribute__((constructor(180)));
+int bar() __attribute__((destructor(180)));
+
+class test {
+   int a;
+public:
+test(int