[PATCH] D11146: [ValueTracking] Add support for floating-point minnum and maxnum

[James Molloy]

Hi Hal,

Sure, I'll apply those changes. Just to clarify, when you say "this is sad"
- are you referring to the deliberate ambiguity in the standard or the
conservativeness of the code I've written?

Cheers,

James

On Sun, 26 Jul 2015 at 14:10 hfinkel at anl.gov <hfinkel at anl.gov> wrote:

> hfinkel added inline comments.
>
> ================
> Comment at: lib/Analysis/ValueTracking.cpp:3345
> @@ +3344,3 @@
> +  //   (0.0 <= -0.0) ? 0.0 : -0.0 // Returns 0.0
> +  //   minNum(0.0, -0.0)          // May return -0.0 or 0.0 (IEEE
> 754-2008 5.3.1)
> +  // Therefore we behave conservatively and only proceed if at least one
> of the
> ----------------
> I feel compelled to say that this is sad. The relevant text for
> minNum/maxNum even notes, "this means results might differ among
> implementations", and this is the only place I can find in the standard
> that says that.
>
>
> ================
> Comment at: lib/Analysis/ValueTracking.cpp:3409
> @@ +3408,3 @@
> +
> +  // (icmp X, Y) ? X : Y
> +  if (TrueVal == CmpLHS && FalseVal == CmpRHS) {
> ----------------
> This comment should now say icmp/fcmp (or similar).
>
> ================
> Comment at: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:2277
> @@ -2276,3 +2276,3 @@
>      Value *LHS, *RHS;
> -    SelectPatternFlavor SPF = matchSelectPattern(const_cast<User*>(&I),
> LHS, RHS);
> +    SelectPatternFlavor SPF = matchSelectPattern(const_cast<User*>(&I),
> LHS, RHS).Flavor;
>      ISD::NodeType Opc = ISD::DELETED_NODE;
> ----------------
> Line is too long.
>
> ================
> Comment at: lib/Transforms/InstCombine/InstCombineCasts.cpp:1312
> @@ +1311,3 @@
> +  //  - but only if this isn't part of a min/max operation, else we'll
> +  // ruin min/max canonical form.
> +  Value *LHS, *RHS;
> ----------------
> What is the canonical form? I think it would be helpful to say (or refer
> to somewhere else that says).
>
>
> Repository:
>   rL LLVM
>
> http://reviews.llvm.org/D11146
>
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150726/3d6f1438/attachment.html>