[RFA is for the regcprop bits, reverting the mips.md hack seems like a no-brainer with the regcprop change. ]


Per the recent discussion between Richard S. and myself, this is a better fix for the mips16 codegen issue where it's creating invalid lwu insns.

As Richard S. pointed out there's been a long standing problem where regcprop would create a reference to the stack pointer that was unique from stack_pointer_rtx. That's the root cause of the codegen issue.

We can't re-use stack_pointer_rtx in the code in question because we're going to modify the underlying RTX in fun and interesting ways. Ports (such as the mips) assume references to the stack pointer are unique (ie, they can identify a stack pointer reference by stack_pointer_rtx rather than checking register #s).

So this patch just rejects propagation of the stack pointer. It's conservative in that it doesn't reject other special registers.

An alternate approach would be to declare that ports can not depend on looking at stack_pointer_rtx to find all stack references that instead they have to look at the underlying regno. The amount of auditing here would be significant.

I've bootstrapped and regression tested this on x86_64-linux-gnu. It's also built libgcc/glibc/newlib on about 100 different targets.

OK for the trunk?

jeff
diff --git a/gcc/config/mips/mips.md b/gcc/config/mips/mips.md
2017-04-17  Jeff Law  <l...@redhat.com>

        * regcprop.c (maybe_mode_change): Avoid creating copies of the
        stack pointer.

        Revert:
        2017-04-13  Jeff Law  <l...@redhat.com>
        * config/mips.mips.md (zero_extendsidi2): Do not allow SP to appear
        in operands[1] if it is a MEM and TARGET_MIPS16 is active.

index dd5e1e7..7acf00d 100644
--- a/gcc/config/mips/mips.md
+++ b/gcc/config/mips/mips.md
@@ -3493,10 +3493,7 @@
 (define_insn_and_split "*zero_extendsidi2"
   [(set (match_operand:DI 0 "register_operand" "=d,d")
         (zero_extend:DI (match_operand:SI 1 "nonimmediate_operand" "d,W")))]
-  "TARGET_64BIT && !ISA_HAS_EXT_INS
-   && !(TARGET_MIPS16
-        && MEM_P (operands[1])
-        && reg_mentioned_p (stack_pointer_rtx, operands[1]))"
+  "TARGET_64BIT && !ISA_HAS_EXT_INS"
   "@
    #
    lwu\t%0,%1"
@@ -3512,10 +3509,7 @@
 (define_insn "*zero_extendsidi2_dext"
   [(set (match_operand:DI 0 "register_operand" "=d,d")
         (zero_extend:DI (match_operand:SI 1 "nonimmediate_operand" "d,W")))]
-  "TARGET_64BIT && ISA_HAS_EXT_INS
-   && !(TARGET_MIPS16
-        && MEM_P (operands[1])
-        && reg_mentioned_p (stack_pointer_rtx, operands[1]))"
+  "TARGET_64BIT && ISA_HAS_EXT_INS"
   "@
    dext\t%0,%1,0,32
    lwu\t%0,%1"
diff --git a/gcc/regcprop.c b/gcc/regcprop.c
index ddc6252..367d85a 100644
--- a/gcc/regcprop.c
+++ b/gcc/regcprop.c
@@ -396,6 +396,13 @@ maybe_mode_change (machine_mode orig_mode, machine_mode 
copy_mode,
       && GET_MODE_SIZE (copy_mode) < GET_MODE_SIZE (new_mode))
     return NULL_RTX;
 
+  /* Avoid creating multiple copies of the stack pointer.  Some ports
+     assume there is one and only one stack pointer.
+
+     It's unclear if we need to do the same for other special registers.  */
+  if (regno == STACK_POINTER_REGNUM)
+    return NULL_RTX;
+
   if (orig_mode == new_mode)
     return gen_raw_REG (new_mode, regno);
   else if (mode_change_ok (orig_mode, new_mode, regno))

Reply via email to