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

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

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

> Just thinking aloud. I really don't have enough experience with this framework to say for sure...
>
> This hasNoSignedZeros(...) function is pretty rigid:
>
>   bool Instruction::hasNoSignedZeros() const {
>     assert(isa<FPMathOperator>(this) && "getting fast-math flag on invalid op");
>     return cast<FPMathOperator>(this)->hasNoSignedZeros();
>   }
>
>
> So this will assert if the class isn't an FPMathOperator. Maybe this function (and friends) should be relaxed to return false if it's not an FPMathOperator?
>
> That way our explicit code wouldn't be so verbose. E,g.:
>
>   if (!BinaryOperator::isNot(I) && !BinaryOperator::isNeg(I) &&
>       !(match(I, m_FNeg(m_Value()))) &&
>       !(I->hasNoSignedZeros() && match(I, m_FNegNSZ(m_Value()))))
>     ++Rank;
>
>
> *Note: that could probably be cleaned up more. Just a rough example.


Ok, I may not be seeing the problem correctly, but let me try 1 more suggestion. What if we adjust the regular m_FNeg() definition to look for 'nsz' on the op itself:

  Index: include/llvm/IR/PatternMatch.h
  ===================================================================
  --- include/llvm/IR/PatternMatch.h	(revision 344898)
  +++ include/llvm/IR/PatternMatch.h	(working copy)
  @@ -659,11 +659,32 @@
     return BinaryOp_match<LHS, RHS, Instruction::FSub>(L, R);
   }
   
  +template <typename Op_t> struct FNeg_match {
  +  Op_t X;
  +
  +  FNeg_match(const Op_t &Op) : X(Op) {}
  +  template <typename OpTy> bool match(OpTy *V) {
  +    auto *FPMO = dyn_cast<FPMathOperator>(V);
  +    if (!FPMO || FPMO->getOpcode() != Instruction::FSub)
  +      return false;
  +    if (FPMO->hasNoSignedZeros()) {
  +      // With 'nsz', any zero goes.
  +      if (!cstfp_pred_ty<is_any_zero_fp>().match(FPMO->getOperand(0)))
  +        return false;
  +    } else {
  +      // Without 'nsz', we need fsub -0.0, X exactly.
  +      if (!cstfp_pred_ty<is_neg_zero_fp>().match(FPMO->getOperand(0)))
  +        return false;
  +    }
  +    return X.match(FPMO->getOperand(1));
  +  }
  +};
  +
   /// Match 'fneg X' as 'fsub -0.0, X'.
  -template <typename RHS>
  -inline BinaryOp_match<cstfp_pred_ty<is_neg_zero_fp>, RHS, Instruction::FSub>
  -m_FNeg(const RHS &X) {
  -  return m_FSub(m_NegZeroFP(), X);
  +template <typename OpTy>
  +inline FNeg_match<OpTy>
  +m_FNeg(const OpTy &X) {
  +  return FNeg_match<OpTy>(X);
   }
   
   /// Match 'fneg X' as 'fsub +-0.0, X'.


Repository:
  rL LLVM

https://reviews.llvm.org/D53205