[llvm] r288189 - [LiveRangeEdit] Handle instructions with no defs correctly.

[Geoff Berry via llvm-commits]

On 12/2/2016 9:13 AM, Hal Finkel wrote:
> ----- Original Message -----
>> From: "Geoff Berry via llvm-commits" <llvm-commits at lists.llvm.org>
>> To: llvm-commits at lists.llvm.org
>> Sent: Tuesday, November 29, 2016 1:31:35 PM
>> Subject: [llvm] r288189 - [LiveRangeEdit] Handle instructions with no defs correctly.
>>
>> Author: gberry
>> Date: Tue Nov 29 13:31:35 2016
>> New Revision: 288189
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=288189&view=rev
>> Log:
>> [LiveRangeEdit] Handle instructions with no defs correctly.
>>
>> Summary:
>> The code in LiveRangeEdit::eliminateDeadDef() that computes isOrigDef
>> doesn't handle instructions in which operand 0 is not a def (e.g.
>> KILL)
>> correctly.  Add a check that operand 0 is a def before doing the rest
>> of
>> the isOrigDef computation.
>>
>> Reviewers: qcolombet, MatzeB, wmi
>>
>> Subscribers: mcrosier, llvm-commits
>>
>> Differential Revision: https://reviews.llvm.org/D27174
>>
>> Modified:
>>      llvm/trunk/lib/CodeGen/LiveRangeEdit.cpp
>>
>> Modified: llvm/trunk/lib/CodeGen/LiveRangeEdit.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/LiveRangeEdit.cpp?rev=288189&r1=288188&r2=288189&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/CodeGen/LiveRangeEdit.cpp (original)
>> +++ llvm/trunk/lib/CodeGen/LiveRangeEdit.cpp Tue Nov 29 13:31:35 2016
>> @@ -272,7 +272,8 @@ void LiveRangeEdit::eliminateDeadDef(Mac
>>     bool ReadsPhysRegs = false;
>>     bool isOrigDef = false;
>>     unsigned Dest;
>> -  if (VRM && MI->getOperand(0).isReg()) {
>> +  if (VRM && MI->getOperand(0).isReg() && MI->getOperand(0).isDef())
>> {
>> +    assert(MI->getDesc().getNumDefs() == 1);
> I see in the review thread that the issue of instructions that define multiple operands was discussed with the comment that, "we don't have to worry about multiple definition because isReallyTriviallyReMaterializableGeneric filter out multidef instructions." Could you please add a comment here explaining that, because I don't think it is obvious.

Sure, how about something like this:

diff --git a/lib/CodeGen/LiveRangeEdit.cpp b/lib/CodeGen/LiveRangeEdit.cpp
index 88fed62..918be47 100644
--- a/lib/CodeGen/LiveRangeEdit.cpp
+++ b/lib/CodeGen/LiveRangeEdit.cpp
@@ -273,7 +273,9 @@ void LiveRangeEdit::eliminateDeadDef(MachineInstr 
*MI, ToShrinkSet &ToShrink,
    bool isOrigDef = false;
    unsigned Dest;
    if (VRM && MI->getOperand(0).isReg() && MI->getOperand(0).isDef()) {
-    assert(MI->getDesc().getNumDefs() == 1);
+    // It is assumed that callers of eliminateDeadDefs() will never 
pass in dead
+    // instructions with multiple virtual register defs.
+    assert(MI->getDesc().getNumDefs() == 1 && "Unexpected instruction 
with multiple defs.");
      Dest = MI->getOperand(0).getReg();
      unsigned Original = VRM->getOriginal(Dest);
      LiveInterval &OrigLI = LIS.getInterval(Original);


> Thanks again,
> Hal
>
>>       Dest = MI->getOperand(0).getReg();
>>       unsigned Original = VRM->getOriginal(Dest);
>>       LiveInterval &OrigLI = LIS.getInterval(Original);
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>

-- 
Geoff Berry
Employee of Qualcomm Datacenter Technologies, Inc.
  Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.  Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.