[PATCH] D21551: AMDGPU: fix local stack slot allocation bugs

Nicolai Hähnle via llvm-commits llvm-commits at lists.llvm.org
Wed Jun 22 12:21:15 PDT 2016


nhaehnle added inline comments.

================
Comment at: lib/Target/AMDGPU/SIMachineFunctionInfo.cpp:111-112
@@ -110,4 +108,1 @@
 
-  if (HasStackObjects || MaySpill)
-    PrivateSegmentWaveByteOffset = true;
-
----------------
arsenm wrote:
> We shouldn't need to always enable this if a new vreg is created for the constant
Right, but is there a way to tell at this stage? This is the MachineFunctionInfo constructor...

================
Comment at: lib/Target/AMDGPU/SIRegisterInfo.cpp:286-290
@@ -285,7 +285,3 @@
 
-  MachineRegisterInfo &MRI = MF->getRegInfo();
-  unsigned UnusedCarry = MRI.createVirtualRegister(&AMDGPU::SReg_64RegClass);
-
-  BuildMI(*MBB, Ins, DL, TII->get(AMDGPU::V_ADD_I32_e64), BaseReg)
-    .addReg(UnusedCarry, RegState::Define | RegState::Dead)
     .addImm(Offset)
----------------
arsenm wrote:
> This is all pre-RA, so there's no issue creating new virtual registers. A new vreg can be created if the immediate isn't a valid inline immediate for the constant, and then it can be folded/shrunk later if needed.
Sure, the problem wasn't creating the virtual register, the problem was that the _e64 encoding only allows a very limited set of immediates. That is, the testcase in the diff fails with a "illegal immediate operand" error.

If some other pass is supposed to pull the immediate out into a V_MOV before the instruction verifier runs, then that wasn't successful...


http://reviews.llvm.org/D21551





More information about the llvm-commits mailing list