[tentative,reload] Make push_reload work with more types of subregs?

Message ID 8760rqi5d1.fsf@atmel.com
State New
Headers show

Commit Message

Senthil Kumar Selvaraj July 28, 2016, 7:33 a.m.
Hi,

  When analyzing PR 71873 (ICE in push_reload), I found that that code
  in push_reload that recursively calls push_reload for subreg
  expressions doesn't correctly set subreg_in_class for a few cases.

  Specifically, reload_inner_reg_of_subreg returns true if SUBREG_REG(x)
  is CONSTANT_P or if it's code is PLUS. The code that tries to find a
  valid class (before recursively calling push_reload), however, only
  does that if SUBREG_REG is REG_P or if it's a SYMBOL_REF. For the
  other cases, subreg_in_class is set to the default NO_REGS, and this
  triggers the rclass != NO_REGS assert just before find_reusable_reload.

  For PR 71873, reload sees

  (set (reg/f:HI 87)
        (const:HI (plus:HI (symbol_ref:HI ("a")  <var_decl 0x7ff218a11bd0 a>)  
                (const_int 1 [0x1])))) ../test.c:24 83 {*movhi}
     (expr_list:REG_EQUIV (const:HI (plus:HI (symbol_ref:HI ("a")  <var_decl 0x7ff218a11bd0 a>) 
                (const_int 1 [0x1])))
  and

  (set (mem:QI (post_dec:HI (reg/f:HI 32 __SP_L__)) [0  S1 A8]) 
        (subreg:QI (reg/f:HI 87) 1))

  and decides to replace pseudo reg 87 in the latter insn with the
  REG_EQUIV it found in the former. The resulting RTL expression
   
  (subreg:QI (const:HI (plus:HI (symbol_ref:HI ("a") <var_decl 0x7ffff7ff5bd0 a>)
            (const_int 1 [0x1]))) 1)

   does not match any of the conditions that handle subregs because
   subreg_low_part is false and the inner expr is not a REG or a SYMBOL_REF.

  Is there a reason why only REG and SYMBOL_REFs get valid
  subreg_in_class? I tried extending it handle constants and PLUS
  expressions, and it fixes PR 71873. It also fixes a another
  bug that was a work around for the reload failure (PR 64452) - that
  had a plus expression instead of the const.

  Reg testing on avr and x86_64 did not show any new failures. Is this
  the right way to fix this?

Regards
Senthil

Comments

Bernd Schmidt July 29, 2016, 11:28 a.m. | #1
On 07/28/2016 09:33 AM, Senthil Kumar Selvaraj wrote:
>
>   Is there a reason why only REG and SYMBOL_REFs get valid
>   subreg_in_class? I tried extending it handle constants and PLUS
>   expressions, and it fixes PR 71873. It also fixes a another
>   bug that was a work around for the reload failure (PR 64452) - that
>   had a plus expression instead of the const.
>
>   Reg testing on avr and x86_64 did not show any new failures. Is this
>   the right way to fix this?

I think it looks quite plausible. Note that testing x86_64 on trunk will 
not do anything - it is no longer using reload. Could you go back to an 
older branch (4.7 I think is the last one using reload) and retest 
x86_64 with that, for better test coverage?


Bernd

Patch

diff --git gcc/reload.c gcc/reload.c
index 06426d9..f80d849 100644
--- gcc/reload.c
+++ gcc/reload.c
@@ -1141,7 +1141,9 @@  push_reload (rtx in, rtx out, rtx *inloc, rtx *outloc,
                                                   SUBREG_BYTE (in),
                                                   GET_MODE (in)),
                              REGNO (SUBREG_REG (in)));
-      else if (GET_CODE (SUBREG_REG (in)) == SYMBOL_REF)
+      else if (GET_CODE (SUBREG_REG (in)) == SYMBOL_REF
+               || CONSTANT_P (SUBREG_REG (in))
+               || GET_CODE (SUBREG_REG (in)) == PLUS)
        subreg_in_class = find_valid_class_1 (inmode,
                                              GET_MODE (SUBREG_REG (in)),
                                              rclass);