[AMDGPU] Fix bitcast v4i64/v16i16

[piotr]

Summary

Fix a regression introduced in D128865.

[piotr]

While this fixes a regression in an important benchmark, I think we need to re-think how these BitConverts are specified in a follow-up work, as the current list has been highly inconsistent, even before my changes.

I guess, the list has grown organically from the cases that isel needs supporting. Does anyone know if we should be listing all possible cases of a bitcast for a given width? And always have both SReg/VReg? Or it's fine to have a list like specified currently?

[arsenm]

llvm/test/CodeGen/AMDGPU/bitcast-v4i64-v16i16.ll

30–31

Don't you need just this part and the other stores? Why the control flow?

[piotr]

llvm/test/CodeGen/AMDGPU/bitcast-v4i64-v16i16.ll

30–31

Looks the failing pattern is very sensitive to the IR coming to the isel. I couldn't repro this with only one bitcast either.

[arsenm]

llvm/test/CodeGen/AMDGPU/bitcast-v4i64-v16i16.ll

30–31

Merge with amdgcn.bitcast.ll?

I think the trick is to have the value defined in a different block and to not have it sink. You can reduce the block count

[piotr]

Good tip, thanks! Added a couple of tests in bitcast.ll, and reverted an erroneous edit from the original commit that the tests uncovered.

llvm/lib/Target/AMDGPU/SIInstructions.td

 def : BitConvert <v16f16, v8i32, VReg_256>;
 def : BitConvert <v16i16, v8i32, VReg_256>;
 def : BitConvert <v16f16, v8f32, VReg_256>;
 def : BitConvert <v16i16, v8f32, VReg_256>;
 def : BitConvert <v8i32, v16f16, VReg_256>;
 def : BitConvert <v8i32, v16i16, VReg_256>;
 def : BitConvert <v8f32, v16f16, VReg_256>;
 def : BitConvert <v8f32, v16i16, VReg_256>;
+def : BitConvert <v16f16, v4i64, VReg_256>;
+def : BitConvert <v16i16, v4i64, VReg_256>;
+def : BitConvert <v16f16, v4f64, VReg_256>;
+def : BitConvert <v16i16, v4f64, VReg_256>;
+def : BitConvert <v4i64, v16f16, VReg_256>;
+def : BitConvert <v4i64, v16i16, VReg_256>;
+def : BitConvert <v4f64, v16f16, VReg_256>;
+def : BitConvert <v4f64, v16i16, VReg_256>;
 
 // 512-bit bitcast
 def : BitConvert <v16i32, v16f32, VReg_512>;
 def : BitConvert <v16f32, v16i32, VReg_512>;
 def : BitConvert <v8i64,  v8f64,  VReg_512>;
 def : BitConvert <v8f64,  v8i64,  VReg_512>;
 def : BitConvert <v8i64,  v16i32, VReg_512>;
 def : BitConvert <v8f64,  v16i32, VReg_512>;

llvm/test/CodeGen/AMDGPU/bitcast-v4i64-v16i16.ll

+; RUN: llc -global-isel=0 -march=amdgcn -mcpu=gfx900 -verify-machineinstrs < %s | FileCheck %s
+; RUN: llc -global-isel=1 -march=amdgcn -mcpu=gfx900 -verify-machineinstrs < %s | FileCheck %s
+
+; CHECK-LABEL: {{^}}bitcast_4x64_16i16:
+; CHECK: global_store_dwordx4
+; CHECK: global_store_dwordx4
+
+define amdgpu_kernel void @bitcast_4x64_16i16(<16 x i16> addrspace(1)* %out) {
+entry:
+  br i1 undef, label %for.body29, label %for.cond60.preheader
+
+for.cond60.preheader:                             ; preds = %if.end.3, %cond.end
+  %tmp.sroa.0.1.lcssa = phi <4 x i64> [ zeroinitializer, %entry ], [ %i7, %if.end.3 ]
+  %i5 = bitcast <4 x i64> %tmp.sroa.0.1.lcssa to <16 x i16>
+  store <16 x i16> %i5, <16 x i16> addrspace(1)* %out, align 32
+  br label %if.end76.2
+
+for.body29:                                       ; preds = %if.end.3, %for.body29.preheader
+  %tmp.sroa.0.153 = phi <4 x i64> [ %i7, %if.end.3 ], [ zeroinitializer, %entry ]
+  %i7 = insertelement <4 x i64> %tmp.sroa.0.153, i64 0, i64 0
+  br label %if.end.3
+
+if.end.3:                                         ; preds = %for.body29
+  br i1 undef, label %for.body29, label %for.cond60.preheader
+
+if.end76.2:                                       ; preds = %for.cond60.preheader
+  br label %if.then69.3
+
+if.then69.3:                                      ; preds = %if.end76.2
+  %i11 = bitcast <4 x i64> %tmp.sroa.0.1.lcssa to <16 x i16>
+  store <16 x i16> %i11, <16 x i16> addrspace(1)* %out, align 32
+  unreachable
+}