[PATCH] D60633: [AMDGPU] Avoid DAG combining assert with fneg(fadd(A,0))

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added a comment.

In D60633#1466195 <https://reviews.llvm.org/D60633#1466195>, @tpr wrote:

> I have cut down the test a bit more and put it into fneg-combines.ll. I did not manage to repro any problems with the other cases that I added fixes for.
>
> I don't have a massive amount of time to spend on this. Would you prefer I land it as is, without tests for the other cases, or remove the precautionary fix for those cases and wait for someone else to possibly encounter them?


In either case, you should open a bug somewhere to look at this again.



================
Comment at: lib/Target/AMDGPU/AMDGPUISelLowering.cpp:3715-3716
     SDValue Res = DAG.getNode(ISD::FADD, SL, VT, LHS, RHS, N0->getFlags());
+    if (Res.getOpcode() == ISD::FNEG)
+      return SDValue(); // Op got folded away.
     if (!N0.hasOneUse())
----------------
I think it would be slightly safer to check != ISD::FADD, in case some other fold decided to do something else


================
Comment at: test/CodeGen/AMDGPU/fneg-combines.ll:219
+; GCN-LABEL: {{^}}fneg_fadd_0:
+
+define amdgpu_ps float @fneg_fadd_0(float inreg %tmp2, float inreg %tmp6, <4 x i32> %arg) local_unnamed_addr #0 {
----------------
Should check something


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60633/new/

https://reviews.llvm.org/D60633