[llvm] r280783 - Remove unnecessary call to getAllocatableRegClass

Justin Bogner via llvm-commits llvm-commits at lists.llvm.org
Wed Sep 21 16:49:59 PDT 2016


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?

> +        BuildMI(*MBB, InsertPos, Op.getNode()->getDebugLoc(),
> +                TII->get(TargetOpcode::COPY), NewVReg).addReg(VReg);
> +        VReg = NewVReg;
> +      } else {
> +        assert(ConstrainedRC->isAllocatable() &&
> +           "Constraining an allocatable VReg produced an unallocatable class?");
> +      }
>      }
>    }
>  
>
> Modified: llvm/trunk/lib/Target/AMDGPU/SIRegisterInfo.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AMDGPU/SIRegisterInfo.h?rev=280783&r1=280782&r2=280783&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Target/AMDGPU/SIRegisterInfo.h (original)
> +++ llvm/trunk/lib/Target/AMDGPU/SIRegisterInfo.h Wed Sep  7 01:16:45 2016
> @@ -107,13 +107,6 @@ public:
>    /// \returns true if this class contains VGPR registers.
>    bool hasVGPRs(const TargetRegisterClass *RC) const;
>  
> -  /// returns true if this is a pseudoregister class combination of VGPRs and
> -  /// SGPRs for operand modeling. FIXME: We should set isAllocatable = 0 on
> -  /// them.
> -  static bool isPseudoRegClass(const TargetRegisterClass *RC) {
> -    return RC == &AMDGPU::VS_32RegClass || RC == &AMDGPU::VS_64RegClass;
> -  }
> -
>    /// \returns A VGPR reg class with the same width as \p SRC
>    const TargetRegisterClass *getEquivalentVGPRClass(
>                                            const TargetRegisterClass *SRC) const;
>
> Modified: llvm/trunk/lib/Target/AMDGPU/SIRegisterInfo.td
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AMDGPU/SIRegisterInfo.td?rev=280783&r1=280782&r2=280783&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Target/AMDGPU/SIRegisterInfo.td (original)
> +++ llvm/trunk/lib/Target/AMDGPU/SIRegisterInfo.td Wed Sep  7 01:16:45 2016
> @@ -346,10 +346,12 @@ def VReg_1 : RegisterClass<"AMDGPU", [i1
>    let Size = 32;
>  }
>  
> -def VS_32 : RegisterClass<"AMDGPU", [i32, f32], 32, (add VGPR_32, SReg_32)>;
> +def VS_32 : RegisterClass<"AMDGPU", [i32, f32], 32, (add VGPR_32, SReg_32)> {
> +  let isAllocatable = 0;
> +}
>  
>  def VS_64 : RegisterClass<"AMDGPU", [i64, f64], 32, (add VReg_64, SReg_64)> {
> -  let CopyCost = 2;
> +  let isAllocatable = 0;
>  }
>  
>  //===----------------------------------------------------------------------===//
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits


More information about the llvm-commits mailing list