[PATCH] D128647: [InstructionSimplify] handle denormal constant input for fcmp

[ChenZheng via Phabricator via llvm-commits]

shchenz added inline comments.


================
Comment at: llvm/include/llvm/Analysis/ConstantFolding.h:70-74
 /// ConstantFoldCompareInstOperands - Attempt to constant fold a compare
 /// instruction (icmp/fcmp) with the specified operands.  If it fails, it
 /// returns a constant expression of the specified operands.
-///
-Constant *
-ConstantFoldCompareInstOperands(unsigned Predicate, Constant *LHS,
-                                Constant *RHS, const DataLayout &DL,
-                                const TargetLibraryInfo *TLI = nullptr);
+/// For fcmp, denormal constant input will be folded further according to the
+/// denormal handling mode.
----------------
spatel wrote:
> Since we are updating this comment, please remove the function name at the start:
> "Don’t duplicate function or class name at the beginning of the comment."
> https://llvm.org/docs/CodingStandards.html#doxygen-use-in-documentation-comments
> 
> Also, change the wording slightly:
>   For fcmp, denormal inputs may be flushed based on the denormal handling mode.
Thanks for remind.


================
Comment at: llvm/test/Transforms/InstSimplify/constant-fold-fp-denormal.ll:773
 
 define i1 @fcmp_float_positive_zero() #6 {
 ; CHECK-LABEL: @fcmp_float_positive_zero(
----------------
spatel wrote:
> spatel wrote:
> > This is identical to the previous test?
> Is this still the same as the previous test? It would be better to make the denorm constant as operand 0 and maybe vary the fcmp predicate for better coverage.
Sorry, the attribute here should be `#4`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128647