[PATCH] D57066: [TargetLowering] try harder to determine undef elements of vector binops

[Simon Pilgrim via Phabricator via llvm-commits]

RKSimon accepted this revision.
RKSimon added a comment.
This revision is now accepted and ready to land.

LGTM



================
Comment at: lib/CodeGen/SelectionDAG/TargetLowering.cpp:1472
+    if (C0 && C1 && C0.getValueType() == EltVT && C1.getValueType() == EltVT)
+      if (DAG.getNode(BO.getOpcode(), SDLoc(BO), EltVT, C0, C1).isUndef())
+        KnownUndef.setBit(i);
----------------
spatel wrote:
> RKSimon wrote:
> > What's the policy on creating nodes on the fly like this?
> I don't think it's stated anywhere, but we don't want to do it in general. 
> 
> I've seen cases in x86 lowering where a temp *variable* node is created speculatively and that inhibits other folds because it changes the number of uses of other values.
> 
> In this case, I've tried to ensure that we are only creating a constant or undef, so I could assert that to see if we escaped that assumption.
> 
> I may be missing some alternate means, but if we use FoldConstantArithmetic() directly, then we'll have to deal with FP separately because there's no equivalent/unified call for folding FP math. AFAIK, that's done directly within getNode().
OK, please can you add a TODO comment.


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

https://reviews.llvm.org/D57066