https://gcc.gnu.org/bugzilla/show_bug.cgi?id=55212

--- Comment #346 from Oleg Endo <olegendo at gcc dot gnu.org> ---
(In reply to Kazumoto Kojima from comment #343)
> 
> Yes. The wrong instruction
> 
>       mov.b   @(5,fpul),r0    ! 517   [c=2 l=2]  *movqi/8
> 
> is generated with *movqi insn which is defined by
> 
> (define_insn "*mov<mode>"
>   [(set (match_operand:QIHI 0 "general_movdst_operand"
>                             "=r,r,r,Sid,^zr,Ssd,r,  Sdd,z,  r,l")
>       (match_operand:QIHI 1 "general_movsrc_operand"
>                              "Q,r,i,^zr,Sid,r,  Ssd,z,  Sdd,l,r"))]
>   "TARGET_SH1
>    && (arith_reg_operand (operands[0], <MODE>mode)
>        || arith_reg_operand (operands[1], <MODE>mode))"
>  
> and the 8-th alternative is an Sdd memory load. 
> general_mov{dst,src}_operand predicates check the memory displacement
> expression.  They only check the address register with REG_P, though.

Right ... all the memory constraints have not been checking for the actual
registers.  Perhaps with LRA the address modifications are going through a
different validation path?  Anyway ... 

In your attachment 59219:

>  && (GENERAL_REGISTER_P (REGNO (XEXP (x, 0)))
>      || REGNO (XEXP (x, 0)) >= FIRST_PSEUDO_REGISTER)

... I've noticed that this is the same as the existing
MAYBE_BASE_REGISTER_RTX_P.

I've inserted a patch into the stash to tighten all the existing memory
predicates and constraints to use MAYBE_BASE_REGISTER_RTX_P and
MAYBE_INDEX_REGISTER_RTX_P.  As a result your added check in
'general_mov{dst,src}_operand' becomes unnecessary, but I've left it in anyway.

This is the patch
https://github.com/olegendo/gcc/commit/9636a55a071fd9b9acc3a38fb25799fc70177d28

I think this kind of thing could be already upstreamed after reg-testing
without LRA, it's a good catch.

I've also tried to wade through the movsf_ie related patterns a bit, without
any meaningful results though.  The need to add a clobber-R0 pattern variants
looks very strange.  Looks like some missed corner case in LRA itself?  At
least I remember, when I tried to do something like that with old reload, it
never really worked in fixing the R0 spill failures.

Anyway, it seems after tightening the memory predicates and constraints, some
of the previously added things become redundant.  See follow up patch

https://github.com/olegendo/gcc/commit/015db793f1b6a2abd25dd6b1205d6cb280139b61


I've tested the branch https://github.com/olegendo/gcc/commits/devel/sh-lra/ by
building sh-elf compiler and target libs (newlib, libstdc++) and compiling the
last two webkit test cases.

Reply via email to