[PATCH] D34300: [AMDGPU] simplify add x, *ext (setcc) => addc|subb x, 0, setcc

[Stanislav Mekhanoshin via Phabricator via llvm-commits]

rampitec added inline comments.


================
Comment at: lib/Target/AMDGPU/AMDGPUInstrInfo.td:186
 
+def AMDGPUadde : SDNode<"ISD::ADDCARRY", AMDGPUAddeSubeOp, []>;
+
----------------
arsenm wrote:
> Weird that the td generic nodes don't exist already for this. Looks like this was added only in April, so you should probably add the generic addcarry/subcarry to TargetSelectionDAG.td
I'm not sure really. The intent is to gradually switch from addc/subc, so probably new td defs were not assumed.
I think we need to use our nodes for now and remove it in the future. I can add todo here.


================
Comment at: lib/Target/AMDGPU/SIISelLowering.cpp:4866
+    auto Cond = RHS.getOperand(0);
+    if (Cond.getOpcode() == ISD::SETCC) {
+      SDVTList VTList = DAG.getVTList(MVT::i32, MVT::i1);
----------------
arsenm wrote:
> This is likely any i1 source. We have some other intrinsics that return i1 values, so maybe a todo?
I have initially tried to use it with just any i1 input. That is worse though.
In case if we have i1 argument to a function for example we would create an extra compare if I just perform it with any i1. It looks like it is only beneficial in some cases, like setcc.


Repository:
  rL LLVM

https://reviews.llvm.org/D34300