[llvm-commits] [llvm] r138289 - /llvm/trunk/lib/CodeGen/TargetInstrInfoImpl.cpp

[Evan Cheng]

The only idea I have is to look for this statistics: "Number of instruction commuting performed". You're right it's hard to add a non-fragile test case so I wouldn't worry about it.

Evan

On Aug 22, 2011, at 4:25 PM, Jim Grosbach wrote:

> Hey Evan,
> 
> Thanks for the fix. I'm open to suggestions on how to create a non-fragile test case for something like this. I can reduce the code that failed because of it easily enough, but that'll be pretty fragile such that changes in either isel or the register allocator would likely perturb it. The failure was detected by the nightly test suite. While not optimal, perhaps that's sufficient?
> 
> -Jim
> 
> 
> On Aug 22, 2011, at 4:04 PM, Evan Cheng wrote:
> 
>> Author: evancheng
>> Date: Mon Aug 22 18:04:56 2011
>> New Revision: 138289
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=138289&view=rev
>> Log:
>> Follow up to Jim's r138278. This fixes commuteInstruction so it handles two-address instructions correctly. I'll let Jim add a test case. :-)
>> 
>> Modified:
>>   llvm/trunk/lib/CodeGen/TargetInstrInfoImpl.cpp
>> 
>> Modified: llvm/trunk/lib/CodeGen/TargetInstrInfoImpl.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/TargetInstrInfoImpl.cpp?rev=138289&r1=138288&r2=138289&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/CodeGen/TargetInstrInfoImpl.cpp (original)
>> +++ llvm/trunk/lib/CodeGen/TargetInstrInfoImpl.cpp Mon Aug 22 18:04:56 2011
>> @@ -74,23 +74,25 @@
>> 
>>  assert(MI->getOperand(Idx1).isReg() && MI->getOperand(Idx2).isReg() &&
>>         "This only knows how to commute register operands so far");
>> +  unsigned Reg0 = HasDef ? MI->getOperand(0).getReg() : 0;
>>  unsigned Reg1 = MI->getOperand(Idx1).getReg();
>>  unsigned Reg2 = MI->getOperand(Idx2).getReg();
>>  bool Reg1IsKill = MI->getOperand(Idx1).isKill();
>>  bool Reg2IsKill = MI->getOperand(Idx2).isKill();
>> -  bool ChangeReg0 = false;
>> -  if (HasDef && MI->getOperand(0).getReg() == Reg1) {
>> -    // Must be two address instruction!
>> -    assert(MI->getDesc().getOperandConstraint(0, MCOI::TIED_TO) &&
>> -           "Expecting a two-address instruction!");
>> +  // If destination is tied to either of the commuted source register, then
>> +  // it must be updated.
>> +  if (HasDef && Reg0 == Reg1 &&
>> +      MI->getDesc().getOperandConstraint(Idx1, MCOI::TIED_TO) == 0) {
>>    Reg2IsKill = false;
>> -    ChangeReg0 = true;
>> +    Reg0 = Reg2;
>> +  } else if (HasDef && Reg0 == Reg2 &&
>> +             MI->getDesc().getOperandConstraint(Idx2, MCOI::TIED_TO) == 0) {
>> +    Reg1IsKill = false;
>> +    Reg0 = Reg1;
>>  }
>> 
>>  if (NewMI) {
>>    // Create a new instruction.
>> -    unsigned Reg0 = HasDef
>> -      ? (ChangeReg0 ? Reg2 : MI->getOperand(0).getReg()) : 0;
>>    bool Reg0IsDead = HasDef ? MI->getOperand(0).isDead() : false;
>>    MachineFunction &MF = *MI->getParent()->getParent();
>>    if (HasDef)
>> @@ -104,8 +106,8 @@
>>        .addReg(Reg1, getKillRegState(Reg2IsKill));
>>  }
>> 
>> -  if (ChangeReg0)
>> -    MI->getOperand(0).setReg(Reg2);
>> +  if (HasDef)
>> +    MI->getOperand(0).setReg(Reg0);
>>  MI->getOperand(Idx2).setReg(Reg1);
>>  MI->getOperand(Idx1).setReg(Reg2);
>>  MI->getOperand(Idx2).setIsKill(Reg1IsKill);
>> 
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>