DAG: Fix mis-legalization of i1 zextload

[arsenm]

Summary

The bits aren't guaranteed to be zero.

[efriedma]

Can you describe the issue you're trying to fix here in a bit more detail? As far as I know, the bits are in fact guaranteed to be zero.

[arsenm]

In D31400#711506, @efriedma wrote:

Can you describe the issue you're trying to fix here in a bit more detail? As far as I know, the bits are in fact guaranteed to be zero.

i1 zextload to i32 is being legalized to AssertZext (zextload i8 to i32), so there was no and truncating to a single bit. As far as I know nothing is guaranteed about the high bits in memory of a < byte value.

[t.p.northover]

It's certainly something many ABIs guarantee, so either BooleanContents models it in some way (the value for i8?) or we could do with something that does.

[arsenm]

In D31400#711522, @arsenm wrote:

In D31400#711506, @efriedma wrote:

Can you describe the issue you're trying to fix here in a bit more detail? As far as I know, the bits are in fact guaranteed to be zero.

i1 zextload to i32 is being legalized to AssertZext (zextload i8 to i32), so there was no and truncating to a single bit. As far as I know nothing is guaranteed about the high bits in memory of a < byte value.

If an <N x i1> vector is supposed to be packed in memory, I would expect
%cast.ptr = bitcast <64 x i1> to i1*
%elt0 = load i1, i1* %cast.ptr
to do the right thing and truncate the value

[efriedma]

We ensure that the high bits of i1 etc. in memory are zero when we lower a store... which is really all that matters. See SelectionDAGLegalize::LegalizeStoreOps.

[arsenm]

I think this is still broken but I lack the will to push on this further

lib/CodeGen/SelectionDAG/LegalizeDAG.cpp

if (SrcWidth != SrcVT.getStoreSizeInBits() &&
 
     Ch = Result.getValue(1); // The chain.
 
     if (ExtType == ISD::SEXTLOAD)
       // Having the top bits zero doesn't help when sign extending.
       Result = DAG.getNode(ISD::SIGN_EXTEND_INREG, dl,
                            Result.getValueType(),
                            Result, DAG.getValueType(SrcVT));
-    else if (ExtType == ISD::ZEXTLOAD || NVT == Result.getValueType())
-      // All the top bits are guaranteed to be zero - inform the optimizers.
-      Result = DAG.getNode(ISD::AssertZext, dl,
-                           Result.getValueType(), Result,
-                           DAG.getValueType(SrcVT));
+    else if (ExtType == ISD::ZEXTLOAD || NVT == Result.getValueType()) {
+      Result = DAG.getZeroExtendInReg(Result, dl, SrcVT);
+    }
 
     Value = Result;
     Chain = Ch;
   } else if (SrcWidth & (SrcWidth - 1)) {
     // If not loading a power-of-2 number of bits, expand as two loads.
     assert(!SrcVT.isVector() && "Unsupported extload!");
     unsigned RoundWidth = 1 << Log2_32(SrcWidth);
     assert(RoundWidth < SrcWidth);

test/CodeGen/AMDGPU/ctlz.ll

define amdgpu_kernel void @v_ctlz_i16_sel_eq_neg1(i16 addrspace(1)* noalias %out, i16 addrspace(1)* noalias %valptr) nounwind {
   %ctlz = call i16 @llvm.ctlz.i16(i16 %val, i1 false) nounwind readnone
   %cmp = icmp eq i16 %val, 0
   %sel = select i1 %cmp, i16 -1, i16 %ctlz
   store i16 %sel, i16 addrspace(1)* %out
   ret void
 }
 
 ; FIXME: Need to handle non-uniform case for function below (load without gep).
+; FIXME: Why difference with VI?
 ; FUNC-LABEL: {{^}}v_ctlz_i7_sel_eq_neg1:
-; GCN: {{buffer|flat}}_load_ubyte [[VAL:v[0-9]+]],
+; GCN-DAG: {{buffer|flat}}_load_ubyte [[VAL:v[0-9]+]],
+; SI-DAG: s_movk_i32 [[MASK:s[0-9]+]], 0x7f
+; VI: v_and_b32_e32 [[VAL]], 0x7f, [[VAL]]
 ; GCN: v_ffbh_u32_e32 [[FFBH:v[0-9]+]], [[VAL]]
-; GCN: v_and_b32_e32 [[TRUNC:v[0-9]+]], 0x7f, [[FFBH]]
+; SI: v_and_b32_e32 [[TRUNC:v[0-9]+]], [[MASK]], [[FFBH]]
+; VI: v_and_b32_e32 [[TRUNC:v[0-9]+]], 0x7f, [[FFBH]]
 ; GCN: {{buffer|flat}}_store_byte [[TRUNC]],
 define amdgpu_kernel void @v_ctlz_i7_sel_eq_neg1(i7 addrspace(1)* noalias %out, i7 addrspace(1)* noalias %valptr) nounwind {
   %tid = call i32 @llvm.r600.read.tidig.x()
   %valptr.gep = getelementptr i7, i7 addrspace(1)* %valptr, i32 %tid
   %val = load i7, i7 addrspace(1)* %valptr.gep
   %ctlz = call i7 @llvm.ctlz.i7(i7 %val, i1 false) nounwind readnone
   %cmp = icmp eq i7 %val, 0
   %sel = select i1 %cmp, i7 -1, i7 %ctlz
   store i7 %sel, i7 addrspace(1)* %out
   ret void
 }

test/CodeGen/AMDGPU/load-local-i1.ll

 ; FUNC-LABEL: {{^}}local_load_v64i1:
 define amdgpu_kernel void @local_load_v64i1(<64 x i1> addrspace(3)* %out, <64 x i1> addrspace(3)* %in) #0 {
   %load = load <64 x i1>, <64 x i1> addrspace(3)* %in
   store <64 x i1> %load, <64 x i1> addrspace(3)* %out
   ret void
 }
 
 ; FUNC-LABEL: {{^}}local_zextload_i1_to_i32:
-; GCN: ds_read_u8
-; GCN: ds_write_b32
+; GCN: ds_read_u8 [[LOAD:v[0-9]+]]
+; GCN: v_and_b32_e32 [[AND:v[0-9]+]], 1, [[LOAD]]
+; GCN: ds_write_b32 v{{[0-9]+}}, [[AND]]
 define amdgpu_kernel void @local_zextload_i1_to_i32(i32 addrspace(3)* %out, i1 addrspace(3)* %in) #0 {
   %a = load i1, i1 addrspace(3)* %in
   %ext = zext i1 %a to i32
   store i32 %ext, i32 addrspace(3)* %out
   ret void
 }
 
 ; FUNC-LABEL: {{^}}local_sextload_i1_to_i32:

test/CodeGen/AMDGPU/udiv.ll

define amdgpu_kernel void @v_udiv_i16(i32 addrspace(1)* %out, i16 addrspace(1)* %in) {
   %den = load i16, i16 addrspace(1) * %den_ptr
   %result = udiv i16 %num, %den
   %result.ext = zext i16 %result to i32
   store i32 %result.ext, i32 addrspace(1)* %out
   ret void
 }
 
 ; FUNC-LABEL: {{^}}v_udiv_i23:
+; SI: s_mov_b32 [[MASK:s[0-9]+]], 0x7fffff
+; SI: v_and_b32_e32 v{{[0-9]+}}, [[MASK]],
+; SI: v_and_b32_e32 v{{[0-9]+}}, [[MASK]],
 ; SI: v_rcp_f32
-; SI: v_and_b32_e32 [[TRUNC:v[0-9]+]], 0x7fffff, v{{[0-9]+}}
+; SI: v_and_b32_e32 [[TRUNC:v[0-9]+]], [[MASK]], v{{[0-9]+}}
 ; SI: buffer_store_dword [[TRUNC]]
 define amdgpu_kernel void @v_udiv_i23(i32 addrspace(1)* %out, i23 addrspace(1)* %in) {
   %den_ptr = getelementptr i23, i23 addrspace(1)* %in, i23 1
   %num = load i23, i23 addrspace(1) * %in
   %den = load i23, i23 addrspace(1) * %den_ptr
   %result = udiv i23 %num, %den
   %result.ext = zext i23 %result to i32
   store i32 %result.ext, i32 addrspace(1)* %out