[PING**2] [PATCH] Fix asm X constraint (PR inline-asm/59155)

[Bernd Edlinger <bernd dot edlinger at hotmail dot de>]

Hi,

ping...

As this discussion did not make any progress, I just attached
the latest version of my patch with the the changes that
Vladimir proposed.

Boot-strapped and reg-tested again on x86_64-linux-gnu.
Is it OK for the trunk?


Thanks
Bernd.


On 06/10/16 16:13, Bernd Edlinger wrote:
> On 06/09/16 18:45, Jakub Jelinek wrote:
>> On Thu, Jun 09, 2016 at 06:43:04PM +0200, Jakub Jelinek wrote:
>>> Yes, I'm all in favor in disabling X constraint for inline asm.
>>> Especially if people actually try to print it as well, rather than
>>> make it
>>> unused.  That is a sure path to ICEs.
>>
>> Though, on the other side, even our documentation mentions
>> asm volatile ("mtfsf 255,%1" : "=X"(sum): "f"(fpenv));
>> So perhaps we need to error just in case such an argument is printed?
>
> note that "=X" is also introduced internally in this asm statement:
>
> asm ("cmpl  %2, 0" : "=@ccz"(z), "=@ccb"(b): "r"(i));
>
> see i386.c, ix86_md_asm_adjust.
>
> The first =@cc is replaced by "=Bf" constraint but any
> further =@cc are replaced by "=X" and scratch operand.
>
> Printing the "=X" scratch is harmless, but printing the "=Bf" causes
> another ICE, I shall submit a follow up patch for that:
> asm ("# %0" : "=@ccz"(z));
>
> test.c:6:1: internal compiler error: in print_reg, at
> config/i386/i386.c:17193
>   }
>   ^
> 0xedfc30 print_reg(rtx_def*, int, _IO_FILE*)
>      ../../gcc-trunk/gcc/config/i386/i386.c:17189
> 0xf048a4 ix86_print_operand(_IO_FILE*, rtx_def*, int)
>      ../../gcc-trunk/gcc/config/i386/i386.c:17867
> 0x8bf87c output_operand(rtx_def*, int)
>      ../../gcc-trunk/gcc/final.c:3847
> 0x8c00ee output_asm_insn(char const*, rtx_def**)
>      ../../gcc-trunk/gcc/final.c:3763
> 0x8c1f9c final_scan_insn(rtx_insn*, _IO_FILE*, int, int, int*)
>      ../../gcc-trunk/gcc/final.c:2628
> 0x8c25c9 final(rtx_insn*, _IO_FILE*, int)
>      ../../gcc-trunk/gcc/final.c:2045
> 0x8c2da9 rest_of_handle_final
>      ../../gcc-trunk/gcc/final.c:4445
> 0x8c2da9 execute
>      ../../gcc-trunk/gcc/final.c:4520
>
>
> Well, regarding the X constraint, I do think that
> it's definitely OK to use different rules if it is
> used in asms vs. when if it is used internally in .md files.
>
> The patch handles X in asms to be just a synonym to the g constraint,
> except that g allows only GENERAL_REGS and X allows ALL_REGS.
>
> What I am not sure of, is if X should allow more than g in terms of
> CONSTANT_P.  I think it should not, because probably the CONSTANT_P
> handling in general_operand is likely smarter than that of the i
> constraint.
>
>
> Bernd.

Attachment: changelog-pr59155.txt
Description: changelog-pr59155.txt

Index: gcc/lra-constraints.c
===================================================================
--- gcc/lra-constraints.c	(revision 237133)
+++ gcc/lra-constraints.c	(working copy)
@@ -1854,8 +1854,9 @@ process_alt_operands (int only_alternative)
 	  if (curr_static_id->operand_alternative[opalt_num].anything_ok)
 	    {
 	      /* Fast track for no constraints at all.	*/
-	      curr_alt[nop] = NO_REGS;
-	      CLEAR_HARD_REG_SET (curr_alt_set[nop]);
+	      curr_alt[nop] = ALL_REGS;
+	      COPY_HARD_REG_SET (curr_alt_set[nop],
+				 reg_class_contents[ALL_REGS]);
 	      curr_alt_win[nop] = true;
 	      curr_alt_match_win[nop] = false;
 	      curr_alt_offmemok[nop] = false;
Index: gcc/recog.c
===================================================================
--- gcc/recog.c	(revision 237133)
+++ gcc/recog.c	(working copy)
@@ -1778,6 +1778,10 @@ asm_operand_ok (rtx op, const char *constraint, co
 	    result = 1;
 	  break;
 
+	case 'X':
+	  if (scratch_operand (op, VOIDmode))
+	    result = 1;
+	  /* ... fall through ...  */
 	case 'g':
 	  if (general_operand (op, VOIDmode))
 	    result = 1;
Index: gcc/testsuite/gcc.dg/torture/pr59155-1.c
===================================================================
--- gcc/testsuite/gcc.dg/torture/pr59155-1.c	(revision 0)
+++ gcc/testsuite/gcc.dg/torture/pr59155-1.c	(working copy)
@@ -0,0 +1,8 @@
+/* { dg-do compile } */
+double f(double x){
+  asm volatile("":"+X"(x));
+  return x;
+}
+double g(double x,double y){
+  return f(f(x)+f(y));
+}
Index: gcc/testsuite/gcc.dg/torture/pr59155-2.c
===================================================================
--- gcc/testsuite/gcc.dg/torture/pr59155-2.c	(revision 0)
+++ gcc/testsuite/gcc.dg/torture/pr59155-2.c	(working copy)
@@ -0,0 +1,8 @@
+/* { dg-do compile } */
+double f(double x){
+  asm volatile("":"+X"(x));
+  return x;
+}
+double g(){
+  return f(1.);
+}
Index: gcc/testsuite/gcc.dg/torture/pr59155-3.c
===================================================================
--- gcc/testsuite/gcc.dg/torture/pr59155-3.c	(revision 0)
+++ gcc/testsuite/gcc.dg/torture/pr59155-3.c	(working copy)
@@ -0,0 +1,27 @@
+void
+noprop1 (int **x, int y, int z)
+{
+  int *ptr = *x + y * z / 11;
+  asm volatile ("noprop1 %0" : : "X" (*ptr));
+}
+
+void
+noprop2 (int **x, int y, int z)
+{
+  int *ptr = *x + y * z / 11;
+  asm volatile ("noprop2 %0" : : "X" (ptr));
+}
+
+int *global_var;
+
+void
+const1 (void)
+{
+  asm volatile ("const1 %0" : : "X" (global_var));
+}
+
+void
+const2 (void)
+{
+  asm volatile ("const2 %0" : : "X" (*global_var));
+}