<div dir="ltr">Are you sure the i8 load and i64 store are correct on big-endian machines? I was just looking at the IR generated by this code, and it didn't look correct if the 0x01 byte is stored in the byte with the highest address:<div><br></div><div><div>define internal void @__cxx_global_var_init() #2 section ".text.startup" comdat($_ZN9TypeIndexI3FooE5indexE) {</div><div>entry:</div><div> %0 = load i8, i8* bitcast (i64* @_ZGVN9TypeIndexI3FooE5indexE to i8*), align 1</div><div> %guard.uninitialized = icmp eq i8 %0, 0</div><div> br i1 %guard.uninitialized, label %init.check, label %init.end</div><div><br></div><div>init.check: ; preds = %entry</div><div> %1 = load i32, i32* @count, align 4</div><div> %inc = add nsw i32 %1, 1</div><div> store i32 %inc, i32* @count, align 4</div><div> store i32 %1, i32* @_ZN9TypeIndexI3FooE5indexE, align 4</div><div> store i64 1, i64* @_ZGVN9TypeIndexI3FooE5indexE</div><div> br label %init.end</div><div><br></div><div>init.end: ; preds = %init.check, %entry</div><div> ret void</div><div>}</div></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Apr 22, 2014 at 6:50 PM, Justin Bogner <span dir="ltr"><<a href="mailto:mail@justinbogner.com" target="_blank">mail@justinbogner.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: bogner<br>
Date: Tue Apr 22 20:50:10 2014<br>
New Revision: 206937<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=206937&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=206937&view=rev</a><br>
Log:<br>
CodeGen: Unify handling guard variables in the Itanium C++ ABI<br>
<br>
We previously treated ARM separately from the generic Itanium ABI for<br>
initializing guard variables. This code duplication led to things like<br>
the ARM path missing the memory barrier for threadsafe handling, and a<br>
highly misleading comment about how we were (mis)using the generic ABI<br>
for ARM64 when really it went through the ARM codepath.<br>
<br>
This unifies the two code paths. Functionally, this changes the ARM<br>
and ARM64 codepath to use one byte loads instead of 4 and 8,<br>
respectively, and adds the missing atomic acquire to these loads.<br>
Other architectures are unchanged.<br>
<br>
Modified:<br>
cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp<br>
cfe/trunk/test/CodeGenCXX/aarch64-cxxabi.cpp<br>
cfe/trunk/test/CodeGenCXX/arm.cpp<br>
<br>
Modified: cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp?rev=206937&r1=206936&r2=206937&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp?rev=206937&r1=206936&r2=206937&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp (original)<br>
+++ cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp Tue Apr 22 20:50:10 2014<br>
@@ -1393,25 +1393,7 @@ void ItaniumCXXABI::EmitGuardedInit(Code<br>
}<br>
<br>
// Test whether the variable has completed initialization.<br>
- llvm::Value *isInitialized;<br>
-<br>
- // ARM C++ ABI <a href="http://3.2.3.1" target="_blank">3.2.3.1</a>:<br>
- // To support the potential use of initialization guard variables<br>
- // as semaphores that are the target of ARM SWP and LDREX/STREX<br>
- // synchronizing instructions we define a static initialization<br>
- // guard variable to be a 4-byte aligned, 4- byte word with the<br>
- // following inline access protocol.<br>
- // #define INITIALIZED 1<br>
- // if ((obj_guard & INITIALIZED) != INITIALIZED) {<br>
- // if (__cxa_guard_acquire(&obj_guard))<br>
- // ...<br>
- // }<br>
- if (UseARMGuardVarABI && !useInt8GuardVariable) {<br>
- llvm::Value *V = Builder.CreateLoad(guard);<br>
- llvm::Value *Test1 = llvm::ConstantInt::get(guardTy, 1);<br>
- V = Builder.CreateAnd(V, Test1);<br>
- isInitialized = Builder.CreateIsNull(V, "guard.uninitialized");<br>
-<br>
+ //<br>
// Itanium C++ ABI 3.3.2:<br>
// The following is pseudo-code showing how these functions can be used:<br>
// if (obj_guard.first_byte == 0) {<br>
@@ -1427,29 +1409,45 @@ void ItaniumCXXABI::EmitGuardedInit(Code<br>
// }<br>
// }<br>
<br>
- // ARM64 C++ ABI 3.2.2:<br>
- // This ABI instead only specifies the value bit 0 of the static guard<br>
- // variable; all other bits are platform defined. Bit 0 shall be 0 when the<br>
- // variable is not initialized and 1 when it is.<br>
- // FIXME: Reading one bit is no more efficient than reading one byte so<br>
- // the codegen is same as generic Itanium ABI.<br>
- } else {<br>
- // Load the first byte of the guard variable.<br>
- llvm::LoadInst *LI =<br>
+ // Load the first byte of the guard variable.<br>
+ llvm::LoadInst *LI =<br>
Builder.CreateLoad(Builder.CreateBitCast(guard, CGM.Int8PtrTy));<br>
- LI->setAlignment(1);<br>
+ LI->setAlignment(1);<br>
<br>
- // Itanium ABI:<br>
- // An implementation supporting thread-safety on multiprocessor<br>
- // systems must also guarantee that references to the initialized<br>
- // object do not occur before the load of the initialization flag.<br>
- //<br>
- // In LLVM, we do this by marking the load Acquire.<br>
- if (threadsafe)<br>
- LI->setAtomic(llvm::Acquire);<br>
+ // Itanium ABI:<br>
+ // An implementation supporting thread-safety on multiprocessor<br>
+ // systems must also guarantee that references to the initialized<br>
+ // object do not occur before the load of the initialization flag.<br>
+ //<br>
+ // In LLVM, we do this by marking the load Acquire.<br>
+ if (threadsafe)<br>
+ LI->setAtomic(llvm::Acquire);<br>
<br>
- isInitialized = Builder.CreateIsNull(LI, "guard.uninitialized");<br>
- }<br>
+ // For ARM, we should only check the first bit, rather than the entire byte:<br>
+ //<br>
+ // ARM C++ ABI <a href="http://3.2.3.1" target="_blank">3.2.3.1</a>:<br>
+ // To support the potential use of initialization guard variables<br>
+ // as semaphores that are the target of ARM SWP and LDREX/STREX<br>
+ // synchronizing instructions we define a static initialization<br>
+ // guard variable to be a 4-byte aligned, 4-byte word with the<br>
+ // following inline access protocol.<br>
+ // #define INITIALIZED 1<br>
+ // if ((obj_guard & INITIALIZED) != INITIALIZED) {<br>
+ // if (__cxa_guard_acquire(&obj_guard))<br>
+ // ...<br>
+ // }<br>
+ //<br>
+ // and similarly for ARM64:<br>
+ //<br>
+ // ARM64 C++ ABI 3.2.2:<br>
+ // This ABI instead only specifies the value bit 0 of the static guard<br>
+ // variable; all other bits are platform defined. Bit 0 shall be 0 when the<br>
+ // variable is not initialized and 1 when it is.<br>
+ llvm::Value *V =<br>
+ (UseARMGuardVarABI && !useInt8GuardVariable)<br>
+ ? Builder.CreateAnd(LI, llvm::ConstantInt::get(CGM.Int8Ty, 1))<br>
+ : LI;<br>
+ llvm::Value *isInitialized = Builder.CreateIsNull(V, "guard.uninitialized");<br>
<br>
llvm::BasicBlock *InitCheckBlock = CGF.createBasicBlock("init.check");<br>
llvm::BasicBlock *EndBlock = CGF.createBasicBlock("init.end");<br>
<br>
Modified: cfe/trunk/test/CodeGenCXX/aarch64-cxxabi.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/aarch64-cxxabi.cpp?rev=206937&r1=206936&r2=206937&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/aarch64-cxxabi.cpp?rev=206937&r1=206936&r2=206937&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/test/CodeGenCXX/aarch64-cxxabi.cpp (original)<br>
+++ cfe/trunk/test/CodeGenCXX/aarch64-cxxabi.cpp Tue Apr 22 20:50:10 2014<br>
@@ -40,8 +40,8 @@ public:<br>
<br>
void guard_variables(int a) {<br>
static Guarded mine(a);<br>
-// CHECK: [[GUARDBIT:%[0-9]+]] = and i64 {{%[0-9]+}}, 1<br>
-// CHECK: icmp eq i64 [[GUARDBIT]], 0<br>
+// CHECK: [[GUARDBIT:%[0-9]+]] = and i8 {{%[0-9]+}}, 1<br>
+// CHECK: icmp eq i8 [[GUARDBIT]], 0<br>
<br>
// As guards are 64-bit, these helpers should take 64-bit pointers.<br>
// CHECK: call i32 @__cxa_guard_acquire(i64*<br>
<br>
Modified: cfe/trunk/test/CodeGenCXX/arm.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/arm.cpp?rev=206937&r1=206936&r2=206937&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/arm.cpp?rev=206937&r1=206936&r2=206937&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/test/CodeGenCXX/arm.cpp (original)<br>
+++ cfe/trunk/test/CodeGenCXX/arm.cpp Tue Apr 22 20:50:10 2014<br>
@@ -292,9 +292,9 @@ namespace test7 {<br>
<br>
// CHECK-LABEL: define void @_ZN5test74testEv()<br>
void test() {<br>
- // CHECK: [[T0:%.*]] = load i32* @_ZGVZN5test74testEvE1x<br>
- // CHECK-NEXT: [[T1:%.*]] = and i32 [[T0]], 1<br>
- // CHECK-NEXT: [[T2:%.*]] = icmp eq i32 [[T1]], 0<br>
+ // CHECK: [[T0:%.*]] = load atomic i8* bitcast (i32* @_ZGVZN5test74testEvE1x to i8*) acquire, align 1<br>
+ // CHECK-NEXT: [[T1:%.*]] = and i8 [[T0]], 1<br>
+ // CHECK-NEXT: [[T2:%.*]] = icmp eq i8 [[T1]], 0<br>
// CHECK-NEXT: br i1 [[T2]]<br>
// -> fallthrough, end<br>
// CHECK: [[T3:%.*]] = call i32 @__cxa_guard_acquire(i32* @_ZGVZN5test74testEvE1x)<br>
@@ -327,9 +327,9 @@ namespace test8 {<br>
<br>
// CHECK-LABEL: define void @_ZN5test84testEv()<br>
void test() {<br>
- // CHECK: [[T0:%.*]] = load i32* @_ZGVZN5test84testEvE1x<br>
- // CHECK-NEXT: [[T1:%.*]] = and i32 [[T0]], 1<br>
- // CHECK-NEXT: [[T2:%.*]] = icmp eq i32 [[T1]], 0<br>
+ // CHECK: [[T0:%.*]] = load atomic i8* bitcast (i32* @_ZGVZN5test84testEvE1x to i8*) acquire, align 1<br>
+ // CHECK-NEXT: [[T1:%.*]] = and i8 [[T0]], 1<br>
+ // CHECK-NEXT: [[T2:%.*]] = icmp eq i8 [[T1]], 0<br>
// CHECK-NEXT: br i1 [[T2]]<br>
// -> fallthrough, end<br>
// CHECK: [[T3:%.*]] = call i32 @__cxa_guard_acquire(i32* @_ZGVZN5test84testEvE1x)<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</blockquote></div><br></div>