[PATCH] D72643: [InstCombine] form copysign from select of FP constants (PR44153)

[Sanjay Patel via Phabricator via llvm-commits]

spatel marked 2 inline comments as done.
spatel added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp:2331
+  if (!match(TVal, m_APFloat(TC)) || !match(FVal, m_APFloat(FC)) ||
+      !abs(*TC).bitwiseIsEqual(abs(*FC)) || TC == FC)
+    return nullptr;
----------------
nikic wrote:
> spatel wrote:
> > xbolva00 wrote:
> > > spatel wrote:
> > > > nick wrote:
> > > > > I would never expect `TC == FC` to happen here. Are branches folded later than this transform?
> > > > We have to look at this 'select' independently of any other instruction because of the way instcombine processes. But I think you're correct that we can assume TC != FC because that should be simplified before we reach here. I will change that to an assert.
> > > +1 for assert
> > After I wrote that comment, I thought more about the vector case (undef lanes are always a pain!), and we can't assert yet because we're missing a simplify:
> > rG26d2ace9e230
> > 
> > And that's the motivation for:
> > D72784
> > 
> > So let me mark this patch as on hold for the moment and try to make progress on the underlying analysis.
> FWIW the m_APInt and m_APFloat matchers don't allow undefs. Only m_Zero and friends allow undefs.
This is correct. We'll have an option after D72975, but there really wasn't a reason to hold this up currently. I can change to an assert and push this.


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

https://reviews.llvm.org/D72643