[AMDGPU] Fix clamp bit DAG operand

[hliao]

Summary

• Should use targetconstant instead of constant operand for clamp bit, which is expected as an immediate operand. Under certain conditions, such as a common i1 false constant is used in other place and selected before the instruction with clamp bit, register operand may be added instead of immediate one. Use targetcosntant to enforce that.

[arsenm]

Duplicate of D59556

[hliao]

Ah, yes, but this patch covers more cases. Shall we merge them together.

[tpr]

Thanks Michael; that is much better than my fix in D59556. Let's go with this one and I'll abandon my one.

[arsenm]

llvm/lib/Target/AMDGPU/AMDGPUISelDAGToDAG.cpp

1124–1125 ↗

(On Diff #191561)

This is missing a test case

llvm/test/CodeGen/AMDGPU/uaddo.ll

176 ↗

(On Diff #191561)

This should really use VGPR inputs

178 ↗

(On Diff #191561)

Should also cover sub version

[hliao]

Thanks. Done!

[hliao]

sorry, saw this too late, will address in another patch

llvm/lib/Target/AMDGPU/AMDGPUISelDAGToDAG.cpp

1124–1125 ↗

(On Diff #191561)

Sorry, saw this too late, I will address this in a later patch. OK?

llvm/test/CodeGen/AMDGPU/uaddo.ll

178 ↗

(On Diff #191561)

sure, will address that in a later patch

[arsenm]

In D59608#1436942, @hliao wrote:

sorry, saw this too late, will address in another patch

ping

[hliao]

In D59608#1449894, @arsenm wrote:

In D59608#1436942, @hliao wrote:

sorry, saw this too late, will address in another patch

ping

sorry, forget posting them, here's the link, https://reviews.llvm.org/D60071

llvm/trunk/lib/Target/AMDGPU/AMDGPUISelDAGToDAG.cpp

 
 void AMDGPUDAGToDAGISel::SelectUADDO_USUBO(SDNode *N) {
   // The name of the opcodes are misleading. v_add_i32/v_sub_i32 have unsigned
   // carry out despite the _i32 name. These were renamed in VI to _U32.
   // FIXME: We should probably rename the opcodes here.
   unsigned Opc = N->getOpcode() == ISD::UADDO ?
     AMDGPU::V_ADD_I32_e64 : AMDGPU::V_SUB_I32_e64;
 
-  CurDAG->SelectNodeTo(N, Opc, N->getVTList(),
-                       {N->getOperand(0), N->getOperand(1),
-                        CurDAG->getConstant(0, {}, MVT::i1)/*clamp bit*/});
+  CurDAG->SelectNodeTo(
+      N, Opc, N->getVTList(),
+       CurDAG->getTargetConstant(0, {}, MVT::i1) /*clamp bit*/});
 }
 
 void AMDGPUDAGToDAGISel::SelectFMA_W_CHAIN(SDNode *N) {
   SDLoc SL(N);
   //  src0_modifiers, src0,  src1_modifiers, src1, src2_modifiers, src2, clamp, omod
   SDValue Ops[10];
 
   SelectVOP3Mods0(N->getOperand(1), Ops[1], Ops[0], Ops[6], Ops[7]);
if (const ConstantSDNode *C = dyn_cast<ConstantSDNode>(Addr.getOperand(0))) {
           SmallVector<SDValue, 3> Opnds;
           Opnds.push_back(Zero);
           Opnds.push_back(Addr.getOperand(1));
 
           // FIXME: Select to VOP3 version for with-carry.
           unsigned SubOp = AMDGPU::V_SUB_I32_e32;
           if (Subtarget->hasAddNoCarry()) {
             SubOp = AMDGPU::V_SUB_U32_e64;
-            Opnds.push_back(Zero); // clamp bit
+            Opnds.push_back(
+                CurDAG->getTargetConstant(0, {}, MVT::i1)); // clamp bit
           }
 
           MachineSDNode *MachineSub =
               CurDAG->getMachineNode(SubOp, DL, MVT::i32, Opnds);
 
           Base = SDValue(MachineSub, 0);
           Offset = CurDAG->getTargetConstant(ByteOffset, DL, MVT::i16);
           return true;
if (const ConstantSDNode *C = dyn_cast<ConstantSDNode>(Addr.getOperand(0))) {
 
         if (isDSOffsetLegal(Sub, DWordOffset1, 8)) {
           SmallVector<SDValue, 3> Opnds;
           Opnds.push_back(Zero);
           Opnds.push_back(Addr.getOperand(1));
           unsigned SubOp = AMDGPU::V_SUB_I32_e32;
           if (Subtarget->hasAddNoCarry()) {
             SubOp = AMDGPU::V_SUB_U32_e64;
-            Opnds.push_back(Zero); // clamp bit
+            Opnds.push_back(
+                CurDAG->getTargetConstant(0, {}, MVT::i1)); // clamp bit
           }
 
           MachineSDNode *MachineSub
             = CurDAG->getMachineNode(SubOp, DL, MVT::i32, Opnds);
 
           Base = SDValue(MachineSub, 0);
           Offset0 = CurDAG->getTargetConstant(DWordOffset0, DL, MVT::i8);
           Offset1 = CurDAG->getTargetConstant(DWordOffset1, DL, MVT::i8);

llvm/trunk/test/CodeGen/AMDGPU/uaddo.ll

define amdgpu_kernel void @v_uaddo_v2i32(<2 x i32> addrspace(1)* %out, <2 x i32> addrspace(1)* %carryout, <2 x i32> addrspace(1)* %aptr, <2 x i32> addrspace(1)* %bptr) nounwind {
   %val = extractvalue { <2 x i32>, <2 x i1> } %sadd, 0
   %carry = extractvalue { <2 x i32>, <2 x i1> } %sadd, 1
   store <2 x i32> %val, <2 x i32> addrspace(1)* %out, align 4
   %carry.ext = zext <2 x i1> %carry to <2 x i32>
   store <2 x i32> %carry.ext, <2 x i32> addrspace(1)* %carryout
   ret void
 }
 
+; FUNC-LABEL: {{^}}s_uaddo_clamp_bit:
+; GCN: v_add_{{i|u|co_u}}32_e32
+; GCN: s_endpgm
+define amdgpu_kernel void @s_uaddo_clamp_bit(i32 addrspace(1)* %out, i1 addrspace(1)* %carryout, i32 %a, i32 %b) #0 {
+entry:
+  %uadd = call { i32, i1 } @llvm.uadd.with.overflow.i32(i32 %a, i32 %b)
+  %val = extractvalue { i32, i1 } %uadd, 0
+  %carry = extractvalue { i32, i1 } %uadd, 1
+  %c2 = icmp eq i1 %carry, false
+  %cc = icmp eq i32 %a, %b
+  br i1 %cc, label %exit, label %if
+
+if:
+  br label %exit
+
+exit:
+  %cout = phi i1 [false, %entry], [%c2, %if]
+  store i32 %val, i32 addrspace(1)* %out, align 4
+  store i1 %cout, i1 addrspace(1)* %carryout
+  ret void
+}
+
 
 declare i32 @llvm.amdgcn.workitem.id.x() #1
 declare { i16, i1 } @llvm.uadd.with.overflow.i16(i16, i16) #1
 declare { i32, i1 } @llvm.uadd.with.overflow.i32(i32, i32) #1
 declare { i64, i1 } @llvm.uadd.with.overflow.i64(i64, i64) #1
 declare { <2 x i32>, <2 x i1> } @llvm.uadd.with.overflow.v2i32(<2 x i32>, <2 x i32>) nounwind readnone
 
 
 attributes #0 = { nounwind }
 attributes #1 = { nounwind readnone }