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