[PATCHES] AMDGPU/SI: Small ISel improvements

Matt Arsenault via llvm-commits llvm-commits at lists.llvm.org
Wed Oct 14 17:01:43 PDT 2015


> On Oct 14, 2015, at 4:49 PM, Marek Olšák via llvm-commits <llvm-commits at lists.llvm.org> wrote:
> 
>> diff --git a/lib/Target/AMDGPU/SIInstrInfo.cpp
>> b/lib/Target/AMDGPU/SIInstrInfo.cpp
>> index 1af08a8..d7904b0 100644
>> --- a/lib/Target/AMDGPU/SIInstrInfo.cpp
>> +++ b/lib/Target/AMDGPU/SIInstrInfo.cpp
>> @@ -2290,6 +2290,31 @@ void SIInstrInfo::moveToVALU(MachineInstr &TopInst)
>> const {
>>       }
>>       break;
>> 
>> +    case AMDGPU::S_ABS_I32: {
>> +      MachineBasicBlock &MBB = *Inst->getParent();
>> +      MachineRegisterInfo &MRI = MBB.getParent()->getRegInfo();
>> +      MachineBasicBlock::iterator MII = Inst;
>> +      DebugLoc DL = Inst->getDebugLoc();
>> +
>> +      MachineOperand &Dest = Inst->getOperand(0);
>> +      MachineOperand &Src = Inst->getOperand(1);
>> +      unsigned TmpReg =
>> MRI.createVirtualRegister(&AMDGPU::VGPR_32RegClass);
>> +      unsigned ResultReg =
>> MRI.createVirtualRegister(&AMDGPU::VGPR_32RegClass);
>> +
>> +      BuildMI(MBB, MII, DL, get(AMDGPU::V_SUB_I32_e32), TmpReg)
>> +        .addImm(0)
>> +        .addReg(Src.getReg());
>> 
>> 
>> I think this could break if Src is a subregister. This probably needs what
>> splitScalar64BitUnaryOp does to handle subregisters. I would also prefer
>> splitting this into a separate function
> 
> How can Src be a subregister?
> 
> SMAX isn't supported for vector types, so S_ABS_I32 isn't selected for
> those. I wonder why the backend doesn't lower vectors to scalars by
> default.
> 
> Marek

The operation isn’t a vector operation, but if the source is a vector, the individual components are sub registers from a vector load. It’s possible min/max isn’t being matched now for things that happen to be IR vectors because of the weirdness where now min/max is matched during SelectionDAGBuilder if the operation is legal for the type. This would be a good reason to resurrect my patch to match min/max in DAGCombiner so we can catch these on a legalized vector.

-Matt



More information about the llvm-commits mailing list