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

[Quentin Colombet via llvm-commits]

Hi Dehao,

> On May 17, 2017, at 10:51 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?

Quick question, are we expecting this to be beneficial for a lot of targets?
If not, I would suggest to add the pass in the related target specific TargetConfig instead of the generic one.

Regarding the REG_SEQUENCE issue, I am guessing (I haven’t looked at the code) that the algorithm blindly moves thing up when it reduces the number of live-ranges (i.e., more killed args than defs) and that’s obviously wrong if we don’t take into account the width of the definition. What I am saying is regardless of the way we consider PHIs (I don’t think that’s the problem here), we shouldn’t move REG_SEQUENCE up and more generally we shouldn’t move up anything that involves pushing things into a larger register like INSERT_SUBREG.

Cheers,
-Quentin

> 
> - 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