[committed] Fix xstormy16 fallout from Jakub's cselib changes

Message ID b1995f26ebb88ac61ea07efa6387dd92d670d993.camel@redhat.com
State New
Headers show
  • [committed] Fix xstormy16 fallout from Jakub's cselib changes
Related show

Commit Message

Jonathan Wakely via Gcc-patches April 3, 2020, 11:48 p.m.
This fixes more fallout from Jakub's cselib changes.  This shows up as an ICE
building stdarg-3 from the testsuite after Jakub's changes.  However, a reduced
testcase will fail miserably all the way back to gcc-7 (as far back as I tested).

The xstormy port only allows a subset of the registers to be used in memory
operands.  Additionally it allows auto-increment addressing modes.  The
combination of the two creates an interesting problem with reloading.

So you might have something like

(set (pseudo1) (mem (post_inc (pseudo2)))

Now assume that pseudo1 gets a hard reg, a nice convenient one like r2, but sadly
pseudo2 gets no hard reg and is on the stack.  You end up with something like
this during reload

(set (reg 2) (mem (post_inc (mem (plus (sp) (offset))))))

Uh-oh.  Reload knows how to deal with this most of the time :-) Assume that we
get r10 as our reload register.  Reload will generate something like this:

(set (reg 10) (mem (plus (sp) (offset))))    // Load pseudo2 from stack
(set (reg 10) (plus (reg 10) (const_int 2))) // autoinc
(set (mem (plus (sp) (offset)) (reg10))	     // Store autoinc'd value back
(set (reg 10) (plus (reg 10) (const_int -2)))// Undo autoinc
(set (reg 2) (mem (reg 10)))                 // And set the actual output

But wait, I mentioned the port limits what registers can be used in memory
references.  The code above isn't valid because r10 can't be the destination of a
load from memory or the source of a store to memory and we'll ICE.

It took a while to get my bearings.  I haven't had to think about reload in a
long time and after several false starts I realized the problem was the reload
register selection, not secondary reloads.  Get the right kind of reload register
and the right things will just happen.

The preferred_reload_class does get called on the (post_inc ...) mess via
find_reloads_address_1.  ie, given:

(insn 34 171 35 2 (set (reg:HI 2 r2 [orig:37 a3+2 ] [37])
        (mem/c:HI (post_inc:HI (reg/f:HI 89)) [1 a3+2 S2 A16])) "testcase.c":8:6
6 {movhi_internal}
     (expr_list:REG_INC (reg/f:HI 89)

We'll call preferred_reload_class with the post_inc expression where the pseudo
has been replaced with its equivalent memory:

(post_inc:HI (mem/c:HI (plus:HI (reg/f:HI 15 sp)
            (const_int -34 [0xffffffffffffffde])) [3 %sfp+44 S2 A16]))

So the fix here is to realize that reloading an address of that from must be done
with one of the registers that is suitable for memory operands.

I expected this to fix several other long standing ICEs on the port which
(without digging) seemed like they were likely related.  Sadly, it just fixed the

Anyway, I'll be committing this to the trunk momentarily.

commit 7f26e60c2600f0a676fc9370390ebf0a3c78f31c
Author: Jeff Law <law@redhat.com>
Date:   Fri Apr 3 17:47:18 2020 -0600

    Fix stdarg-3 regression on xstormy16 port
            PR rtl-optimization/92264
            * config/stormy16/stormy16.c (xstormy16_preferred_reload_class): Handle
            reloading of auto-increment addressing modes.


diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 6317e385cac..25d1c5d1c38 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,9 @@ 
+2020-04-03  Jeff Law  <law@redhat.com>
+	PR rtl-optimization/92264
+	* config/stormy16/stormy16.c (xstormy16_preferred_reload_class): Handle
+	reloading of auto-increment addressing modes.
 2020-04-03  H.J. Lu  <hongjiu.lu@intel.com>
 	PR target/94467
diff --git a/gcc/config/stormy16/stormy16.c b/gcc/config/stormy16/stormy16.c
index 55fe82954c1..2141531e262 100644
--- a/gcc/config/stormy16/stormy16.c
+++ b/gcc/config/stormy16/stormy16.c
@@ -497,7 +497,17 @@  xstormy16_secondary_reload_class (enum reg_class rclass,
 static reg_class_t
 xstormy16_preferred_reload_class (rtx x, reg_class_t rclass)
-  if (rclass == GENERAL_REGS && MEM_P (x))
+  /* Only the first eight registers can be moved to/from memory.
+     So those prefer EIGHT_REGS.
+     Similarly reloading an auto-increment address is going to
+     require loads and stores, so we must use EIGHT_REGS for those
+     too.  */
+  if (rclass == GENERAL_REGS
+      && (MEM_P (x)
+	  || GET_CODE (x) == POST_INC
+	  || GET_CODE (x) == PRE_DEC
+	  || GET_CODE (x) == PRE_MODIFY))
     return EIGHT_REGS;
   return rclass;