[PATCH] R600/SI: Keep 64-bit not on SALU

[Matt Arsenault]

On Jun 7, 2014, at 7:32 AM, Danil <troshkovdanil at mail.ru> wrote:

> 1 )
>>   case AMDGPU::S_XOR_B64:
>>     splitScalar64BitUnaryOp(Worklist, Inst, AMDGPU::S_XOR_B32); 
> 
> There is a bug here. It is clearly if we create two more tests:
> 
>    ; SI-CHECK-LABEL: @vector_xor_i64
>    ; SI-CHECK: V_XOR_B32_e32
>    ; SI-CHECK: V_XOR_B32_e32
>    define void @vector_xor_i64(i64 addrspace(1)* %out, i64 addrspace(1)* %in0, i64 addrspace(1)* %in1) {
>      %a = load i64 addrspace(1)* %in0
>      %b = load i64 addrspace(1)* %in1
>      %result = xor i64 %a, %b
>      store i64 %result, i64 addrspace(1)* %out
>      ret void
>    }
> 
>    ; SI-CHECK-LABEL: @scalar_xor_i64
>    ; SI-CHECK: S_XOR_B64
>    define void @scalar_xor_i64(i64 addrspace(1)* %out, i64 %a, i64 %b) {
>      %result = xor i64 %a, %b
>      store i64 %result, i64 addrspace(1)* %out
>      ret void
>    }
> 
> You should write:
> 
>    case AMDGPU::S_XOR_B64:
>      splitScalar64BitBinaryOp(Worklist, Inst, AMDGPU::S_XOR_B32); 
> 
> I believe you should place this tests into xor.ll (next to vector_xor_i32 and scalar_xor_i32) because they checks you code…
> 

Fixed, I thought these were tested already


> 2 )
> 
>>  case AMDGPU::S_NOT_B64: return AMDGPU::V_NOT_B32_e32;
> 
> I believe you need not place here this string, because control flow never get it.
> As example consider S_AND_B64, S_OR_B64 and S_XOR_B64 from:
> 

It’s not strictly necessary for this patch, but I think it would be best to keep getVALUOp and isSALUOpSupportedOnVALU returning true if it’s possible to move the operation


>    case AMDGPU::S_AND_B64:
>      splitScalar64BitBinaryOp(Worklist, Inst, AMDGPU::S_AND_B32);
>      Inst->eraseFromParent();
>      continue;
> 
>    case AMDGPU::S_OR_B64:
>      splitScalar64BitBinaryOp(Worklist, Inst, AMDGPU::S_OR_B32);
>      Inst->eraseFromParent();
>      continue;
> 
>    case AMDGPU::S_XOR_B64:
>      splitScalar64BitUnaryOp(Worklist, Inst, AMDGPU::S_XOR_B32);
>      Inst->eraseFromParent();
> 
> 3 )
> using this patch I have a fail on CodeGen/R600/sub.ll:
> 
>    llvm\test\CodeGen\R600\sub.ll:48:6: error: expected string not found in input
>   ;SI: S_XOR_B64
>       ^
>   <stdin>:60:2: note: scanning from here
>   S_LOAD_DWORDX2 s[0:1], s[0:1], 0x9
>       ^
>   <stdin>:83:2: note: possible intended match here
>   S_NOT_B64 s[4:5], s[4:5]
>       ^
> Have you?
> 
> http://reviews.llvm.org/D4054
> 
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits