[PATCH] D53205: [FPEnv][NFCI] Convert more BinaryOperator::isFNeg(...) to m_FNeg(...)

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a reviewer: lebedev.ri.
spatel added a comment.

In https://reviews.llvm.org/D53205#1267982, @cameron.mcinally wrote:

> Ah, yeah, there is a silent regression here. From BinaryOperator::isFNeg(...):
>
>   if (!IgnoreZeroSign)
>             IgnoreZeroSign = cast<Instruction>(V)->hasNoSignedZeros();
>
>
> So that code is checking the NoSignedZero FastMath flag and then ignoring the sign on a zero if found.
>
> We currently don't have that capability with the PatternMatcher functions. Those functions are fairly rigid too, so it won't be easy to add that capability. Also, it will be ugly to compensate for this shortcoming at the caller.
>
> Any suggestions on how to proceed?




1. There are no users of that "IgnoreZeroSign" optional parameter in trunk - just delete it?

  -bool BinaryOperator::isFNeg(const Value *V, bool IgnoreZeroSign) {
  +bool BinaryOperator::isFNeg(const Value *V) {



2. Within instcombine at least, it can't matter anyway. We always canonicalize: "fsub nsz 0.0, X --> fsub nsz -0.0, X".

grep for:

  // Subtraction from -0.0 is the canonical form of fneg."



3. How about adding an m_NSZ() matcher? See m_Exact() for the template. Sorry for straying from the fneg goal, but we'd be better off changing all of the nsw/nuw matchers to this format too?


Repository:
  rL LLVM

https://reviews.llvm.org/D53205