[PATCH] D21769: [InstCombine] shrink type of sdiv if dividend is sexted and constant divisor is small enough (PR28153)

[David Majnemer via llvm-commits]

majnemer accepted this revision.
majnemer added a comment.
This revision is now accepted and ready to land.

LGTM


================
Comment at: lib/Transforms/InstCombine/InstCombineMulDivRem.cpp:1156
@@ +1155,3 @@
+    // to fit in the source type, shrink the division to the narrower type:
+    // (sext X) sdiv C --> sext (X sdiv C)
+    Value *Op0Src;
----------------
spatel wrote:
> spatel wrote:
> > majnemer wrote:
> > > Say X is i16 -32768 and C is i32 65535, the result of the sdiv will be -32768/65535 which is zero.
> > > However, trunc of C to i16 is 65535. the result of X sdiv C will be -32768/-1 which is UB because it overflows.
> > The code comment may be misleading. Your example won't fold because getMinSignedBits() for 65535 is 17. It may be clearer when you see the limits in the test cases. Any ideas about how to make that code comment more specific? Should I put your example case in there?
> Sorry if this comment wasn't clear as well. :)
> I checked in a couple of negative test cases ahead of this patch; they're the last ones in test/Transforms/InstCombine/div.ll.
I missed the MinSignedBits part of your patch, seems fine to me.  I think it'd be good to include by testcase.


http://reviews.llvm.org/D21769