[llvm] r252956 - Revert "Remove unnecessary call to getAllocatableRegClass"
Quentin Colombet via llvm-commits
llvm-commits at lists.llvm.org
Thu Nov 12 18:09:01 PST 2015
Hi Matt,
There are two problems with the original commit:
1. The assertion I’ve suggested in review is not where I asked (see inline comment).
2. Before we create the virtual register we need to restrict the register class on the allocatable reg class. I missed that the call to createVirtualRegister was not to the helper function of SDag but the actual call to createVirtualRegister.
The bottom line is that you may still have cases where the allocatable class will not do what you want, but it is required, like I explained in the review. Anyway, by doing that after the constraining failed, you should be in a much better position than previously. If you aren’t in a better position, then you should drop the whole patch.
Cheers,
-Quentin
> On Nov 12, 2015, at 1:43 PM, Tom Stellard via llvm-commits <llvm-commits at lists.llvm.org> wrote:
>
> Author: tstellar
> Date: Thu Nov 12 15:43:25 2015
> New Revision: 252956
>
> URL: http://llvm.org/viewvc/llvm-project?rev=252956&view=rev
> Log:
> Revert "Remove unnecessary call to getAllocatableRegClass"
>
> This reverts commit r252565.
>
> This also includes the revert of the commit mentioned below in order to
> avoid breaking tests in AMDGPU:
>
> Revert "AMDGPU: Set isAllocatable = 0 on VS_32/VS_64"
>
> This reverts commit r252674.
>
> Modified:
> llvm/trunk/lib/CodeGen/SelectionDAG/InstrEmitter.cpp
> llvm/trunk/lib/Target/AMDGPU/SIRegisterInfo.cpp
> llvm/trunk/lib/Target/AMDGPU/SIRegisterInfo.h
> llvm/trunk/lib/Target/AMDGPU/SIRegisterInfo.td
> llvm/trunk/test/CodeGen/AMDGPU/ftrunc.f64.ll
> llvm/trunk/test/CodeGen/AMDGPU/llvm.round.f64.ll
>
> Modified: llvm/trunk/lib/CodeGen/SelectionDAG/InstrEmitter.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/InstrEmitter.cpp?rev=252956&r1=252955&r2=252956&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/SelectionDAG/InstrEmitter.cpp (original)
> +++ llvm/trunk/lib/CodeGen/SelectionDAG/InstrEmitter.cpp Thu Nov 12 15:43:25 2015
> @@ -330,15 +330,11 @@ 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 *OpRC = nullptr;
> + const TargetRegisterClass *DstRC = nullptr;
> if (IIOpNum < II->getNumOperands())
> - OpRC = TII->getRegClass(*II, IIOpNum, TRI, *MF);
> -
> - if (OpRC && !MRI->constrainRegClass(VReg, OpRC, MinRCSize)) {
> - assert(OpRC->isAllocatable() &&
> - "Constraining an allocatable VReg produced an unallocatable class?");
> -
> - unsigned NewVReg = MRI->createVirtualRegister(OpRC);
> + DstRC = TRI->getAllocatableClass(TII->getRegClass(*II,IIOpNum,TRI,*MF));
> + 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;
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?”);
}
>
> Modified: llvm/trunk/lib/Target/AMDGPU/SIRegisterInfo.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AMDGPU/SIRegisterInfo.cpp?rev=252956&r1=252955&r2=252956&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Target/AMDGPU/SIRegisterInfo.cpp (original)
> +++ llvm/trunk/lib/Target/AMDGPU/SIRegisterInfo.cpp Thu Nov 12 15:43:25 2015
> @@ -79,6 +79,8 @@ unsigned SIRegisterInfo::getRegPressureS
> STI.getMaxWavesPerCU());
> unsigned VGPRLimit = getNumVGPRsAllowed(STI.getMaxWavesPerCU());
>
> + unsigned VSLimit = SGPRLimit + VGPRLimit;
> +
> for (regclass_iterator I = regclass_begin(), E = regclass_end();
> I != E; ++I) {
> const TargetRegisterClass *RC = *I;
> @@ -86,7 +88,11 @@ unsigned SIRegisterInfo::getRegPressureS
> unsigned NumSubRegs = std::max((int)RC->getSize() / 4, 1);
> unsigned Limit;
>
> - if (isSGPRClass(RC)) {
> + if (isPseudoRegClass(RC)) {
> + // FIXME: This is a hack. We should never be considering the pressure of
> + // these since no virtual register should ever have this class.
> + Limit = VSLimit;
> + } else if (isSGPRClass(RC)) {
> Limit = SGPRLimit / NumSubRegs;
> } else {
> Limit = VGPRLimit / NumSubRegs;
>
> Modified: llvm/trunk/lib/Target/AMDGPU/SIRegisterInfo.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AMDGPU/SIRegisterInfo.h?rev=252956&r1=252955&r2=252956&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Target/AMDGPU/SIRegisterInfo.h (original)
> +++ llvm/trunk/lib/Target/AMDGPU/SIRegisterInfo.h Thu Nov 12 15:43:25 2015
> @@ -59,6 +59,13 @@ 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=252956&r1=252955&r2=252956&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Target/AMDGPU/SIRegisterInfo.td (original)
> +++ llvm/trunk/lib/Target/AMDGPU/SIRegisterInfo.td Thu Nov 12 15:43:25 2015
> @@ -272,12 +272,9 @@ def SCSrc_32 : RegInlineOperand<SReg_32>
> // VSrc_* Operands with an SGPR, VGPR or a 32-bit immediate
> //===----------------------------------------------------------------------===//
>
> -def VS_32 : RegisterClass<"AMDGPU", [i32, f32], 32, (add SReg_32, VGPR_32)> {
> - let isAllocatable = 0;
> -}
> +def VS_32 : RegisterClass<"AMDGPU", [i32, f32], 32, (add VGPR_32, SReg_32)>;
>
> -def VS_64 : RegisterClass<"AMDGPU", [i64, f64], 32, (add SReg_64, VReg_64)> {
> - let isAllocatable = 0;
> +def VS_64 : RegisterClass<"AMDGPU", [i64, f64], 32, (add VReg_64, SReg_64)> {
> let CopyCost = 2;
> }
>
>
> Modified: llvm/trunk/test/CodeGen/AMDGPU/ftrunc.f64.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/AMDGPU/ftrunc.f64.ll?rev=252956&r1=252955&r2=252956&view=diff
> ==============================================================================
> --- llvm/trunk/test/CodeGen/AMDGPU/ftrunc.f64.ll (original)
> +++ llvm/trunk/test/CodeGen/AMDGPU/ftrunc.f64.ll Thu Nov 12 15:43:25 2015
> @@ -27,8 +27,8 @@ define void @v_ftrunc_f64(double addrspa
> ; SI: s_and_b32 s{{[0-9]+}}, s{{[0-9]+}}, 0x80000000
> ; SI: s_add_i32 s{{[0-9]+}}, [[SEXP]], 0xfffffc01
> ; SI: s_lshr_b64
> -; SI-DAG: s_not_b64
> -; SI-DAG: s_and_b64
> +; SI: s_not_b64
> +; SI: s_and_b64
> ; SI-DAG: cmp_gt_i32
> ; SI-DAG: cndmask_b32
> ; SI-DAG: cndmask_b32
>
> Modified: llvm/trunk/test/CodeGen/AMDGPU/llvm.round.f64.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/AMDGPU/llvm.round.f64.ll?rev=252956&r1=252955&r2=252956&view=diff
> ==============================================================================
> --- llvm/trunk/test/CodeGen/AMDGPU/llvm.round.f64.ll (original)
> +++ llvm/trunk/test/CodeGen/AMDGPU/llvm.round.f64.ll Thu Nov 12 15:43:25 2015
> @@ -21,7 +21,7 @@ define void @round_f64(double addrspace(
> ; SI-DAG: v_cmp_eq_i32
>
> ; SI-DAG: s_mov_b32 [[BFIMASK:s[0-9]+]], 0x7fffffff
> -; SI-DAG: v_cmp_gt_i32
> +; SI-DAG: v_cmp_gt_i32_e32
> ; SI-DAG: v_bfi_b32 [[COPYSIGN:v[0-9]+]], [[BFIMASK]]
>
> ; SI: buffer_store_dwordx2
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20151112/dc69b446/attachment.html>
More information about the llvm-commits
mailing list