[llvm] r280783 - Remove unnecessary call to getAllocatableRegClass

[Matt Arsenault via llvm-commits]

> On Sep 21, 2016, at 16:49, Justin Bogner via llvm-commits <llvm-commits at lists.llvm.org> wrote:
> 
> Matt Arsenault via llvm-commits <llvm-commits at lists.llvm.org> writes:
>> Author: arsenm
>> Date: Wed Sep  7 01:16:45 2016
>> New Revision: 280783
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=280783&view=rev
>> Log:
>> Remove unnecessary call to getAllocatableRegClass
>> 
>> This reapplies r252565 and r252674, effectively reverting r252956.
>> 
>> This allows VS_32/VS_64 to be unallocatable like they should be.
>> 
>> Modified:
>>    llvm/trunk/lib/CodeGen/SelectionDAG/InstrEmitter.cpp
>>    llvm/trunk/lib/Target/AMDGPU/SIRegisterInfo.h
>>    llvm/trunk/lib/Target/AMDGPU/SIRegisterInfo.td
>> 
>> Modified: llvm/trunk/lib/CodeGen/SelectionDAG/InstrEmitter.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/InstrEmitter.cpp?rev=280783&r1=280782&r2=280783&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/CodeGen/SelectionDAG/InstrEmitter.cpp (original)
>> +++ llvm/trunk/lib/CodeGen/SelectionDAG/InstrEmitter.cpp Wed Sep  7 01:16:45 2016
>> @@ -330,16 +330,22 @@ InstrEmitter::AddRegisterOperand(Machine
>>   // shrink VReg's register class within reason.  For example, if VReg == GR32
>>   // and II requires a GR32_NOSP, just constrain VReg to GR32_NOSP.
>>   if (II) {
>> -    const TargetRegisterClass *DstRC = nullptr;
>> +    const TargetRegisterClass *OpRC = nullptr;
>>     if (IIOpNum < II->getNumOperands())
>> -      DstRC = TRI->getAllocatableClass(TII->getRegClass(*II,IIOpNum,TRI,*MF));
>> -    assert((!DstRC || TargetRegisterInfo::isVirtualRegister(VReg)) &&
>> -           "Expected VReg");
>> -    if (DstRC && !MRI->constrainRegClass(VReg, DstRC, MinRCSize)) {
>> -      unsigned NewVReg = MRI->createVirtualRegister(DstRC);
>> -      BuildMI(*MBB, InsertPos, Op.getNode()->getDebugLoc(),
>> -              TII->get(TargetOpcode::COPY), NewVReg).addReg(VReg);
>> -      VReg = NewVReg;
>> +      OpRC = TII->getRegClass(*II, IIOpNum, TRI, *MF);
>> +
>> +    if (OpRC) {
>> +      const TargetRegisterClass *ConstrainedRC
>> +        = MRI->constrainRegClass(VReg, OpRC, MinRCSize);
>> +      if (!ConstrainedRC) {
>> +        unsigned NewVReg = MRI->createVirtualRegister(OpRC);
> 
> I'm seeing an assert when createVirtualRegister is called here in one of
> my backends, since OpRC isn't allocatable (and createVirtualRegister
> asserts that it is).
> 
> Interestingly, in the review of the revert, r252956, Quentin's example
> code handled this as I would expect:
> 
> In
> http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20151109/312603.html,
> Quentin Colombet <qcolombet at apple.com> writes:
>>> The assertion was supposed to be in the else part, not the if part.
>>> I.e.,
>>> if (DstRC) { // Break this if
>>>  if (!MRI->constrainRegClass(VReg, DstRC, MinRCSize)) {
>>>    // This is point #2 I missed.
>>>    DstRC = TRI->getAllocatableClass(DstRC); // Refine for allocatable class.
>>>    assert(DstRC && “Operation constraints cannot be fulfilled for allocation”);
>>>    unsigned NewVReg = MRI->createVirtualRegister(DstRC);
>>>    ...
>>>  } else // add this else
>>>      assert(OpRC->isAllocatable() &&
>>>             "Constraining an allocatable VReg produced an unallocatable class?”);
>>> }
> 
> Am I missing something here?

Is this the noreg case?

This code was slightly off because the returned register class of constrainRegClass needs to be used, so that it didn’t solve the original problem.

-Matt