[PATCH] D25913: [InstCombine] Fold nuw left-shifts in `ugt`/`ule` comparisons.

bryant via llvm-commits llvm-commits at lists.llvm.org
Mon Oct 24 14:28:58 PDT 2016


bryant added inline comments.


================
Comment at: test/Transforms/InstCombine/icmp-shl-zext-simplify.ll:37
+; CHECK-LABEL: @icmp_ule_i64x2(
+; CHECK-NEXT:    [[TMP2:%.*]] = trunc <2 x i64> %0 to <2 x i48>
+; CHECK-NEXT:    [[D:%.*]] = icmp eq <2 x i48> [[TMP2]], zeroinitializer
----------------
spatel wrote:
> bryant wrote:
> > This `trunc` bothers me. On X86, I think it generates an extra `pand`:
> > 
> > ```
> > .LCPI3_0:
> >         .short  65535                   # 0xffff
> >         .short  65535                   # 0xffff
> >         .short  65535                   # 0xffff
> >         .short  0                       # 0x0
> >         .short  65535                   # 0xffff
> >         .short  65535                   # 0xffff
> >         .short  65535                   # 0xffff
> >         .short  0                       # 0x0
> >         .text
> >         .globl  icmp_ule_i64x2
> >         .p2align        4, 0x90
> >         .type   icmp_ule_i64x2, at function
> > icmp_ule_i64x2:                         # @icmp_ule_i64x2
> >         .cfi_startproc
> > # BB#0:
> >         pand    .LCPI3_0(%rip), %xmm0      # <=======
> >         pxor    %xmm1, %xmm1
> >         pcmpeqd %xmm0, %xmm1
> >         pshufd  $177, %xmm1, %xmm0      # xmm0 = xmm1[1,0,3,2]
> >         pand    %xmm1, %xmm0
> >         retq
> > ```
> > 
> > The same goes for `icmp_ule_64`, but no extra X86 op is generated in that case.
> I noticed that example too and agree it doesn't look like a good fold. We shouldn't be creating illegal types in InstCombine without a fallback in a later pass to make sure the backend is not harmed. 
> 
> But this is the trunc fold above yours that is firing, right? Ie, this test does not change with your patch.
> Please add a vector test that is affected by your patch. 
> 
> You can add all of the tests with the current output based on trunk, then rebase this patch after that commit, so we'll see just the diffs in the resulting IR.
I've already moved this patch above the trunc fold, though.


Repository:
  rL LLVM

https://reviews.llvm.org/D25913





More information about the llvm-commits mailing list