[PATCH] D61516: [SelectionDAG] fold 'fneg undef' to undef

Fri May 3 10:16:59 PDT 2019

arsenm added inline comments.

================
Comment at: llvm/test/CodeGen/X86/vec_fneg.ll:52
-; X64-SSE2-NEXT:    retq
+; X64-SSE-LABEL: scalar_fsub_neg0_undef:
+; X64-SSE:       # %bb.0:
+; X64-SSE-NEXT:    retq
   %r = fsub float -0.0, undef
   ret float %r
----------------
spatel wrote:
> arsenm wrote:
> > spatel wrote:
> > > arsenm wrote:
> > > > It seems problematic that the DAG lowering is still producing an fneg for this
> > > I'm not following. Is there some place before/after getNode() that we should also fix?
> > I mean I don't expect fsub -0, x to be equivalent to fneg x anymore. This test shouldn't be hitting this path, and should be using the fneg instruction?
> Ah, I see. So we need to get our patches in order. 
> I don't think we're ready to pull the plug on SDAG converting 'fsub -0.0, x' to fneg yet because we don't have that canonicalization in IR yet, but let me know if I'm wrong.
> Either way, I should have added tests with fneg in the IR, so we don't lose coverage when we do flip that switch.
Yes, I assumed this compatibility hack was still in here somewhere, but we need to start adding tests for the pure fneg

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

https://reviews.llvm.org/D61516