This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [lno] PATCH rs6000.md fix
On Tue, Mar 30, 2004 at 10:36:46AM -0500, David Edelsohn wrote:
> >>>>> Mostafa Hagog writes:
> Mostafa> * config/rs6000/rs6000.md ("*ctrsi_internal1", "*ctrsi_internal2",
> Mostafa> "*ctrdi_internal1", "*ctrdi_internal2", "*ctrsi_internal3",
> Mostafa> "*ctrsi_internal4", "*ctrdi_internal3", "*ctrdi_internal4",
> Mostafa> "*ctrsi_internal5", "*ctrsi_internal6", "*ctrdi_internal5",
> Mostafa> "*ctrdi_internal6"): Replace register_operand with
> Mostafa> nonimmediate_operand
>
> Yes. I looked into this, but I preferred to see a testcase that
> failed. I will commit this to mainline.
Seeing the ctr{si,di}_* patterns mentioned reminded me of a patch I
submitted a long time ago. I reckon the GE patterns, ie.
ctrsi_internal3, ctrsi_internal4, ctrdi_internal3 and ctrdi_internal4
are all invalid. Consider that dbnz does:
decrement ctr by one.
branch if resulting ctr != 0
This operation is accurately reflected in the rtl for the NE and EQ
patterns, eg.
(define_insn "*ctrsi_internal1"
[(set (pc)
(if_then_else (ne (match_operand:SI 1 "register_operand" "c,*r,*r,*r")
(const_int 1))
(label_ref (match_operand 0 "" ""))
(pc)))
(set (match_operand:SI 2 "register_operand" "=1,*r,m,*q*c*l")
(plus:SI (match_dup 1)
(const_int -1)))
[snip]
However the GE patterns use
(define_insn "*ctrsi_internal3"
[(set (pc)
(if_then_else (ge (match_operand:SI 1 "register_operand" "c,*r,*r,*r")
(const_int 0))
(label_ref (match_operand 0 "" ""))
(pc)))
[snip]
Yet both patterns emit the same bdnz instructions! reg >= 0 is not even
closely equivalent to reg != 1. The correct condition is reg >= 2, but
then you need to guarantee that the initial value of reg is greater or
equal to 2 rather than the currently checked initial value >= 0 via a
REG_NONNEG note.
May I commit a patch to mainline that removes the above mentioned
patterns?
--
Alan Modra
IBM OzLabs - Linux Technology Centre