[llvm] r241027 - RegisterCoalescer: Cleanup empty subranges after shrinkToUses()

[Quentin Colombet]

> On Jul 1, 2015, at 3:36 PM, Matthias Braun <matze at braunis.de> wrote:
> 
> 
>> On Jul 1, 2015, at 3:12 PM, Quentin Colombet <qcolombet at apple.com> wrote:
>> 
>> Hi Matthias,
>> 
>> Is there a way we can do that as part of shrinkToUses?
>> My concern is that future users of that API may have similar problems and would need a similar fix.
> The problem is that these shrinkToUses variants work on a single SubRange, but in the context of a subrange you can't just remove yourself from the list (or you'll very likely invalidate the iterators of people calling the function).

That’s right. I was thinking that we could return a boolean in that case, but that would be inconsistent with the meaning of the boolean for the non-specialized version of shrinkToUses.

I am wondering if we should return via an output parameter (like the dead definition) the empty subrange.

Anyway, we could add a comment on the method to point out that it might generate empty subranges and that those must be cleaned up.

> I can change some verifiers/check functions to catch those cases earlier though.

A check in the verifier would be nice indeed!

> It's probably also possible to loosen the requirements and allow empty subranges but that needs a careful code review.
> 
>> 
>> Also, could you check that we generate something meaningful, not just don’t crash (I know we already have this discussion, I still don’t like test case with FileCheck :)).
> I'm pretty sure you meant "without" here :)

Indeed :).

> I've added some FileCheck there now, but I can't help you with the "meaningful" part because I don't know enough about AMDGPU to judge that, it's just testing for a fixed output now.

That’s good enough for me.

Thanks,
-Quentin
> 
> - Matthias
> 
>> 
>> Thanks,
>> -Quentin
>> 
>>> On Jun 29, 2015, at 5:33 PM, Matthias Braun <matze at braunis.de> wrote:
>>> 
>>> Author: matze
>>> Date: Mon Jun 29 19:33:44 2015
>>> New Revision: 241027
>>> 
>>> URL: http://llvm.org/viewvc/llvm-project?rev=241027&view=rev
>>> Log:
>>> RegisterCoalescer: Cleanup empty subranges after shrinkToUses()
>>> 
>>> A call to removeEmptySubranges() is necessary after every operation that
>>> potentially removes all segments from a subregister range; this case in
>>> the register coalescer was missing.
>>> 
>>> Added:
>>>  llvm/trunk/test/CodeGen/AMDGPU/subreg-coalescer-undef-use.ll
>>> Modified:
>>>  llvm/trunk/lib/CodeGen/RegisterCoalescer.cpp
>>> 
>>> Modified: llvm/trunk/lib/CodeGen/RegisterCoalescer.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/RegisterCoalescer.cpp?rev=241027&r1=241026&r2=241027&view=diff
>>> ==============================================================================
>>> --- llvm/trunk/lib/CodeGen/RegisterCoalescer.cpp (original)
>>> +++ llvm/trunk/lib/CodeGen/RegisterCoalescer.cpp Mon Jun 29 19:33:44 2015
>>> @@ -1449,6 +1449,7 @@ bool RegisterCoalescer::joinCopy(Machine
>>>                  << format("%04X", S.LaneMask) << ")\n");
>>>     LIS->shrinkToUses(S, LI.reg);
>>>   }
>>> +    LI.removeEmptySubRanges();
>>> }
>>> if (ShrinkMainRange) {
>>>   LiveInterval &LI = LIS->getInterval(CP.getDstReg());
>>> 
>>> Added: llvm/trunk/test/CodeGen/AMDGPU/subreg-coalescer-undef-use.ll
>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/AMDGPU/subreg-coalescer-undef-use.ll?rev=241027&view=auto
>>> ==============================================================================
>>> --- llvm/trunk/test/CodeGen/AMDGPU/subreg-coalescer-undef-use.ll (added)
>>> +++ llvm/trunk/test/CodeGen/AMDGPU/subreg-coalescer-undef-use.ll Mon Jun 29 19:33:44 2015
>>> @@ -0,0 +1,20 @@
>>> +; RUN: llc -march=amdgcn -mcpu=SI -o /dev/null %s
>>> +; Don't crash when the use of an undefined value is only detected by the
>>> +; register coalescer because it is hidden with subregister insert/extract.
>>> +target triple="amdgcn--"
>>> +
>>> +define void @foobar(float %a0, float %a1, float addrspace(1)* %out) nounwind {
>>> +entry:
>>> +  %v0 = insertelement <4 x float> undef, float %a0, i32 0
>>> +  br i1 undef, label %ift, label %ife
>>> +
>>> +ift:
>>> +  %v1 = insertelement <4 x float> undef, float %a1, i32 0
>>> +  br label %ife
>>> +
>>> +ife:
>>> +  %val = phi <4 x float> [ %v1, %ift ], [ %v0, %entry ]
>>> +  %v2 = extractelement <4 x float> %val, i32 1
>>> +  store float %v2, float addrspace(1)* %out, align 4
>>> +  ret void
>>> +}
>>> 
>>> 
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>> 
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>