[llvm] r241027 - RegisterCoalescer: Cleanup empty subranges after shrinkToUses()
Quentin Colombet
qcolombet at apple.com
Thu Jul 16 12:58:52 PDT 2015
Thanks!
Q.
> On Jul 16, 2015, at 11:56 AM, Matthias Braun <matze at braunis.de> wrote:
>
> I improved things in r242431.
>
> - Matthias
>
>> On Jul 16, 2015, at 10:37 AM, Quentin Colombet <qcolombet at apple.com <mailto:qcolombet at apple.com>> wrote:
>>
>> Ping!
>>
>> On the highlighted points, look for ##
>>
>>> On Jul 1, 2015, at 4:04 PM, Quentin Colombet <qcolombet at apple.com <mailto:qcolombet at apple.com>> wrote:
>>>
>>>>
>>>> On Jul 1, 2015, at 3:36 PM, Matthias Braun <matze at braunis.de <mailto:matze at braunis.de>> wrote:
>>>>
>>>>
>>>>> On Jul 1, 2015, at 3:12 PM, Quentin Colombet <qcolombet at apple.com <mailto: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!
>>
>> Thanks,
>> -Quentin
>>>
>>>> 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 <mailto: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 <https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject-3Frev-3D241027-26view-3Drev&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=pCcxCKfF2PXt8TiSX5s8Ta-Wnhb2pKT_sldhGZ9NGD4&s=bdHrjCMa5HP0Il4M8ZeICCb7FggLodM0dWuFB16JN1g&e=>
>>>>>> 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 <https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_llvm_trunk_lib_CodeGen_RegisterCoalescer.cpp-3Frev-3D241027-26r1-3D241026-26r2-3D241027-26view-3Ddiff&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=pCcxCKfF2PXt8TiSX5s8Ta-Wnhb2pKT_sldhGZ9NGD4&s=DfXdw403uIKq4O24cYCW9eoorJXzmyzqlvSEtv0uL3k&e=>
>>>>>> ==============================================================================
>>>>>> --- 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 <https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_llvm_trunk_test_CodeGen_AMDGPU_subreg-2Dcoalescer-2Dundef-2Duse.ll-3Frev-3D241027-26view-3Dauto&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=pCcxCKfF2PXt8TiSX5s8Ta-Wnhb2pKT_sldhGZ9NGD4&s=AaVYqE4fF_EZFC5XsYzkpZ6gbqPhRaWV6L9bq1FjSyM&e=>
>>>>>> ==============================================================================
>>>>>> --- 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 <mailto:llvm-commits at cs.uiuc.edu>
>>>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits <http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits>
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> llvm-commits mailing list
>>>>> llvm-commits at cs.uiuc.edu <mailto:llvm-commits at cs.uiuc.edu>
>>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits <http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits>
>>>>
>>>
>>>
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu <mailto:llvm-commits at cs.uiuc.edu>
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits <http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu <mailto:llvm-commits at cs.uiuc.edu>
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150716/06a960ff/attachment.html>
More information about the llvm-commits
mailing list