[RFC][PATCH LRA] WIP patch to fix one part of PR87507

[Peter Bergner]

On 10/22/18 3:58 PM, Jeff Law wrote:
> On 10/19/18 4:39 PM, Peter Bergner wrote:
>> Jeff, maybe once Segher commits his patch, can you give this patch a try
>> on your testers?
> Once committed to the trunk it's automatically picked up :-)  In fact,
> commits to the trunk are triggers, though in reality there's *always*
> something changing from one day to the next in the various relevant
> repos (gcc, binutils, linux kernel, glibc & newlib).
> 
> It's only testing patches that aren't on the trunk that require manual
> intervention.

I meant could you give *my* patch a try once Segher's combine patch hit trunk.
It didn't really make sense to try my patch before his patch hit trunk, since
my patch was catching some illegal insn constraint usage.

That said, here is an updated patch I'd like you to try now that Segher's
patch is on trunk.  It's basically the same patch as before, but using
HARD_REGISTER_P suggested by Segher and another change that catches more
illegal constraint usage, this time matching constraints that don't match,
but where the user has incorrectly forced register usage to different hard
regs.  We used to incorrectly reload one of the hard coded operands and go
on our merry way.  Now we throw an error:

bergner@pike:~/gcc/BUGS/PR87507$ cat ice6.i 
long foo (void)
{
  register long arg asm("r3");
  register long var asm("r4");
  asm ("bla %0 %1" : "=r"(arg) : "0"(var));
  return arg;
}
bergner@pike:~/gcc/BUGS/PR87507$ /home/bergner/gcc/build/gcc-fsf-mainline-pr87507-debug/gcc/xgcc -B/home/bergner/gcc/build/gcc-fsf-mainline-pr87507-debug/gcc -O2 -S ice6.i 
ice6.i: In function ‘foo’:
ice6.i:7:1: error: unable to fixup asm constraints for:
    7 | }
      | ^
(insn 5 2 11 2 (parallel [
            (set (reg/v:DI 3 3 [ arg ])
                (asm_operands:DI ("bla %0 %1") ("=r") 0 [
                        (reg/v:DI 4 4 [ var ])
                    ]
                     [
                        (asm_input:DI ("0") ice6.i:5)
                    ]
                     [] ice6.i:5))
            (clobber (reg:SI 76 ca))
        ]) "ice6.i":5:3 -1
     (expr_list:REG_DEAD (reg/v:DI 4 4 [ var ])
        (expr_list:REG_UNUSED (reg:SI 76 ca)
            (nil))))
during RTL pass: asmcons
ice6.i:7:1: internal compiler error: in match_asm_constraints_1, at function.c:6461


I'm currently bootstrapping and regtesting this on powerpc64le-linux, x86_64-linux
and s390x-linux.  If you could throw it on your testers to test the other arches,
that would be great!

Peter


	* function.c (match_asm_constraints_1): Catch illegal inline asm
	constraint usage.
	* lra-constraints.c (process_alt_operands): Abort on illegal hard
	register usage.  Prefer reloading non hard register operands.

Index: gcc/function.c
===================================================================
--- gcc/function.c	(revision 265399)
+++ gcc/function.c	(working copy)
@@ -6453,6 +6453,13 @@ match_asm_constraints_1 (rtx_insn *insn,
 	  || !general_operand (input, GET_MODE (output)))
 	continue;
 
+      /* If we have a matching constraint and both operands are hard registers,
+	 then they must be the same hard register.  */
+      if (HARD_REGISTER_P (output)
+	  && HARD_REGISTER_P (input)
+	  && REGNO (output) != REGNO (input))
+	fatal_insn ("unable to fixup asm constraints for:", insn);
+
       /* We can't do anything if the output is also used as input,
 	 as we're going to overwrite it.  */
       for (j = 0; j < ninputs; j++)
Index: gcc/lra-constraints.c
===================================================================
--- gcc/lra-constraints.c	(revision 265399)
+++ gcc/lra-constraints.c	(working copy)
@@ -2146,9 +2146,15 @@ process_alt_operands (int only_alternati
 		      }
 		    else
 		      {
-			/* Operands don't match.  Both operands must
-			   allow a reload register, otherwise we
-			   cannot make them match.  */
+			/* Operands don't match.  Make sure the two operands
+			   are not two different explicit hard registers.  */
+			if (HARD_REGISTER_P (*curr_id->operand_loc[nop])
+			    && HARD_REGISTER_P (*curr_id->operand_loc[m]))
+			  fatal_insn ("unable to generate reloads for:",
+				      curr_insn);
+
+			/* Both operands must allow a reload register,
+			   otherwise we cannot make them match.  */
 			if (curr_alt[m] == NO_REGS)
 			  break;
 			/* Retroactively mark the operand we had to
@@ -2910,18 +2916,28 @@ process_alt_operands (int only_alternati
 		if (first_conflict_j < 0)
 		  first_conflict_j = j;
 		last_conflict_j = j;
+		/* Both the earlyclobber operand and conflicting operand
+		   cannot both be hard registers.  */
+		if (HARD_REGISTER_P (operand_reg[i])
+		    && operand_reg[j] != NULL_RTX
+		    && HARD_REGISTER_P (operand_reg[j]))
+		  fatal_insn ("unable to generate reloads for:", curr_insn);
 	      }
 	  if (last_conflict_j < 0)
 	    continue;
-	  /* If earlyclobber operand conflicts with another
-	     non-matching operand which is actually the same register
-	     as the earlyclobber operand, it is better to reload the
-	     another operand as an operand matching the earlyclobber
-	     operand can be also the same.  */
-	  if (first_conflict_j == last_conflict_j
-	      && operand_reg[last_conflict_j] != NULL_RTX
-	      && ! curr_alt_match_win[last_conflict_j]
-	      && REGNO (operand_reg[i]) == REGNO (operand_reg[last_conflict_j]))
+
+	  /* If an earlyclobber operand conflicts with another non-matching
+	     operand (ie, they have been assigned the same hard register),
+	     then it is better to reload the other operand, as there may
+	     exist yet another operand with a matching constraint associated
+	     with the earlyclobber operand.  However, if one of the operands
+	     is an explicit use of a hard register, then we must reload the
+	     other non-hard register operand.  */
+	  if (HARD_REGISTER_P (operand_reg[i])
+	      || (first_conflict_j == last_conflict_j
+		  && operand_reg[last_conflict_j] != NULL_RTX
+		  && !curr_alt_match_win[last_conflict_j]
+		  && !HARD_REGISTER_P (operand_reg[last_conflict_j])))
 	    {
 	      curr_alt_win[last_conflict_j] = false;
 	      curr_alt_dont_inherit_ops[curr_alt_dont_inherit_ops_num++]