[Mesa-dev] [PATCH] R600/SI: Use V_ADD_F32 instead of V_MOV_B32 for clamp/neg/abs modifiers.

[Tom Stellard]

On Thu, Feb 07, 2013 at 06:28:51PM +0100, Michel Dänzer wrote:
> From: Michel Dänzer <michel.daenzer at amd.com>
> 
> The modifiers don't seem to have any effect with V_MOV_B32, supposedly it's
> meant to just move bits untouched.
> 
> Fixes 46 piglit tests with radeonsi, though unfortunately 11 of those had
> just regressed because they started using the clamp modifier.
> 
> Signed-off-by: Michel Dänzer <michel.daenzer at amd.com>

Reviewed-by: Tom Stellard <thomas.stellard at amd.com>

Go ahead and commit this to the upstream LLVM tree and mark it is a
candidate for the stable branches, just like we do for Mesa.

> ---
>  lib/Target/R600/SIISelLowering.cpp | 24 +++++++++---------------
>  1 file changed, 9 insertions(+), 15 deletions(-)
> 
> diff --git a/lib/Target/R600/SIISelLowering.cpp b/lib/Target/R600/SIISelLowering.cpp
> index 202584b..87cf596 100644
> --- a/lib/Target/R600/SIISelLowering.cpp
> +++ b/lib/Target/R600/SIISelLowering.cpp
> @@ -74,13 +74,11 @@ MachineBasicBlock * SITargetLowering::EmitInstrWithCustomInserter(
>      return AMDGPUTargetLowering::EmitInstrWithCustomInserter(MI, BB);
>    case AMDGPU::BRANCH: return BB;
>    case AMDGPU::CLAMP_SI:
> -    BuildMI(*BB, I, BB->findDebugLoc(I), TII->get(AMDGPU::V_MOV_B32_e64))
> +    BuildMI(*BB, I, BB->findDebugLoc(I), TII->get(AMDGPU::V_ADD_F32_e64))
>             .addOperand(MI->getOperand(0))
>             .addOperand(MI->getOperand(1))
> -           // VSRC1-2 are unused, but we still need to fill all the
> -           // operand slots, so we just reuse the VSRC0 operand
> -           .addOperand(MI->getOperand(1))
> -           .addOperand(MI->getOperand(1))
> +           .addReg(AMDGPU::SREG_LIT_0)
> +           .addReg(AMDGPU::SREG_LIT_0)
>             .addImm(0) // ABS
>             .addImm(1) // CLAMP
>             .addImm(0) // OMOD
> @@ -89,13 +87,11 @@ MachineBasicBlock * SITargetLowering::EmitInstrWithCustomInserter(
>      break;
>  
>    case AMDGPU::FABS_SI:
> -    BuildMI(*BB, I, BB->findDebugLoc(I), TII->get(AMDGPU::V_MOV_B32_e64))
> +    BuildMI(*BB, I, BB->findDebugLoc(I), TII->get(AMDGPU::V_ADD_F32_e64))
>                   .addOperand(MI->getOperand(0))
>                   .addOperand(MI->getOperand(1))
> -                 // VSRC1-2 are unused, but we still need to fill all the
> -                 // operand slots, so we just reuse the VSRC0 operand
> -                 .addOperand(MI->getOperand(1))
> -                 .addOperand(MI->getOperand(1))
> +                 .addReg(AMDGPU::SREG_LIT_0)
> +                 .addReg(AMDGPU::SREG_LIT_0)
>                   .addImm(1) // ABS
>                   .addImm(0) // CLAMP
>                   .addImm(0) // OMOD
> @@ -104,13 +100,11 @@ MachineBasicBlock * SITargetLowering::EmitInstrWithCustomInserter(
>      break;
>  
>    case AMDGPU::FNEG_SI:
> -    BuildMI(*BB, I, BB->findDebugLoc(I), TII->get(AMDGPU::V_MOV_B32_e64))
> +    BuildMI(*BB, I, BB->findDebugLoc(I), TII->get(AMDGPU::V_ADD_F32_e64))
>                   .addOperand(MI->getOperand(0))
>                   .addOperand(MI->getOperand(1))
> -                 // VSRC1-2 are unused, but we still need to fill all the
> -                 // operand slots, so we just reuse the VSRC0 operand
> -                 .addOperand(MI->getOperand(1))
> -                 .addOperand(MI->getOperand(1))
> +                 .addReg(AMDGPU::SREG_LIT_0)
> +                 .addReg(AMDGPU::SREG_LIT_0)
>                   .addImm(0) // ABS
>                   .addImm(0) // CLAMP
>                   .addImm(0) // OMOD
> -- 
> 1.8.1.2
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev