Hello! Here is a patch for this bug, as it is still unresolved and the behavior can still be reproduced in trunk.
It looks like it is simply an issue with the return type deduction of __atomic_* builtins. They are not using an unqualified version of the value type, whereas the __sync_* builtins are. Regards, -- Beren Minor On Wed, Nov 20, 2013 at 12:07 AM, Reid Kleckner <[email protected]> wrote: > I think that's this bug here: > http://llvm.org/bugs/show_bug.cgi?id=17306 > > Probably it's a bug in Clang's builtin IR generation. > > > On Tue, Nov 19, 2013 at 3:11 AM, Beren Minor <[email protected]> > wrote: > >> Hello, >> >> I found some strange codegen output when playing with Clang's >> implementation of GCC atomic builtins and would like to know if this is >> some expected behavior or known issue. >> >> The test case is very simple, and the disassembly can be seen here: >> http://bit.ly/18LED7t >> >> It looks to me that these instructions after the load are unnecessary, >> and GCC does not generate them: >> movl %eax, -4(%rsp) >> movl -4(%rsp), %eax >> >> After investigating, it appears that this is caused by the volatile >> qualifier of the pointer parameter. Without it, the generated code is the >> same as GCC. >> >> It looks like the volatile qualifier is propagated to some temporary >> variable that Clang uses in the LLVM IR, and after storing the atomic load >> result in this temporary, it has to reload it again before returning it. >> Showing the IR with -emit-llvm exhibits clearly this issue. >> >> It seems to me that is is unnecessary to make this temporary volatile, >> isn't it? >> -- >> Beren Minor >> >> _______________________________________________ >> cfe-dev mailing list >> [email protected] >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev >> >> >
From ebd3d3d3288c7b0757a7537ffd968a872e565ca1 Mon Sep 17 00:00:00 2001 From: Beren Minor <[email protected]> Date: Wed, 10 Dec 2014 14:24:20 +0100 Subject: [PATCH] Remove qualifiers on deduced __atomic builtins return type. Returned temporary does not need to be qualified, and this causes unnecessary store/load sequence to be generated by the temporary to r-value conversion when calling an __atomic_* builtin on a volatile variable. (#17306) __sync_* builtins are already using unqualified type for the return type. --- lib/Sema/SemaChecking.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/Sema/SemaChecking.cpp b/lib/Sema/SemaChecking.cpp index 8e55925..07fea12 100644 --- a/lib/Sema/SemaChecking.cpp +++ b/lib/Sema/SemaChecking.cpp @@ -1304,7 +1304,8 @@ ExprResult Sema::SemaAtomicOpsOverloaded(ExprResult TheCallResult, return ExprError(); } - QualType ResultType = ValType; + // Strip any qualifiers off ValType. + QualType ResultType = ValType.getUnqualifiedType(); if (Form == Copy || Form == GNUXchg || Form == Init) ResultType = Context.VoidTy; else if (Form == C11CmpXchg || Form == GNUCmpXchg) -- 2.2.0
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
