Re: IRA: matches insn even though !reload_in_progress

[Georg-Johann Lay <avr at gjlay dot de>]

Sameera Deshpande wrote:

Please answer to the lists, thanks.

> Hi Johann,
> 
> Try using clobber (match_operand:QI 3 "register_operand" "")
> 
> gen_reg_rtx may not work in this case. Please have a look at
> define_split description in GCC the internal documents. It says: "The
> preparation-statements are similar to those statements that are
> specified for define_expand (see Expander Definitions) and are
> executed before the new RTL is generated to prepare for the generated
> code or emit some insns whose pattern is not fixed. Unlike those in
> define_expand, however, these statements must not generate any new
> pseudo-registers. Once reload has completed, they also must not
> allocate any space in the stack frame."

This is to be a pre-reload split as outlined by Michael Meissner in
"Upgrading your GCC port to use modern features" from GCC 2009 Summit.

gen_reg_rtx is ok in split1 which runs pre-reload and even with -O0.
All works fine except that IRA/reload does not respect the
   !reload_in_progress && !reload_competed
insn condition.

Johann

> Hope this helps,
> 
> - Thanks and regards,
>   Sameera D
> 
> 
> **-----Original Message-----
> **From: gcc-owner@gcc.gnu.org [mailto:gcc-owner@gcc.gnu.org] On Behalf Of
> **Georg-Johann Lay
> **Sent: 11 July 2011 12:28
> **To: gcc@gcc.gnu.org
> **Subject: Re: IRA: matches insn even though !reload_in_progress
> **
> **Georg-Johann Lay wrote:
> **> Georg-Johann Lay wrote:
> **>> The following pattern shall be generated by insn combine
> **>> and then be split by pre-reload split:
> **>>
> **>> (define_insn_and_split "*mulsqihi3.const"
> **>>   [(set (match_operand:HI 0 "register_operand"
> **>>  "=&r")
> **>>         (mult:HI (sign_extend:HI (match_operand:QI 1
> **>> "register_operand" "a"))
> **>>                  (match_operand:HI 2 "u8_operand"
> **>>   "n")))]
> **>>   "AVR_HAVE_MUL
> **>>    && !reload_completed
> **>>    && !reload_in_progress"
> **>>   { gcc_unreachable(); }
> **>>   "&& 1"
> **>>   [(set (match_dup 3)
> **>>         (match_dup 2))
> **>>    ; *mulsu
> **>>    (mult:HI (sign_extend:HI (match_dup 1))
> **>>             (zero_extend:HI (match_dup 3)))]
         ^^^^^^^^^^^
Bad typo, of course, but fixing it does not help, of course.

> **>>   {
> **>>     operands[3] = gen_reg_rtx (QImode);
> **>>   })
> **>>
> **>> All works, and in .asmcons insns look like that:
> **>>
> **>> (insn 7 6 8 2 (set (reg:HI 48)
> **>>         (const_int 155 [0x9b])) wmul.c:29 10 {*movhi}
> **>>      (nil))
> **>>
> **>> (insn 8 7 21 2 (set (reg:HI 46)
> **>>         (mult:HI (sign_extend:HI (reg:QI 24 r24 [ x ]))
> **>>             (reg:HI 48))) wmul.c:29 38 {*mulsqihi3}
> **>>      (expr_list:REG_DEAD (reg:HI 48)
> **>>         (expr_list:REG_DEAD (reg:QI 24 r24 [ x ])
> **>>             (nil))))
> **>>
> **>> IRA now propagates insn 7 into insn 8 so that in insn-output gcc
> **runs
> **>> into the gcc_unreachable() even though !reload_in_progress etc
> **should
> **>> keep IRA/reload from generating the insn.
> **>>
> **>> After IRA/reload the code is:
> **>>
> **>> (insn 8 6 21 2 (set (reg:HI 24 r24 [46])
> **>>         (mult:HI (sign_extend:HI (reg:QI 24 r24 [ x ]))
> **>>             (const_int 155 [0x9b]))) wmul.c:29 39 {*mulsqihi3.const}
> **>>      (nil))
> **>>
> **>> which of course must crash.
> **>>
> **>> How do I write a pre-reload combine + pre-reload split correctly?
> **>> I'd like to avoid clobber reg.
> **>
> **> This solution with (clobber (scratch:QI)) appears to work.
> **> (clobber (match_scratch:QI 3 "=&d")) does not.
> **>
> **> (define_insn_and_split "*mulsqihi3.const"
> **>   [(set (match_operand:HI 0 "register_operand" "=&r")
> **>         (mult:HI (sign_extend:HI (match_operand:QI 1
> **> "register_operand" "a"))
> **>                  (match_operand:HI 2 "u8_operand"
> **>   "n")))
> **>    (clobber (scratch:QI))]
> **>   "AVR_HAVE_MUL
> **>    && !reload_completed
> **>    && !reload_in_progress"
> **>   { gcc_unreachable(); }
> **>   "&& 1"
> **>   [(set (match_dup 3)
> **>         (match_dup 2))
> **>    ; *mulsu
> **>    (mult:HI (sign_extend:HI (match_dup 1))
> **>             (zero_extend:HI (match_dup 3)))]
> **>   {
> **>     if (SCRATCH == GET_CODE (operands[3]))
> **>       operands[3] = gen_reg_rtx (QImode);
> **>   })
> **>
> **> Is that the correct approach?
> **
> **The answer it "no" because IRA does not synthesize the insn, but
> **reload fails to synthesize it, too.
> **
> **So that pattern is just dead code :-(
> **
> **Johann
> **