[PATCH] LRA: Revert "Remove useless move insns"

[H.J. Lu]

On Sat, Apr 20, 2019 at 2:54 PM Vladimir Makarov <vmakarov@redhat.com> wrote:
>
>
> On 4/20/19 4:55 AM, Uros Bizjak wrote:
> > On 4/20/19, Vladimir Makarov <vmakarov@redhat.com> wrote:
> >> On 11/21/18 2:33 PM, Uros Bizjak wrote:
> >>> Hello!
> >>>
> >>> Before the recent patch to post-reload mode switching, vzeroupper
> >>> insertion depended on the existence of the return copy instructions
> >>> pair in functions that return a value. The first instruction in the
> >>> pair represents a move to a function return hard register, and the
> >>> second was a USE of the function return hard register. Sometimes a nop
> >>> move was generated (e.g. %eax->%eax) for the first instruction of the
> >>> return copy instructions pair and the patch [1] teached LRA  to remove
> >>> these useless instructions on the fly.
> >>>
> >>> The removal caused optimize mode switching to trigger the assert,
> >>> since the first instruction of a return pair was not found. The
> >>> relevant part of the patch was later reverted. With the recent
> >>> optimize mode switching patch, this is no longer necessary for
> >>> vzeroupper insertion pass, so attached patch reverts the revert.
> >>>
> >>> 2018-11-21  Uros Bizjak  <ubizjak@gmail.com>
> >>>
> >>>       Revert the revert:
> >>>       2013-10-26  Vladimir Makarov  <vmakarov@redhat.com>
> >>>
> >>>       Revert:
> >>>       2013-10-25  Vladimir Makarov  <vmakarov@redhat.com>
> >>>
> >>>       * lra-spills.c (lra_final_code_change): Remove useless move insns.
> >>>
> >>> Patch was bootstrapped and regression tested on x86_64-linux-gnu {,-m32}.
> >>>
> >>> OK for mainline?
> >> Sure, Uros. I support the patch.  But I think it would be wise to
> >> postpone its committing after releasing GCC-9.  Simply it is hard to
> >> predict the patch effect to other targets and I would avoid any risk at
> >> this stage.
> > Actually, the "revert of the revert" patch was already committed to
> > mainline some time ago.
>
> Sorry for confusion, Uros. I did not check the date of your original
> posting.  Insn removal was added to RA just to avoid wasting CPU cycles
> on such insn processing afterwards.  Such insns are removed anyway later
> in the pass pipeline.  The CPU time savings are tiny but the removal
> creates too many problems including new one PR90178.  I should have
> avoided to put this code in the first place.
>
> I think we should remove this code forever. It is not convenient for me
> to do this now because I am traveling.  If somebody wants to remove the
> code, i am approving this in advance.

I am checking in this patch.

Thanks.

>
> > To clear the possible misunderstanding, let me summarise the issue:
> >
> > - the original patch that remove useless move insn caused a breakage
> > in vzeroupper pass.
> > - the original patch was reverted due to the above breakage
> > - the vzeroupper pass was later adjusted to tolerate removed useless
> > move instructions, and this cleared the way to revert the revert. Now
> > LRA removes useless move insns.
> >
> > An orthogonal issue (PR90178) was discovered, showing that some passes
> > also depend on the presence of useless move insn.
> >
> > The bisection stumbled on the "revert of the revert" patch that
> > (obviously) re-introduced the issue. I'm not in the position to decide
> > if useless move insn can be removed or if these later passes should be
> > fixed, I can only say that the vzeroupper pass is now agnostic to the
> > presence of useless move insns.
> >
> > Uros.



-- 
H.J.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-LRA-Revert-Remove-useless-move-insns.patch
Type: application/x-patch
Size: 3865 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20190421/de552fee/attachment.bin>