[PATCH] D32563: Add LiveRangeShrink pass to shrink live range within BB.

[Xinliang David Li via llvm-commits]

Revert and iterate sounds fine to me unless it is a small quick fix.

David

On Wed, May 17, 2017 at 10:56 AM, Chandler Carruth <chandlerc at google.com>
wrote:

> If there were issues raised in review and not addressed, it should
> probably be reverted until they're addressed.
>
> On Wed, May 17, 2017 at 11:52 AM Matthias Braun via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
>
>> Hi Dehao,
>>
>> can you please address these issues?
>>
>> I think this should at least:
>> - Not consider PHIs as block-local definitions and move things towards
>> them
>> - Be conservative and stay away from cases involving different register
>> classes so we do not constrain regalloc
>> - Could this be moved to TargetPassConfig::addPreRegAlloc() or similar
>> so that targets can override and opt-out of using this pass?
>>
>> - Matthias
>>
>> > On May 17, 2017, at 10:40 AM, via llvm-commits <
>> llvm-commits at lists.llvm.org> wrote:
>> >
>> > This patch caused some fairly significant regressions on our GPU
>> target. While I can’t guarantee this is all of the problems, here is a
>> particular case that exemplified to me “things this pass is doing that it
>> should not”.
>> >
>> >
>> > Original code:
>> >
>> > PHI
>> > PHI
>> > PHI
>> > […]
>> > foo = REG_SEQUENCE […] // inputs come from the phi
>> > STORE foo
>> > bar = REG_SEQUENCE […]
>> > STORE bar
>> > foobar = REG_SEQUENCE […]
>> > STORE foobar
>> > […]
>> >
>> > New code:
>> >
>> > PHI
>> > PHI
>> > PHI
>> > […]
>> > foo = REG_SEQUENCE […]
>> > bar = REG_SEQUENCE […]
>> > foobar = REG_SEQUENCE […]
>> > […]
>> > STORE bar
>> > STORE foo
>> > STORE foobar
>> >
>> > This causes a vast increase in register count because the reg_sequences
>> constrain the registers to be adjacent far longer than necessary.
>> >
>> > Two things are going on here that seem dubious.
>> >
>> > 1. why are we moving them up… to a PHI??? does this make sense for any
>> instruction?
>> > 2. why are we moving reg_sequences??? away from their use, at that?
>> >
>> > I can’t imagine this makes sense on any target.
>> >
>> > —escha
>> > _______________________________________________
>> > llvm-commits mailing list
>> > llvm-commits at lists.llvm.org
>> > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170517/bf3eb9d8/attachment.html>