[AMDGPU] Fix crash when dag-combining bitcast

[ruiling]

Summary

From the code after the 'break', they are processing 64bit scalar and
vector bitcast. So I think the break-condition should be (cond1 || cond2)
This means we only execute following code if (64bit and dest-is-vector).

Also remove a previous fix which is not needed with this new fix.
(introduced in: 1349a04ef5f594dda705ec80474dda4837f26dba)

[arsenm]

llvm/test/CodeGen/AMDGPU/amdgcn.bitcast.ll

306

Does this need the constant canonicalize? I'd rather avoid relying on some specific constant folding behavior.

I'm also not sure how this produces a 64-bit value

[ruiling]

llvm/test/CodeGen/AMDGPU/amdgcn.bitcast.ll

306

I am not sure why the constant canonicalize was generated right now. The IR in the test-case will become a bitcast from fp32 constant to <1 x i32> in SelectionDAG, I cannot directly make an IR to bitcast a constantfp to <1 x i32>, that will be directly optimized off as a i32 constant after optimization. The code here checking for break condition is wrong, a bitcast from constantfp to <1 x i32> should not execute this piece of code handling 64bit bitcast. It should break-away early. But the break-condition checking expression is false because the DestVT is vector. So a 32bit bitcast goes into if (ConstantFPSDNode *C = dyn_cast<ConstantFPSDNode>(Src)). That's why I modify the break-condition check from && to ||. I think a bitcast from fp32 constant to <2 x i16> or fp32 constant cast to <4 x i8> also need such fix.

[arsenm]

LGTM. I'm slightly worried InstSimplify may get too smart and start folding out the intrinsic before it hits the DAG someday, but that probably won't happen

[ruiling]

I see. Is there other way to keep a bitcast from contantfp to i32 at LLVM IR level from being optimized off before the DAG?

[arsenm]

In D85804#2213241, @ruiling wrote:

I see. Is there other way to keep a bitcast from contantfp to i32 at LLVM IR level from being optimized off before the DAG?

I don't have any great ideas. It can be difficult to figure out how to produce operations that will produce the right type combination at the right time in the DAG

llvm/lib/Target/AMDGPU/AMDGPUISelLowering.cpp

if (DestVT.isVector()) {
             CastedElts.push_back(DAG.getNode(ISD::BITCAST, DL, DestEltVT, Elt));
           }
 
           return DAG.getBuildVector(DestVT, SL, CastedElts);
         }
       }
     }
 
-    if (DestVT.getSizeInBits() != 64 && !DestVT.isVector())
+    if (DestVT.getSizeInBits() != 64 || !DestVT.isVector())
       break;
 
     // Fold bitcasts of constants.
     //
     // v2i32 (bitcast i64:k) -> build_vector lo_32(k), hi_32(k)
     // TODO: Generalize and move to DAGCombiner
     SDValue Src = N->getOperand(0);
     if (ConstantSDNode *C = dyn_cast<ConstantSDNode>(Src)) {
-      if (Src.getValueType() == MVT::i64) {
-        SDLoc SL(N);
-        uint64_t CVal = C->getZExtValue();
-        SDValue BV = DAG.getNode(ISD::BUILD_VECTOR, SL, MVT::v2i32,
-                                 DAG.getConstant(Lo_32(CVal), SL, MVT::i32),
-                                 DAG.getConstant(Hi_32(CVal), SL, MVT::i32));
-        return DAG.getNode(ISD::BITCAST, SL, DestVT, BV);
-      }
-    }
 
     if (ConstantFPSDNode *C = dyn_cast<ConstantFPSDNode>(Src)) {
       const APInt &Val = C->getValueAPF().bitcastToAPInt();
       SDLoc SL(N);
       uint64_t CVal = Val.getZExtValue();
       SDValue Vec = DAG.getNode(ISD::BUILD_VECTOR, SL, MVT::v2i32,
                                 DAG.getConstant(Lo_32(CVal), SL, MVT::i32),
                                 DAG.getConstant(Hi_32(CVal), SL, MVT::i32));

llvm/test/CodeGen/AMDGPU/amdgcn.bitcast.ll

 ; FUNC-LABEL: {{^}}bitcast_v4f32_to_v2i64:
 ; GCN: s_buffer_load_dwordx4
 define <2 x i64> @bitcast_v4f32_to_v2i64(<2 x i64> %arg) {
   %val = call <4 x float> @llvm.amdgcn.s.buffer.load.v4f32(<4 x i32> undef, i32 0, i32 0)
   %cast = bitcast <4 x float> %val to <2 x i64>
   %div = udiv <2 x i64> %cast, %arg
   ret <2 x i64> %div
 }
+
+declare half @llvm.canonicalize.f16(half)
+
+; FUNC-LABEL: {{^}}bitcast_f32_to_v1i32:
+define amdgpu_kernel void @bitcast_f32_to_v1i32(i32 addrspace(1)* %out) {
+  %f16 = call arcp afn half @llvm.canonicalize.f16(half 0xH03F0)
+  %f32 = fpext half %f16 to float
+  %v = bitcast float %f32 to <1 x i32>
+  %v1 = extractelement <1 x i32> %v, i32 0
+  store i32 %v1, i32 addrspace(1)* %out
+  ret void
+}