[PATCH] D44424: [InstCombine] peek through unsigned FP casts for zero-equality compares (PR36682)

[Sanjay Patel via Phabricator via llvm-commits]

spatel added inline comments.


================
Comment at: lib/Transforms/InstCombine/InstCombineCompares.cpp:4516
+      return new ICmpInst(Pred, X, ConstantInt::getNullValue(X->getType()));
+  }
+
----------------
lebedev.ri wrote:
> spatel wrote:
> > Don't need braces for the single-line 'if' clause.
> It occupies more than one line though.
> But ok, if you insist.
It's a fuzzy rule, so I was just going by the formatting of the code above this.


================
Comment at: test/Transforms/InstCombine/cast-unsigned-icmp-eqcmp-0.ll:4
 
 ; target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 
----------------
lebedev.ri wrote:
> spatel wrote:
> > Remove unnecessary comment line.
> For future reference, could you please clarify *when* `target datalayout` //should// be specified in tests?
Sure - it should be specified when it affects the outcome. For instcombine, this would be when endianness or type legality is used to determine if/how a transform fires. So you'd see some query of the datalayout in the code.

Note that in this case, the line is commented out, so it definitely doesn't add anything to the test....unless I've missed some reason for it to be specified as a comment?


Repository:
  rL LLVM

https://reviews.llvm.org/D44424