[PATCH] R600/SI: Fix indirect addressing with a negative constant offset

Christian König deathsimple at vodafone.de
Mon Mar 23 02:51:09 PDT 2015


On 20.03.2015 02:35, Tom Stellard wrote:
> When the base register index of the vector plus the constant offset
> was less than zero, we were passing the wrong base register to the indirect
> addressing instruction.
>
> In this case, we need to set the base register to v0 and then add
> the computed (negative) index to m0.

Strange, I clearly remember that I thought about handling that case when 
I wrote the initial code, but never did so. Anyway patch LGTM.

Regards,
Christian.

> ---
>   lib/Target/R600/SILowerControlFlow.cpp      | 71 ++++++++++++++++++++++-------
>   test/CodeGen/R600/indirect-addressing-si.ll | 57 +++++++++++++++++++++++
>   2 files changed, 112 insertions(+), 16 deletions(-)
>
> diff --git a/lib/Target/R600/SILowerControlFlow.cpp b/lib/Target/R600/SILowerControlFlow.cpp
> index 2e08c9f..c319b32 100644
> --- a/lib/Target/R600/SILowerControlFlow.cpp
> +++ b/lib/Target/R600/SILowerControlFlow.cpp
> @@ -88,7 +88,8 @@ private:
>     void Kill(MachineInstr &MI);
>     void Branch(MachineInstr &MI);
>   
> -  void LoadM0(MachineInstr &MI, MachineInstr *MovRel);
> +  void LoadM0(MachineInstr &MI, MachineInstr *MovRel, int Offset = 0);
> +  void computeIndirectRegAndOffset(unsigned VecReg, unsigned &Reg, int &Offset);
>     void IndirectSrc(MachineInstr &MI);
>     void IndirectDst(MachineInstr &MI);
>   
> @@ -323,7 +324,7 @@ void SILowerControlFlowPass::Kill(MachineInstr &MI) {
>     MI.eraseFromParent();
>   }
>   
> -void SILowerControlFlowPass::LoadM0(MachineInstr &MI, MachineInstr *MovRel) {
> +void SILowerControlFlowPass::LoadM0(MachineInstr &MI, MachineInstr *MovRel, int Offset) {
>   
>     MachineBasicBlock &MBB = *MI.getParent();
>     DebugLoc DL = MI.getDebugLoc();
> @@ -333,8 +334,14 @@ void SILowerControlFlowPass::LoadM0(MachineInstr &MI, MachineInstr *MovRel) {
>     unsigned Idx = MI.getOperand(3).getReg();
>   
>     if (AMDGPU::SReg_32RegClass.contains(Idx)) {
> -    BuildMI(MBB, &MI, DL, TII->get(AMDGPU::S_MOV_B32), AMDGPU::M0)
> -            .addReg(Idx);
> +    if (Offset) {
> +      BuildMI(MBB, &MI, DL, TII->get(AMDGPU::S_ADD_I32), AMDGPU::M0)
> +              .addReg(Idx)
> +              .addImm(Offset);
> +    } else {
> +      BuildMI(MBB, &MI, DL, TII->get(AMDGPU::S_MOV_B32), AMDGPU::M0)
> +              .addReg(Idx);
> +    }
>       MBB.insert(I, MovRel);
>     } else {
>   
> @@ -363,6 +370,11 @@ void SILowerControlFlowPass::LoadM0(MachineInstr &MI, MachineInstr *MovRel) {
>       BuildMI(MBB, &MI, DL, TII->get(AMDGPU::S_AND_SAVEEXEC_B64), AMDGPU::VCC)
>               .addReg(AMDGPU::VCC);
>   
> +    if (Offset) {
> +      BuildMI(MBB, &MI, DL, TII->get(AMDGPU::S_ADD_I32), AMDGPU::M0)
> +              .addReg(AMDGPU::M0)
> +              .addImm(Offset);
> +    }
>       // Do the actual move
>       MBB.insert(I, MovRel);
>   
> @@ -384,6 +396,33 @@ void SILowerControlFlowPass::LoadM0(MachineInstr &MI, MachineInstr *MovRel) {
>     MI.eraseFromParent();
>   }
>   
> +/// \param @VecReg The register which holds element zero of the vector
> +///                 being addressed into.
> +/// \param[out] @Reg The base register to use in the indirect addressing instruction.
> +/// \param[in,out] @Offset As an input, this is the constant offset part of the
> +//                         indirect Index. e.g. v0 = v[VecReg + Offset]
> +//                         As an output, this is a constant value that needs
> +//                         to be added to the value stored in M0.
> +void SILowerControlFlowPass::computeIndirectRegAndOffset(unsigned VecReg,
> +                                                         unsigned &Reg,
> +                                                         int &Offset) {
> +  unsigned SubReg = TRI->getSubReg(VecReg, AMDGPU::sub0);
> +  if (!SubReg)
> +    SubReg = VecReg;
> +
> +  const TargetRegisterClass *RC = TRI->getPhysRegClass(SubReg);
> +  int RegIdx = TRI->getHWRegIndex(SubReg) + Offset;
> +
> +  if (RegIdx < 0) {
> +    Offset = RegIdx;
> +    RegIdx = 0;
> +  } else {
> +    Offset = 0;
> +  }
> +
> +  Reg = RC->getRegister(RegIdx);
> +}
> +
>   void SILowerControlFlowPass::IndirectSrc(MachineInstr &MI) {
>   
>     MachineBasicBlock &MBB = *MI.getParent();
> @@ -391,18 +430,18 @@ void SILowerControlFlowPass::IndirectSrc(MachineInstr &MI) {
>   
>     unsigned Dst = MI.getOperand(0).getReg();
>     unsigned Vec = MI.getOperand(2).getReg();
> -  unsigned Off = MI.getOperand(4).getImm();
> -  unsigned SubReg = TRI->getSubReg(Vec, AMDGPU::sub0);
> -  if (!SubReg)
> -    SubReg = Vec;
> +  int Off = MI.getOperand(4).getImm();
> +  unsigned Reg;
> +
> +  computeIndirectRegAndOffset(Vec, Reg, Off);
>   
>     MachineInstr *MovRel =
>       BuildMI(*MBB.getParent(), DL, TII->get(AMDGPU::V_MOVRELS_B32_e32), Dst)
> -            .addReg(SubReg + Off)
> +            .addReg(Reg)
>               .addReg(AMDGPU::M0, RegState::Implicit)
>               .addReg(Vec, RegState::Implicit);
>   
> -  LoadM0(MI, MovRel);
> +  LoadM0(MI, MovRel, Off);
>   }
>   
>   void SILowerControlFlowPass::IndirectDst(MachineInstr &MI) {
> @@ -411,20 +450,20 @@ void SILowerControlFlowPass::IndirectDst(MachineInstr &MI) {
>     DebugLoc DL = MI.getDebugLoc();
>   
>     unsigned Dst = MI.getOperand(0).getReg();
> -  unsigned Off = MI.getOperand(4).getImm();
> +  int Off = MI.getOperand(4).getImm();
>     unsigned Val = MI.getOperand(5).getReg();
> -  unsigned SubReg = TRI->getSubReg(Dst, AMDGPU::sub0);
> -  if (!SubReg)
> -    SubReg = Dst;
> +  unsigned Reg;
> +
> +  computeIndirectRegAndOffset(Dst, Reg, Off);
>   
>     MachineInstr *MovRel =
>       BuildMI(*MBB.getParent(), DL, TII->get(AMDGPU::V_MOVRELD_B32_e32))
> -            .addReg(SubReg + Off, RegState::Define)
> +            .addReg(Reg, RegState::Define)
>               .addReg(Val)
>               .addReg(AMDGPU::M0, RegState::Implicit)
>               .addReg(Dst, RegState::Implicit);
>   
> -  LoadM0(MI, MovRel);
> +  LoadM0(MI, MovRel, Off);
>   }
>   
>   bool SILowerControlFlowPass::runOnMachineFunction(MachineFunction &MF) {
> diff --git a/test/CodeGen/R600/indirect-addressing-si.ll b/test/CodeGen/R600/indirect-addressing-si.ll
> index 319910f..d6624f2 100644
> --- a/test/CodeGen/R600/indirect-addressing-si.ll
> +++ b/test/CodeGen/R600/indirect-addressing-si.ll
> @@ -25,6 +25,33 @@ entry:
>     ret void
>   }
>   
> +; CHECK-LABEL: {{^}}extract_neg_offset_sgpr:
> +; The offset depends on the register that holds the first element of the vector.
> +; CHECK: s_add_i32 m0, s{{[0-9]+}}, 0xfffffe{{[0-9a-z]+}}
> +; CHECK: v_movrels_b32_e32 v{{[0-9]}}, v0
> +define void @extract_neg_offset_sgpr(i32 addrspace(1)* %out, i32 %offset) {
> +entry:
> +  %index = add i32 %offset, -512
> +  %value = extractelement <4 x i32> <i32 0, i32 1, i32 2, i32 3>, i32 %index
> +  store i32 %value, i32 addrspace(1)* %out
> +  ret void
> +}
> +
> +; CHECK-LABEL: {{^}}extract_neg_offset_vgpr:
> +; The offset depends on the register that holds the first element of the vector.
> +; CHECK: v_readfirstlane_b32
> +; CHECK: s_add_i32 m0, m0, 0xfffffe{{[0-9a-z]+}}
> +; CHECK-NEXT: v_movrels_b32_e32 v{{[0-9]}}, v0
> +; CHECK: s_cbranch_execnz
> +define void @extract_neg_offset_vgpr(i32 addrspace(1)* %out) {
> +entry:
> +  %id = call i32 @llvm.r600.read.tidig.x() #1
> +  %index = add i32 %id, -512
> +  %value = extractelement <4 x i32> <i32 0, i32 1, i32 2, i32 3>, i32 %index
> +  store i32 %value, i32 addrspace(1)* %out
> +  ret void
> +}
> +
>   ; CHECK-LABEL: {{^}}insert_w_offset:
>   ; CHECK: s_mov_b32 m0
>   ; CHECK-NEXT: v_movreld_b32_e32
> @@ -47,3 +74,33 @@ entry:
>     store float %1, float addrspace(1)* %out
>     ret void
>   }
> +
> +; CHECK-LABEL: {{^}}insert_neg_offset_sgpr:
> +; The offset depends on the register that holds the first element of the vector.
> +; CHECK: s_add_i32 m0, s{{[0-9]+}}, 0xfffffe{{[0-9a-z]+}}
> +; CHECK: v_movreld_b32_e32 v0, v{{[0-9]}}
> +define void @insert_neg_offset_sgpr(i32 addrspace(1)* %in, <4 x i32> addrspace(1)* %out, i32 %offset) {
> +entry:
> +  %index = add i32 %offset, -512
> +  %value = insertelement <4 x i32> <i32 0, i32 1, i32 2, i32 3>, i32 5, i32 %index
> +  store <4 x i32> %value, <4 x i32> addrspace(1)* %out
> +  ret void
> +}
> +
> +; CHECK-LABEL: {{^}}insert_neg_offset_vgpr:
> +; The offset depends on the register that holds the first element of the vector.
> +; CHECK: v_readfirstlane_b32
> +; CHECK: s_add_i32 m0, m0, 0xfffffe{{[0-9a-z]+}}
> +; CHECK-NEXT: v_movreld_b32_e32 v0, v{{[0-9]}}
> +; CHECK: s_cbranch_execnz
> +define void @insert_neg_offset_vgpr(i32 addrspace(1)* %in, <4 x i32> addrspace(1)* %out) {
> +entry:
> +  %id = call i32 @llvm.r600.read.tidig.x() #1
> +  %index = add i32 %id, -512
> +  %value = insertelement <4 x i32> <i32 0, i32 1, i32 2, i32 3>, i32 5, i32 %index
> +  store <4 x i32> %value, <4 x i32> addrspace(1)* %out
> +  ret void
> +}
> +
> +declare i32 @llvm.r600.read.tidig.x() #1
> +attributes #1 = { nounwind readnone }




More information about the llvm-commits mailing list