[PATCH] D8304: Fix backward operands in call to isTruncateFree() and improve comments.

[hfinkel@anl.gov via llvm-commits]

hfinkel added a subscriber: hfinkel.
hfinkel accepted this revision.
hfinkel added a reviewer: hfinkel.
hfinkel added a comment.
This revision is now accepted and ready to land.

This LGTM. If you have a test case, great, otherwise you can commit without one.


================
Comment at: include/llvm/Target/TargetLowering.h:1561-1562
@@ -1559,6 +1560,4 @@
   /// This does not necessarily include registers defined in unknown ways, such
   /// as incoming arguments, or copies from unknown virtual registers. Also, if
-  /// isTruncateFree(Ty2, Ty1) is true, this does not necessarily apply to
-  /// truncate instructions. e.g. on x86-64, all instructions that define 32-bit
-  /// values implicit zero-extend the result out to 64 bits.
-  virtual bool isZExtFree(Type * /*Ty1*/, Type * /*Ty2*/) const {
+  /// isTruncateFree(FromTy, ToTy) is true, this does not necessarily apply to
+  /// truncate instructions, e.g. on x86_64, all instructions that define 32-bit
----------------
srking wrote:
> jroelofs wrote:
> > Now that I've read it again, I don't get what it's trying to say either... I can kind of see it making sense if the operands were accidentally swapped.
> Ya, I tried for a while to assimilate this one.  Confusing comments are often worse than nothing.  If you aprove, I will update the patch to delete this comment.
Exactly what would you want to delete? Perhaps we should just reword this to say that the function should return true when it is likely that the truncate can be freely folded with an instruction defining a value of FromTy. If the defining instruction is unknown (because you're looking at a function argument, PHI, etc.) then you may need an explicit truncate, and that's not necessarily free (but this function does not deal with those cases).



http://reviews.llvm.org/D8304