| Message ID | 8760rqi5d1.fsf@atmel.com |
|---|---|
| State | New |
| Headers | show |
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
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);
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