Hi James, That’s perfectly fine. It’s important to get resolved, but not super urgent.
Thanks, Jim > On Jul 18, 2014, at 1:00 PM, James Molloy <[email protected]> wrote: > > Hi Jim, > > Understood - I'm on vacation until Tuesday, and I'll need to have a chat with > others about the right way forward here. Is delaying this until Tuesday OK > with you? > > Cheers, > James > From: Jim Grosbach > Sent: 18/07/2014 19:57 > To: James Molloy > Cc: [email protected] > Subject: Re: r206963 - [ARM64] Change inline assembly constraints to be more > lax, to match the behaviour of Clang/AArch64 and GCC. > > Hi James, > > I’d like to revisit this. There were objections raised when this first went > in that were never resolved. This warning was added because it actively found > bugs, not for a theoretical concern. The lack of this error is now actively > causing problems. I would very much like to see this reverted entirely. At > the very least, please re-implement this and keep the old behavior, including > the diagnostic, for Darwin. If other platforms want to be more flexible and > allow potentially broken code to go through undiagnosed, on their own heads > be it. > > -Jim > > > On Apr 23, 2014, at 3:26 AM, James Molloy <[email protected]> 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 > > [email protected] > > 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
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
