[PATCH] D129958: [IndVars] Eliminate redundant type cast with different sizes

[Allen zhong via Phabricator via llvm-commits]

Allen added a comment.

In D129958#3709096 <https://reviews.llvm.org/D129958#3709096>, @nikic wrote:

> I don't think the last update changed any code?

sorry, I upload with an old version, and now it is ok.



================
Comment at: llvm/lib/Transforms/Utils/SimplifyIndVar.cpp:713
+          Conv = Builder.CreateTrunc(IVOperand, CI->getType(), Name + ".trunc");
+        } else if (Opcode == CastInst::FPToSI) {
+          Conv = Builder.CreateSExt(IVOperand, CI->getType(), Name + ".sext");
----------------
nikic wrote:
> I believe we only need sext if both fptosi and sitofp are used. If one of them is unsigned, then we can use zext. This would match InstCombine logic.
> 
> In any case, we should have a test for the uitofp+fptosi combination.
Done, thanks @nikic very much!


================
Comment at: llvm/test/Transforms/IndVarSimplify/floating-point-small-iv.ll:181
+
+define void @sitofp_fptoui_range_zext_postinc() {
+;
----------------
nikic wrote:
> This isn't quite what I had in mind. By "postinc IV" I mean that `%inc.int` gets passed to sitofp, rather than `%iv.int`. I think your current code may result in a wrong insertion point for this case.
Now, I just update code to fix the crash for the case with "postinc IV", and I'll try to improve it next patch as I need some time to get the right insertion point.


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

https://reviews.llvm.org/D129958