Identified a while before the release, there is an issue with 64-bit atomics on ARM that was making Clang mis-compile a lot of code, including Clang itself. http://llvm.org/bugs/show_bug.cgi?id=15429 Attached is a patch proposed by Benjamin with the corrections to the test. I'm not an expert on how Clang lowers C11 atomics, but the resulting IR seems correct, and after self-hosting Clang with this patch, it managed to pass all test-suite (as before), which is a good indication that the patch does fix the problem. Please review! cheers, --renato -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20130513/96b2ef27/attachment.html> -------------- next part -------------- A non-text attachment was scrubbed... Name: atomic_arm.patch Type: application/octet-stream Size: 6943 bytes Desc: not available URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20130513/96b2ef27/attachment.obj>
Hi Rafael, As you mentioned in the bug, we should only apply this change when hard-float is set, which it is by default on armv7a, I presume.>From that part of the code, I can infer that by the time"MaxAtomicPromoteWidth = 64;", the variable SoftFloat is not properly set, so a simple "if (!SoftFloat)" won't cut in there. It seems SoftFloat is being set on HandleTargetFeatures() which is a virtual method, probably called indirectly. Do you have a better place to set MaxAtomicInlineWidth 64? cheers, --renato On 13 May 2013 13:56, Renato Golin <renato.golin at linaro.org> wrote:> Identified a while before the release, there is an issue with 64-bit > atomics on ARM that was making Clang mis-compile a lot of code, including > Clang itself. > > http://llvm.org/bugs/show_bug.cgi?id=15429 > > Attached is a patch proposed by Benjamin with the corrections to the test. > > I'm not an expert on how Clang lowers C11 atomics, but the resulting IR > seems correct, and after self-hosting Clang with this patch, it managed to > pass all test-suite (as before), which is a good indication that the patch > does fix the problem. > > Please review! > > cheers, > --renato >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20130513/c2b8fbc4/attachment.html>
On 13 May 2013 10:59, Renato Golin <renato.golin at linaro.org> wrote:> Hi Rafael, > > As you mentioned in the bug, we should only apply this change when > hard-float is set, which it is by default on armv7a, I presume. > > From that part of the code, I can infer that by the time > "MaxAtomicPromoteWidth = 64;", the variable SoftFloat is not properly set, > so a simple "if (!SoftFloat)" won't cut in there. It seems SoftFloat is > being set on HandleTargetFeatures() which is a virtual method, probably > called indirectly. Do you have a better place to set MaxAtomicInlineWidth > 64?I have asked on #gcc what gcc does. I have posted a detailed description in the bug, but the summary is that there is some cooperation with the kernel going on that should make it safe to set MaxAtomicInlineWidth to 64 when targeting linux armv6 or newer. Hard float using it then just becomes a consequence of it implying armv7. I know think we need something like the attached patch (with tests and comments added). Cheers, Rafael -------------- next part -------------- A non-text attachment was scrubbed... Name: t.patch Type: application/octet-stream Size: 1135 bytes Desc: not available URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20130513/d36a4baf/attachment.obj>