RE: r290539 - [inline-asm]No error for conflict between inputs\outputs and clobber list

[Yatsina, Marina via cfe-commits]

It should.
Ziv, there seems to be some bug with the implementation, can you fix it please?

Thanks,
Marina

-Original Message-
From: Dimitry Andric [mailto:dimi...@andric.com] 
Sent: Tuesday, January 31, 2017 22:39
To: Yatsina, Marina 
Cc: cfe-commits ; Izhar, Ziv 
Subject: Re: r290539 - [inline-asm]No error for conflict between inputs\outputs 
and clobber list

On 26 Dec 2016, at 13:23, Marina Yatsina via cfe-commits 
 wrote:
> 
> Author: myatsina
> Date: Mon Dec 26 06:23:42 2016
> New Revision: 290539
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=290539=rev
> Log:
> [inline-asm]No error for conflict between inputs\outputs and clobber 
> list
> 
> According to extended asm syntax, a case where the clobber list includes a 
> variable from the inputs or outputs should be an error - conflict.
> for example:
> 
> const long double a = 0.0;
> int main()
> {
> 
> char b;
> double t1 = a;
> __asm__ ("fucompp": "=a" (b) : "u" (t1), "t" (t1) : "cc", "st", 
> "st(1)");
> 
> return 0;
> }
> 
> This should conflict with the output - t1 which is st, and st which is st 
> aswell.

In FreeBSD ports, we are now getting this new error message when compiling 
svgalib (see https://bugs.freebsd.org/216154), when compiling with clang 4.0.0.

While fixing it, we noticed something strange (or interesting, depending on 
your point of view), namely that it does not seem to handle the first input or 
output operand, e.g. "0".  For example, svgalib has this inline function:

/* Always 32-bit align destination, even for a small number of bytes. */ static 
inline void *  __memcpy_aligndest(void *dest, const void *src, int n) {
__asm__ __volatile__("cmpl $3, %%ecx\n\t"
 "ja 1f\n\t"
 "call * __memcpy_jumptable (, %%ecx, 4)\n\t"
 "jmp 2f\n\t"
 "1:call __memcpyasm_regargs\n\t"
 "2:":
 :"S"(dest), "d"(src), "c"(n)
 :"ax", "0", "1", "2");
return dest;
}

The error produced for this is:

svgalib-1.4.3/gl/inlstring.h:281:17: error: asm-specifier for input or output 
variable conflicts with asm clobber list
 :"ax", "0", "1", "2");
 ^

And indeed, removing the "1" and "2" input operands fixes the error.  However, 
by definition, "0" is always an input or output operand, so should it not 
produce an error too?

-Dimitry

-
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r290539 - [inline-asm]No error for conflict between inputs\outputs and clobber list

[Dimitry Andric via cfe-commits]

On 26 Dec 2016, at 13:23, Marina Yatsina via cfe-commits 
 wrote:
> 
> Author: myatsina
> Date: Mon Dec 26 06:23:42 2016
> New Revision: 290539
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=290539=rev
> Log:
> [inline-asm]No error for conflict between inputs\outputs and clobber list
> 
> According to extended asm syntax, a case where the clobber list includes a 
> variable from the inputs or outputs should be an error - conflict.
> for example:
> 
> const long double a = 0.0;
> int main()
> {
> 
> char b;
> double t1 = a;
> __asm__ ("fucompp": "=a" (b) : "u" (t1), "t" (t1) : "cc", "st", "st(1)");
> 
> return 0;
> }
> 
> This should conflict with the output - t1 which is st, and st which is st 
> aswell.

In FreeBSD ports, we are now getting this new error message when compiling 
svgalib (see https://bugs.freebsd.org/216154), when compiling with clang 4.0.0.

While fixing it, we noticed something strange (or interesting, depending on 
your point of view), namely that it does not seem to handle the first input or 
output operand, e.g. "0".  For example, svgalib has this inline function:

/* Always 32-bit align destination, even for a small number of bytes. */
static inline void *
 __memcpy_aligndest(void *dest, const void *src, int n)
{
__asm__ __volatile__("cmpl $3, %%ecx\n\t"
 "ja 1f\n\t"
 "call * __memcpy_jumptable (, %%ecx, 4)\n\t"
 "jmp 2f\n\t"
 "1:call __memcpyasm_regargs\n\t"
 "2:":
 :"S"(dest), "d"(src), "c"(n)
 :"ax", "0", "1", "2");
return dest;
}

The error produced for this is:

svgalib-1.4.3/gl/inlstring.h:281:17: error: asm-specifier for input or output 
variable conflicts with asm clobber list
 :"ax", "0", "1", "2");
 ^

And indeed, removing the "1" and "2" input operands fixes the error.  However, 
by definition, "0" is always an input or output operand, so should it not 
produce an error too?

-Dimitry



signature.asc
Description: Message signed with OpenPGP using GPGMail
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

RE: r290539 - [inline-asm]No error for conflict between inputs\outputs and clobber list

[Yatsina, Marina via cfe-commits]

Adding Ziv, the author of this patch.

From: Chandler Carruth [mailto:chandl...@google.com]
Sent: Thursday, December 29, 2016 11:57
To: Yatsina, Marina ; cfe-commits@lists.llvm.org
Subject: Re: r290539 - [inline-asm]No error for conflict between inputs\outputs 
and clobber list

On Mon, Dec 26, 2016 at 4:34 AM Marina Yatsina via cfe-commits 
> wrote:
Author: myatsina
Date: Mon Dec 26 06:23:42 2016
New Revision: 290539

URL: http://llvm.org/viewvc/llvm-project?rev=290539=rev
Log:
[inline-asm]No error for conflict between inputs\outputs and clobber list

According to extended asm syntax, a case where the clobber list includes a 
variable from the inputs or outputs should be an error - conflict.
for example:

const long double a = 0.0;
int main()
{

char b;
double t1 = a;
__asm__ ("fucompp": "=a" (b) : "u" (t1), "t" (t1) : "cc", "st", "st(1)");

return 0;
}

This should conflict with the output - t1 which is st, and st which is st 
aswell.
The patch fixes it.

Commit on behald of Ziv Izhar.

Differential Revision: https://reviews.llvm.org/D15075


Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/include/clang/Basic/TargetInfo.h
cfe/trunk/lib/Basic/TargetInfo.cpp
cfe/trunk/lib/Basic/Targets.cpp
cfe/trunk/lib/Headers/intrin.h
cfe/trunk/lib/Sema/SemaStmtAsm.cpp
cfe/trunk/test/Sema/asm.c

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=290539=290538=290539=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Mon Dec 26 06:23:42 
2016
@@ -7069,6 +7069,10 @@ let CategoryName = "Inline Assembly Issu
 "constraint '%0' is already present here">;
 }

+  def error_inoutput_conflict_with_clobber : Error<
+"asm-specifier for input or output variable conflicts with asm"
+" clobber list">;

Clang generally works to avoid this kind of error message. All this does is say 
"there was a problem of this kind" without identifying any of the specifics. 
And for this error in particular I think this is of the utmost importance. 
Developers are not going to understand what went wrong here.

I would suggest at a minimum to enhance this to explain:

1) What operands and clobbers conflict, preferably with source ranges 
underlining them.

2) Why they conflict (for example the fact that "D" means the di register 
group, of which "%rdi" is a member)

Beyond that, I wonder if you could add a note suggesting to remove the clobber 
if the input (or output) operand is sufficient.

You can make this note explain carefully the case where something would need to 
be added to the inputs or outputs instead, but I think it at least makes sense 
to clarify that this will be a common fix and what to look out for that might 
make it an incorrect fix.
-
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r290539 - [inline-asm]No error for conflict between inputs\outputs and clobber list

[Chandler Carruth via cfe-commits]

On Mon, Dec 26, 2016 at 4:34 AM Marina Yatsina via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: myatsina
> Date: Mon Dec 26 06:23:42 2016
> New Revision: 290539
>
> URL: http://llvm.org/viewvc/llvm-project?rev=290539=rev
> Log:
> [inline-asm]No error for conflict between inputs\outputs and clobber list
>
> According to extended asm syntax, a case where the clobber list includes a
> variable from the inputs or outputs should be an error - conflict.
> for example:
>
> const long double a = 0.0;
> int main()
> {
>
> char b;
> double t1 = a;
> __asm__ ("fucompp": "=a" (b) : "u" (t1), "t" (t1) : "cc", "st", "st(1)");
>
> return 0;
> }
>
> This should conflict with the output - t1 which is st, and st which is st
> aswell.
> The patch fixes it.


> Commit on behald of Ziv Izhar.
>
> Differential Revision: https://reviews.llvm.org/D15075
>
>
> Modified:
> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> cfe/trunk/include/clang/Basic/TargetInfo.h
> cfe/trunk/lib/Basic/TargetInfo.cpp
> cfe/trunk/lib/Basic/Targets.cpp
> cfe/trunk/lib/Headers/intrin.h
> cfe/trunk/lib/Sema/SemaStmtAsm.cpp
> cfe/trunk/test/Sema/asm.c
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=290539=290538=290539=diff
>
> ==
> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Mon Dec 26
> 06:23:42 2016
> @@ -7069,6 +7069,10 @@ let CategoryName = "Inline Assembly Issu
>  "constraint '%0' is already present here">;
>  }
>
> +  def error_inoutput_conflict_with_clobber : Error<
> +"asm-specifier for input or output variable conflicts with asm"
> +" clobber list">;
>

Clang generally works to avoid this kind of error message. All this does is
say "there was a problem of this kind" without identifying any of the
specifics. And for this error in particular I think this is of the utmost
importance. Developers are not going to understand what went wrong here.

I would suggest at a minimum to enhance this to explain:

1) What operands and clobbers conflict, preferably with source ranges
underlining them.

2) Why they conflict (for example the fact that "D" means the di register
group, of which "%rdi" is a member)

Beyond that, I wonder if you could add a note suggesting to remove the
clobber if the input (or output) operand is sufficient.

You can make this note explain carefully the case where something would
need to be added to the inputs or outputs instead, but I think it at least
makes sense to clarify that this will be a common fix and what to look out
for that might make it an incorrect fix.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r290539 - [inline-asm]No error for conflict between inputs\outputs and clobber list

[Marina Yatsina via cfe-commits]

Author: myatsina
Date: Mon Dec 26 06:23:42 2016
New Revision: 290539

URL: http://llvm.org/viewvc/llvm-project?rev=290539=rev
Log:
[inline-asm]No error for conflict between inputs\outputs and clobber list

According to extended asm syntax, a case where the clobber list includes a 
variable from the inputs or outputs should be an error - conflict.
for example:

const long double a = 0.0;
int main()
{

char b;
double t1 = a;
__asm__ ("fucompp": "=a" (b) : "u" (t1), "t" (t1) : "cc", "st", "st(1)");

return 0;
}

This should conflict with the output - t1 which is st, and st which is st 
aswell.
The patch fixes it.

Commit on behald of Ziv Izhar.

Differential Revision: https://reviews.llvm.org/D15075


Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/include/clang/Basic/TargetInfo.h
cfe/trunk/lib/Basic/TargetInfo.cpp
cfe/trunk/lib/Basic/Targets.cpp
cfe/trunk/lib/Headers/intrin.h
cfe/trunk/lib/Sema/SemaStmtAsm.cpp
cfe/trunk/test/Sema/asm.c

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=290539=290538=290539=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Mon Dec 26 06:23:42 
2016
@@ -7069,6 +7069,10 @@ let CategoryName = "Inline Assembly Issu
 "constraint '%0' is already present here">;
 }
 
+  def error_inoutput_conflict_with_clobber : Error<
+"asm-specifier for input or output variable conflicts with asm"
+" clobber list">;
+
 let CategoryName = "Semantic Issue" in {
 
 def err_invalid_conversion_between_vectors : Error<

Modified: cfe/trunk/include/clang/Basic/TargetInfo.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/TargetInfo.h?rev=290539=290538=290539=diff
==
--- cfe/trunk/include/clang/Basic/TargetInfo.h (original)
+++ cfe/trunk/include/clang/Basic/TargetInfo.h Mon Dec 26 06:23:42 2016
@@ -605,8 +605,16 @@ public:
 
   /// \brief Returns the "normalized" GCC register name.
   ///
-  /// For example, on x86 it will return "ax" when "eax" is passed in.
-  StringRef getNormalizedGCCRegisterName(StringRef Name) const;
+  /// ReturnCannonical true will return the register name without any additions
+  /// such as "{}" or "%" in it's canonical form, for example:
+  /// ReturnCanonical = true and Name = "rax", will return "ax".
+  StringRef getNormalizedGCCRegisterName(StringRef Name,
+ bool ReturnCanonical = false) const;
+ 
+  virtual StringRef getConstraintRegister(const StringRef ,
+  const StringRef ) const {
+return "";
+  }
 
   struct ConstraintInfo {
 enum {

Modified: cfe/trunk/lib/Basic/TargetInfo.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/TargetInfo.cpp?rev=290539=290538=290539=diff
==
--- cfe/trunk/lib/Basic/TargetInfo.cpp (original)
+++ cfe/trunk/lib/Basic/TargetInfo.cpp Mon Dec 26 06:23:42 2016
@@ -410,8 +410,8 @@ bool TargetInfo::isValidGCCRegisterName(
   return false;
 }
 
-StringRef
-TargetInfo::getNormalizedGCCRegisterName(StringRef Name) const {
+StringRef TargetInfo::getNormalizedGCCRegisterName(StringRef Name,
+   bool ReturnCanonical) const 
{
   assert(isValidGCCRegisterName(Name) && "Invalid register passed in");
 
   // Get rid of any register prefix.
@@ -436,7 +436,7 @@ TargetInfo::getNormalizedGCCRegisterName
   // Make sure the register that the additional name is for is within
   // the bounds of the register names from above.
   if (AN == Name && ARN.RegNum < Names.size())
-return Name;
+return ReturnCanonical ? Names[ARN.RegNum] : Name;
 }
 
   // Now check aliases.

Modified: cfe/trunk/lib/Basic/Targets.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets.cpp?rev=290539=290538=290539=diff
==
--- cfe/trunk/lib/Basic/Targets.cpp (original)
+++ cfe/trunk/lib/Basic/Targets.cpp Mon Dec 26 06:23:42 2016
@@ -2789,6 +2789,40 @@ public:
   const char *getClobbers() const override {
 return "~{dirflag},~{fpsr},~{flags}";
   }
+
+  StringRef getConstraintRegister(const StringRef ,
+  const StringRef ) const override {
+StringRef::iterator I, E;
+for (I = Constraint.begin(), E = Constraint.end(); I != E; ++I) {
+  if (isalpha(*I))
+break;
+}
+if (I == E)
+  return "";
+switch (*I) {
+// For the register constraints, return the matching register name
+case 'a':
+  return "ax";
+case 'b':
+  return "bx";
+