r206963 - [ARM64] Change inline assembly constraints to be more lax, to match the behaviour of Clang/AArch64 and GCC.
James Molloy
James.Molloy at arm.com
Thu Apr 24 01:24:54 PDT 2014
Hi Jim, Eric,
I originally discussed with Tim what he thought the behaviour should be, as I actually had no opinion but the tests were conflicting. He recommended to stick with "what GCC does". Adding him on CC so he can weigh in.
I have no issue with reverting this behaviour change. But if we're incompatible with GCC, will we not be asked to change the behaviour back again at a later point, for example by the LLVMLinux project?
Cheers,
James
> -----Original Message-----
> From: Jim Grosbach [mailto:grosbach at apple.com]
> Sent: 24 April 2014 00:35
> To: Eric Christopher; James Molloy
> Cc: cfe-commits at cs.uiuc.edu
> Subject: Re: r206963 - [ARM64] Change inline assembly constraints to be
> more lax, to match the behaviour of Clang/AArch64 and GCC.
>
> I share Eric's concern here. As I recall, the spec from ARM was very clear on
> this point, requiring a modifier to specify anything other than a 64-bit GPR.
> Matching the behavior of GCC is not a sufficient reason to change that unless
> the specification changed as well.
>
> -Jim
>
> On Apr 23, 2014, at 4:29 PM, Eric Christopher <echristo at gmail.com> wrote:
>
> > Ugh.
> >
> > Why? Are people writing bad code that doesn't conform to the processor?
> >
> > -eric
> >
> > On Wed, Apr 23, 2014 at 3:26 AM, James Molloy <james.molloy at arm.com>
> wrote:
> >> Author: jamesm
> >> Date: Wed Apr 23 05:26:19 2014
> >> New Revision: 206963
> >>
> >> URL: http://llvm.org/viewvc/llvm-project?rev=206963&view=rev
> >> Log:
> >> [ARM64] Change inline assembly constraints to be more lax, to match the
> behaviour of Clang/AArch64 and GCC.
> >>
> >> GCC allows sub-64bit values to use the 'r' register constraint.
> >>
> >> Modified:
> >> cfe/trunk/lib/Basic/Targets.cpp
> >> cfe/trunk/test/CodeGen/aarch64-inline-asm.c
> >> cfe/trunk/test/Sema/arm64-inline-asm.c
> >> cfe/trunk/test/Sema/inline-asm-validate.c
> >>
> >> Modified: cfe/trunk/lib/Basic/Targets.cpp
> >> URL: http://llvm.org/viewvc/llvm-
> project/cfe/trunk/lib/Basic/Targets.cpp?rev=206963&r1=206962&r2=206963
> &view=diff
> >>
> ==========================================================
> ====================
> >> --- cfe/trunk/lib/Basic/Targets.cpp (original)
> >> +++ cfe/trunk/lib/Basic/Targets.cpp Wed Apr 23 05:26:19 2014
> >> @@ -4643,46 +4643,37 @@ public:
> >> case 'w': // Floating point and SIMD registers (V0-V31)
> >> Info.setAllowsRegister();
> >> return true;
> >> + case 'I': // Constant that can be used with an ADD instruction
> >> + case 'J': // Constant that can be used with a SUB instruction
> >> + case 'K': // Constant that can be used with a 32-bit logical instruction
> >> + case 'L': // Constant that can be used with a 64-bit logical instruction
> >> + case 'M': // Constant that can be used as a 32-bit MOV immediate
> >> + case 'N': // Constant that can be used as a 64-bit MOV immediate
> >> + case 'Y': // Floating point constant zero
> >> + case 'Z': // Integer constant zero
> >> + return true;
> >> + case 'Q': // A memory reference with base register and no offset
> >> + Info.setAllowsMemory();
> >> + return true;
> >> + case 'S': // A symbolic address
> >> + Info.setAllowsRegister();
> >> + return true;
> >> + case 'U':
> >> + // Ump: A memory address suitable for ldp/stp in SI, DI, SF and DF
> modes, whatever they may be
> >> + // Utf: A memory address suitable for ldp/stp in TF mode, whatever it
> may be
> >> + // Usa: An absolute symbolic address
> >> + // Ush: The high part (bits 32:12) of a pc-relative symbolic address
> >> + llvm_unreachable("FIXME: Unimplemented support for bizarre
> constraints");
> >> case 'z': // Zero register, wzr or xzr
> >> Info.setAllowsRegister();
> >> return true;
> >> case 'x': // Floating point and SIMD registers (V0-V15)
> >> Info.setAllowsRegister();
> >> return true;
> >> - case 'Q': // A memory address that is a single base register.
> >> - Info.setAllowsMemory();
> >> - return true;
> >> }
> >> return false;
> >> }
> >>
> >> - virtual bool validateConstraintModifier(StringRef Constraint,
> >> - const char Modifier,
> >> - unsigned Size) const {
> >> - // Strip off constraint modifiers.
> >> - while (Constraint[0] == '=' || Constraint[0] == '+' || Constraint[0] ==
> '&')
> >> - Constraint = Constraint.substr(1);
> >> -
> >> - switch (Constraint[0]) {
> >> - default:
> >> - return true;
> >> - case 'z':
> >> - case 'r': {
> >> - switch (Modifier) {
> >> - case 'x':
> >> - case 'w':
> >> - // For now assume that the person knows what they're
> >> - // doing with the modifier.
> >> - return true;
> >> - default:
> >> - // By default an 'r' constraint will be in the 'x'
> >> - // registers.
> >> - return (Size == 64);
> >> - }
> >> - }
> >> - }
> >> - }
> >> -
> >> virtual const char *getClobbers() const { return ""; }
> >>
> >> int getEHDataRegisterNumber(unsigned RegNo) const {
> >>
> >> Modified: cfe/trunk/test/CodeGen/aarch64-inline-asm.c
> >> URL: http://llvm.org/viewvc/llvm-
> project/cfe/trunk/test/CodeGen/aarch64-inline-
> asm.c?rev=206963&r1=206962&r2=206963&view=diff
> >>
> ==========================================================
> ====================
> >> --- cfe/trunk/test/CodeGen/aarch64-inline-asm.c (original)
> >> +++ cfe/trunk/test/CodeGen/aarch64-inline-asm.c Wed Apr 23 05:26:19
> 2014
> >> @@ -1,4 +1,5 @@
> >> // RUN: %clang_cc1 -triple aarch64-none-linux-gnu -emit-llvm -o - %s |
> FileCheck %s
> >> +// RUN: %clang_cc1 -triple arm64-none-linux-gnu -emit-llvm -o - %s |
> FileCheck %s
> >>
> >> // The only part clang really deals with is the lvalue/rvalue
> >> // distinction on constraints. It's sufficient to emit llvm and make
> >>
> >> Modified: cfe/trunk/test/Sema/arm64-inline-asm.c
> >> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/arm64-
> inline-asm.c?rev=206963&r1=206962&r2=206963&view=diff
> >>
> ==========================================================
> ====================
> >> --- cfe/trunk/test/Sema/arm64-inline-asm.c (original)
> >> +++ cfe/trunk/test/Sema/arm64-inline-asm.c Wed Apr 23 05:26:19 2014
> >> @@ -1,8 +1,9 @@
> >> // RUN: %clang_cc1 -triple arm64-apple-ios7.1 -fsyntax-only -verify %s
> >> +// expected-no-diagnostics
> >> +
> >> void foo() {
> >> asm volatile("USE(%0)" :: "z"(0LL));
> >> asm volatile("USE(%x0)" :: "z"(0LL));
> >> asm volatile("USE(%w0)" :: "z"(0));
> >>
> >> - asm volatile("USE(%0)" :: "z"(0)); // expected-warning {{value size does
> not match register size specified by the constraint and modifier}}
> >> }
> >>
> >> Modified: cfe/trunk/test/Sema/inline-asm-validate.c
> >> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/inline-
> asm-validate.c?rev=206963&r1=206962&r2=206963&view=diff
> >>
> ==========================================================
> ====================
> >> --- cfe/trunk/test/Sema/inline-asm-validate.c (original)
> >> +++ cfe/trunk/test/Sema/inline-asm-validate.c Wed Apr 23 05:26:19 2014
> >> @@ -1,8 +1,9 @@
> >> // RUN: %clang_cc1 -triple arm64-apple-macosx10.8.0 -fsyntax-only -
> verify %s
> >> +// expected-no-diagnostics
> >>
> >> unsigned t, r, *p;
> >>
> >> int foo (void) {
> >> - __asm__ __volatile__( "stxr %w[_t], %[_r], [%[_p]]" : [_t] "=&r" (t) :
> [_p] "p" (p), [_r] "r" (r) : "memory"); // expected-warning {{value size does
> not match register size specified by the constraint and modifier}}
> >> + __asm__ __volatile__( "stxr %w[_t], %[_r], [%[_p]]" : [_t] "=&r" (t) :
> [_p] "p" (p), [_r] "r" (r) : "memory");
> >> return 1;
> >> }
> >>
> >>
> >> _______________________________________________
> >> cfe-commits mailing list
> >> cfe-commits at cs.uiuc.edu
> >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
> > _______________________________________________
> > cfe-commits mailing list
> > cfe-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
-- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No: 2557590
ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No: 2548782
More information about the cfe-commits
mailing list