[FE] Manipulate the first byte of guard variable type in both load and store operation

[Xiangling_L]

Summary

As Itanium ABI[http://itanium-cxx-abi.github.io/cxx-abi/abi.html#once-ctor] points out:

The size of the guard variable is 64 bits. The first byte (i.e. the byte at the address of the full variable) shall contain the value 0 prior to initialization of the associated variable, and 1 after initialization is complete.

We should manipulate the first byte of guard variable in both load and store operation in general cases.

[hubert.reinterpretcast]

clang/test/CodeGenCXX/aix-static-init-temp-spec-and-inline-var.cpp

193

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.

[hubert.reinterpretcast]

clang/test/CodeGenCXX/aix-static-init.cpp

143

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.

[Xiangling_L]

clang/test/CodeGenCXX/aix-static-init-temp-spec-and-inline-var.cpp

193

That makes sense to me. I will update the patch accordingly.

[Xiangling_L]

Align load and store operation related to guard variable by both using the first byte of the variable.

[abhina.sreeskantharajan]

clang/test/CodeGenCXX/global-init.cpp

84 ↗

(On Diff #320864)

nit: the lines above omit the end of the line. It's probably better to be consistent

[hubert.reinterpretcast]

guard.patch

1 ↗

(On Diff #320864)

Looks like this file got added by accident.

[Xiangling_L]

clang/test/CodeGenCXX/global-init.cpp

84 ↗

(On Diff #320864)

It didn't omit the end of the line. It is because before my patch, there was no casting.

guard.patch

1 ↗

(On Diff #320864)

Thanks!

[hubert.reinterpretcast]

clang/test/CodeGenCXX/global-init.cpp

84 ↗

(On Diff #320864)

Hi @Xiangling_L, I think Abhina meant the lines above in this file. The load has a bitcast too (and stops after the guard variable name).

[Xiangling_L]

Remove redundant file;

[Xiangling_L]

Addressed the comments related to test formatting;

clang/test/CodeGenCXX/global-init.cpp

84 ↗

(On Diff #320864)

I see. Sorry for the misunderstanding. @abhina.sreeskantharajan

[abhina.sreeskantharajan]

LGTM, thanks!

[hubert.reinterpretcast]

LGTM. I think there would be more concern if the load/store actions were previously symmetric, but this patch is actually needed for the symmetry. Nevertheless, please wait a bit before committing this (since this is a change for other platforms as well).

clang/lib/CodeGen/ItaniumCXXABI.cpp

void ItaniumCXXABI::EmitGuardedInit(CodeGenFunction &CGF,
   // We only need to use thread-safe statics for local non-TLS variables and
   // inline variables; other global initialization is always single-threaded
   // or (through lazy dynamic loading in multiple threads) unsequenced.
   bool threadsafe = getContext().getLangOpts().ThreadsafeStatics &&
                     (D.isLocalVarDecl() || NonTemplateInline) &&
                     !D.getTLSKind();
 
   // If we have a global variable with internal linkage and thread-safe statics
-  // are disabled, we can just let the guard variable be of type i8.
-  bool useInt8GuardVariable = !threadsafe && var->hasInternalLinkage();
+  // are disabled or if we are using sinit and sterm functions, we can just let
+  // the guard variable be of type i8.
+  bool useInt8GuardVariable =
+      (!threadsafe && var->hasInternalLinkage()) || useSinitAndSterm();
 
   llvm::IntegerType *guardTy;
   CharUnits guardAlignment;
   if (useInt8GuardVariable) {
     guardTy = CGF.Int8Ty;
     guardAlignment = CharUnits::One();
   } else {
     // Guard variables are 64 bits in the generic ABI and size width on ARM

clang/test/CodeGenCXX/aix-static-init-temp-spec-and-inline-var.cpp

 // CHECK:   br label %destruct.end
 
 // CHECK: destruct.end:
 // CHECK:   ret void
 // CHECK: }
 
 // 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
 
 // CHECK: init:
 // 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:
 // CHECK:   ret void
 // CHECK: }
 
 // CHECK: define internal void @__dtor__ZN5test12t2E() [[ATTR:#[0-9]+]] {
 // CHECK: entry:

 // 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:
-// 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:
 // CHECK:   ret void
 // CHECK: }
 
 // CHECK: define internal void @__dtor__ZN5test21AIvE8instanceE() [[ATTR:#[0-9]+]] {
 // CHECK: entry:

 // CHECK:   br label %destruct.end
 
 // CHECK: destruct.end:
 // CHECK:   ret void
 // CHECK: }
 
 // 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
 
 // CHECK: init.check:
 // 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:   br label %init.end
 
 // CHECK: init.end:
 // CHECK:   ret void
 // CHECK: }
 
 // CHECK: define internal void @__dtor__ZN5test12t1IiEE() [[ATTR:#[0-9]+]] {
 // CHECK: entry:

clang/test/CodeGenCXX/aix-static-init.cpp

 // CHECK:   br label %destruct.end
 
 // CHECK: destruct.end:
 // CHECK:   ret void
 // CHECK: }
 
 // 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:
 // CHECK:   ret void
 // CHECK: }
 
 // CHECK: define internal void @__dtor__ZZN5test41fEvE11staticLocal() [[ATTR:#[0-9]+]] {
 // CHECK: entry: