[PATCH] D95822: [FE][AIX] Use i8 as guard variable type in both load and store operation

[Xiangling Liao via Phabricator via cfe-commits]

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



Comment at: 
clang/test/CodeGenCXX/aix-static-init-temp-spec-and-inline-var.cpp:193
 // CHECK:   %1 = call i32 @atexit(void ()* @__dtor__ZN5test12t1IiEE)
-// CHECK:   store i64 1, i64* @_ZGVN5test12t1IiEE
 // CHECK:   br label %init.end

hubert.reinterpretcast wrote:
> Okay, so the store here is wrong, but it's got nothing to do with AIX.
> 
> The ABI doc (http://itanium-cxx-abi.github.io/cxx-abi/abi.html#once-ctor) 
> says:
> > the first byte (i.e., the byte with lowest address)
> 
> So the bug here seems to be that the Itanium ABI implementation in Clang is 
> incorrect for big-endian systems.
That makes sense to me. I will update the patch accordingly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95822

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

[PATCH] D95822: [FE][AIX] Use i8 as guard variable type in both load and store operation

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/test/CodeGenCXX/aix-static-init.cpp:143
 // CHECK: init.check:
-// CHECK:   %1 = call i32 @__cxa_guard_acquire(i64* 
@_ZGVZN5test41fEvE11staticLocal)
+// CHECK:   %1 = call i32 @__cxa_guard_acquire(i8* 
@_ZGVZN5test41fEvE11staticLocal)
 // CHECK:   %tobool = icmp ne i32 %1, 0

What actually does the definition of `_ZGVZN5test41fEvE11staticLocal` look 
like? The test seems to be missing something pertinent for the change being 
proposed here. I will note that the symbol is 8-bytes (and aligned accordingly) 
from XL.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95822

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

[PATCH] D95822: [FE][AIX] Use i8 as guard variable type in both load and store operation

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: 
clang/test/CodeGenCXX/aix-static-init-temp-spec-and-inline-var.cpp:193
 // CHECK:   %1 = call i32 @atexit(void ()* @__dtor__ZN5test12t1IiEE)
-// CHECK:   store i64 1, i64* @_ZGVN5test12t1IiEE
 // CHECK:   br label %init.end

Okay, so the store here is wrong, but it's got nothing to do with AIX.

The ABI doc (http://itanium-cxx-abi.github.io/cxx-abi/abi.html#once-ctor) says:
> the first byte (i.e., the byte with lowest address)

So the bug here seems to be that the Itanium ABI implementation in Clang is 
incorrect for big-endian systems.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95822

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

[PATCH] D95822: [FE][AIX] Use i8 as guard variable type in both load and store operation

[Xiangling Liao via Phabricator via cfe-commits]

Xiangling_L created this revision.
Xiangling_L added reviewers: hubert.reinterpretcast, jasonliu, 
abhina.sreeskantharajan.
Herald added a subscriber: jfb.
Xiangling_L requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

For static init, use i8 as guard variable type in both load and store 
operation. And this matches XL behavior on AIX.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D95822

Files:
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/test/CodeGenCXX/aix-static-init-temp-spec-and-inline-var.cpp
  clang/test/CodeGenCXX/aix-static-init.cpp

Index: clang/test/CodeGenCXX/aix-static-init.cpp
===
--- clang/test/CodeGenCXX/aix-static-init.cpp
+++ clang/test/CodeGenCXX/aix-static-init.cpp
@@ -135,19 +135,19 @@
 
 // CHECK: define void @_ZN5test41fEv() [[ATTR:#[0-9]+]] {
 // CHECK: entry:
-// CHECK:   %0 = load atomic i8, i8* bitcast (i64* @_ZGVZN5test41fEvE11staticLocal to i8*) acquire
+// CHECK:   %0 = load atomic i8, i8* @_ZGVZN5test41fEvE11staticLocal acquire, align 1
 // CHECK:   %guard.uninitialized = icmp eq i8 %0, 0
 // CHECK:   br i1 %guard.uninitialized, label %init.check, label %init.end
 
 // CHECK: init.check:
-// CHECK:   %1 = call i32 @__cxa_guard_acquire(i64* @_ZGVZN5test41fEvE11staticLocal)
+// CHECK:   %1 = call i32 @__cxa_guard_acquire(i8* @_ZGVZN5test41fEvE11staticLocal)
 // CHECK:   %tobool = icmp ne i32 %1, 0
 // CHECK:   br i1 %tobool, label %init, label %init.end
 
 // CHECK: init:
 // CHECK:   call void @_ZN5test45Test4C1Ev(%"struct.test4::Test4"* {{[^,]*}} @_ZZN5test41fEvE11staticLocal)
 // CHECK:   %2 = call i32 @atexit(void ()* @__dtor__ZZN5test41fEvE11staticLocal)
-// CHECK:   call void @__cxa_guard_release(i64* @_ZGVZN5test41fEvE11staticLocal)
+// CHECK:   call void @__cxa_guard_release(i8* @_ZGVZN5test41fEvE11staticLocal)
 // CHECK:   br label %init.end
 
 // CHECK: init.end:
Index: clang/test/CodeGenCXX/aix-static-init-temp-spec-and-inline-var.cpp
===
--- clang/test/CodeGenCXX/aix-static-init-temp-spec-and-inline-var.cpp
+++ clang/test/CodeGenCXX/aix-static-init-temp-spec-and-inline-var.cpp
@@ -77,12 +77,12 @@
 
 // CHECK: define internal void @__cxx_global_var_init.1() [[ATTR:#[0-9]+]] {
 // CHECK: entry:
-// CHECK:   %0 = load atomic i8, i8* bitcast (i64* @_ZGVN5test12t2E to i8*) acquire
+// CHECK:   %0 = load atomic i8, i8* @_ZGVN5test12t2E acquire, align 1
 // CHECK:   %guard.uninitialized = icmp eq i8 %0, 0
 // CHECK:   br i1 %guard.uninitialized, label %init.check, label %init.end
 
 // CHECK: init.check:
-// CHECK:   %1 = call i32 @__cxa_guard_acquire(i64* @_ZGVN5test12t2E)
+// CHECK:   %1 = call i32 @__cxa_guard_acquire(i8* @_ZGVN5test12t2E)
 // CHECK:   %tobool = icmp ne i32 %1, 0
 // CHECK:   br i1 %tobool, label %init, label %init.end
 
@@ -90,7 +90,7 @@
 // CHECK32: call void @_ZN5test15Test1C1Ei(%"struct.test1::Test1"* {{[^,]*}} @_ZN5test12t2E, i32 2)
 // CHECK64: call void @_ZN5test15Test1C1Ei(%"struct.test1::Test1"* {{[^,]*}} @_ZN5test12t2E, i32 signext 2)
 // CHECK:   %2 = call i32 @atexit(void ()* @__dtor__ZN5test12t2E)
-// CHECK:   call void @__cxa_guard_release(i64* @_ZGVN5test12t2E)
+// CHECK:   call void @__cxa_guard_release(i8* @_ZGVN5test12t2E)
 // CHECK:   br label %init.end
 
 // CHECK: init.end:
@@ -119,14 +119,14 @@
 
 // CHECK: define internal void @__cxx_global_var_init.2() [[ATTR:#[0-9]+]] {
 // CHECK: entry:
-// CHECK:   %0 = load i8, i8* bitcast (i64* @_ZGVN5test21AIvE8instanceE to i8*)
+// CHECK:   %0 = load i8, i8* @_ZGVN5test21AIvE8instanceE, align 1
 // CHECK:   %guard.uninitialized = icmp eq i8 %0, 0
 // CHECK:   br i1 %guard.uninitialized, label %init.check, label %init.end
 
 // CHECK: init.check:
 // CHECK:   call void @_ZN5test21AIvEC1Ev(%"struct.test2::A"* {{[^,]*}} @_ZN5test21AIvE8instanceE)
 // CHECK:   %1 = call i32 @atexit(void ()* @__dtor__ZN5test21AIvE8instanceE)
-// CHECK:   store i64 1, i64* @_ZGVN5test21AIvE8instanceE
+// CHECK:   store i8 1, i8* @_ZGVN5test21AIvE8instanceE, align 1
 // CHECK:   br label %init.end
 
 // CHECK: init.end:
@@ -182,7 +182,7 @@
 
 // CHECK: define internal void @__cxx_global_var_init.4() [[ATTR:#[0-9]+]] {
 // CHECK: entry:
-// CHECK:   %0 = load i8, i8* bitcast (i64* @_ZGVN5test12t1IiEE to i8*)
+// CHECK:   %0 = load i8, i8* @_ZGVN5test12t1IiEE, align 1
 // CHECK:   %guard.uninitialized = icmp eq i8 %0, 0
 // CHECK:   br i1 %guard.uninitialized, label %init.check, label %init.end
 
@@ -190,7 +190,7 @@
 // CHECK32: call void @_ZN5test15Test1C1Ei(%"struct.test1::Test1"* {{[^,]*}} @_ZN5test12t1IiEE, i32 2)
 // CHECK64: call void @_ZN5test15Test1C1Ei(%"struct.test1::Test1"* {{[^,]*}} @_ZN5test12t1IiEE, i32 signext 2)
 // CHECK:   %1 = call i32 @atexit(void ()* @__dtor__ZN5test12t1IiEE)
-// CHECK:   store i64 1, i64* @_ZGVN5test12t1IiEE
+// CHECK:   store i8 1, i8* @_ZGVN5test12t1IiEE, align 1
 // CHECK