<div dir="ltr">Hi Hal,<div><br></div><div>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?</div><div><br></div><div>Cheers,</div><div><br></div><div>James</div></div><br><div class="gmail_quote"><div dir="ltr">On Sun, 26 Jul 2015 at 14:10 <a href="mailto:hfinkel@anl.gov">hfinkel@anl.gov</a> <<a href="mailto:hfinkel@anl.gov">hfinkel@anl.gov</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">hfinkel added inline comments.<br>
<br>
================<br>
Comment at: lib/Analysis/ValueTracking.cpp:3345<br>
@@ +3344,3 @@<br>
+  //   (0.0 <= -0.0) ? 0.0 : -0.0 // Returns 0.0<br>
+  //   minNum(0.0, -0.0)          // May return -0.0 or 0.0 (IEEE 754-2008 5.3.1)<br>
+  // Therefore we behave conservatively and only proceed if at least one of the<br>
----------------<br>
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.<br>
<br>
<br>
================<br>
Comment at: lib/Analysis/ValueTracking.cpp:3409<br>
@@ +3408,3 @@<br>
+<br>
+  // (icmp X, Y) ? X : Y<br>
+  if (TrueVal == CmpLHS && FalseVal == CmpRHS) {<br>
----------------<br>
This comment should now say icmp/fcmp (or similar).<br>
<br>
================<br>
Comment at: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:2277<br>
@@ -2276,3 +2276,3 @@<br>
     Value *LHS, *RHS;<br>
-    SelectPatternFlavor SPF = matchSelectPattern(const_cast<User*>(&I), LHS, RHS);<br>
+    SelectPatternFlavor SPF = matchSelectPattern(const_cast<User*>(&I), LHS, RHS).Flavor;<br>
     ISD::NodeType Opc = ISD::DELETED_NODE;<br>
----------------<br>
Line is too long.<br>
<br>
================<br>
Comment at: lib/Transforms/InstCombine/InstCombineCasts.cpp:1312<br>
@@ +1311,3 @@<br>
+  //  - but only if this isn't part of a min/max operation, else we'll<br>
+  // ruin min/max canonical form.<br>
+  Value *LHS, *RHS;<br>
----------------<br>
What is the canonical form? I think it would be helpful to say (or refer to somewhere else that says).<br>
<br>
<br>
Repository:<br>
  rL LLVM<br>
<br>
<a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__reviews.llvm.org_D11146&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=UIHqWiGK4ncKrt0hW3NSWLJf3a4quf7euJw2WnEU_C0&s=2Nxn2-Txd3eW0bcZIJ58jq_hqCW8Vc8_LAaNJM44JKk&e=" rel="noreferrer" target="_blank">http://reviews.llvm.org/D11146</a><br>
<br>
<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</blockquote></div>