[PATCH] D71467: [FPEnv] Generate constrained FP comparisons from clang

[Ulrich Weigand via Phabricator via cfe-commits]

uweigand added a comment.

In D71467#1817939 <https://reviews.llvm.org/D71467#1817939>, @rjmccall wrote:

> Is this approach going to work with scope-local strictness?  We need a way to do a comparison that has the non-strict properties but appears in a function that enables strictness elsewhere.


Well, just like for all the other FP builder methods, you can use the setIsFPConstrained method on the builder object to switch between strict and non-strict mode.   Does this not suffice, or is there anything particular about the comparisons that would require anything extra?

> Please document the difference between these two methods.

OK, checked in header file comments as 6aca3e8 <https://reviews.llvm.org/rG6aca3e8dfa228fb75e410e34db74982a0ab3939f>.

> Can you make a helper method for the common code in the non-constrained paths here?

Would you prefer something like

  private:
    Value *CreateFCmpHelper(CmpInst::Predicate P, Value *LHS, Value *RHS,
                            const Twine &Name, MDNode *FPMathTag) {
      if (auto *LC = dyn_cast<Constant>(LHS))
        if (auto *RC = dyn_cast<Constant>(RHS))
          return Insert(Folder.CreateFCmp(P, LC, RC), Name);
      return Insert(setFPAttrs(new FCmpInst(P, LHS, RHS), FPMathTag, FMF), Name);
    }
  
  public:
    Value *CreateFCmp(CmpInst::Predicate P, Value *LHS, Value *RHS,
                      const Twine &Name = "", MDNode *FPMathTag = nullptr) {
      if (IsFPConstrained)
        return CreateConstrainedFPCmp(Intrinsic::experimental_constrained_fcmp,
                                      P, LHS, RHS, Name);
  
      return CreateFCmpHelper(P, LHS, RHS, Name, FPMathTag);
    }
    [...]

or rather something like:

  private:
    Value *CreateFCmpHelper(CmpInst::Predicate P, Value *LHS, Value *RHS,
                            bool IsSignaling, const Twine &Name, MDNode *FPMathTag) {
      if (IsFPConstrained)
        return CreateConstrainedFPCmp(IsSignaling ? Intrinsic::experimental_constrained_fcmps
                                                  : Intrinsic::experimental_constrained_fcmp,
                                      P, LHS, RHS, Name);
  
      if (auto *LC = dyn_cast<Constant>(LHS))
        if (auto *RC = dyn_cast<Constant>(RHS))
          return Insert(Folder.CreateFCmp(P, LC, RC), Name);
      return Insert(setFPAttrs(new FCmpInst(P, LHS, RHS), FPMathTag, FMF), Name);
    }
  
  public:
    Value *CreateFCmp(CmpInst::Predicate P, Value *LHS, Value *RHS,
                      const Twine &Name = "", MDNode *FPMathTag = nullptr) {
      return CreateFCmpHelper(P, LHS, RHS, false, Name, FPMathTag);
    }
    [...]

or maybe simply have CreateFCmpS call CreateFCmp directly in the non-strict case?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71467