[PATCH] D90892: [AIX][FE] Support constructor/destructor attribute

[Sean Fertile via Phabricator via cfe-commits]

sfertile added inline comments.


================
Comment at: clang/test/CodeGen/aix-constructor-attribute.cpp:8
 
-int foo() __attribute__((constructor(180)));
+// CHECK: @llvm.global_ctors = appending global [3 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* bitcast (i32 ()* @foo3 to void ()*), i8* null }, { i32, void ()*, i8* } { i32 180, void ()* bitcast (i32 ()* @foo2 to void ()*), i8* null }, { i32, void ()*, i8* } { i32 180, void ()* bitcast (i32 ()* @foo to void ()*), i8* null }]
 
----------------
Did you mean for this test to be a C or C++ test? If it is meant to be C++ it needs to mangle the function names, but considering the director it is in and the fact we have the same test in CodeGenCXX directory I expect this was meant to be C in which case it needs a `.c` extension and lose the `-x c++` in the run steps.


================
Comment at: clang/test/CodeGen/aix-destructor-attribute.cpp:1
-// RUN: not %clang_cc1 -triple powerpc-ibm-aix-xcoff -x c++ -emit-llvm < %s \
-// RUN:     2>&1 | \
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -emit-llvm \
+// RUN:     -fno-use-cxa-atexit < %s | \
----------------
Similarly if this is meant to be a C test, give the file a .c extension.


================
Comment at: clang/test/CodeGen/aix-sinit-register-global-dtors-with-atexit.cpp:1
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -emit-llvm \
+// RUN:     -fno-use-cxa-atexit -fregister-global-dtors-with-atexit < %s | \
----------------
I really think this test should be included in aix-destructor-attribute.c to highlight the codeine differences with and without the option.


================
Comment at: clang/test/CodeGenCXX/aix-destructor-attribute.cpp:1
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -x c++ -emit-llvm \
+// RUN:     -fno-use-cxa-atexit < %s | \
----------------
Can I ask why this is added as a new file? Its the same test as `aix-sinit-register-global-dtors-with-atexit.cpp` but without using `-fregister-global-dtors-with-atexit`. I suggest combining the 2.


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

https://reviews.llvm.org/D90892