[PATCH] D12965: [InstCombine] shrink switch conditions better (PR24766)

[hfinkel@anl.gov via llvm-commits]

hfinkel added a comment.

In http://reviews.llvm.org/D12965#250155, @spatel wrote:

> In http://reviews.llvm.org/D12965#249222, @hfinkel wrote:
>
> > Okay, but I'm still hesitant to generate unnecessary instructions. Do we still end up with the unnecessary zext instructions with this patch?
>
>
> Yes, we're producing unnecessary zexts. Based on that comment in InstCombine, I thought this was just a quirk of x86 (possibly related to bug 22473), but it's a target-independent problem for SelectionDAGBuilder::visitSwitch(). It causes unnecessary mask instructions for PPC and ARM64 too:
>
>   Optimized lowered selection DAG: BB#8 'positive1:entry'
>            t0: i32 = Register %vreg3
>          t1: i32,ch = CopyFromReg t0, t0
>        t2: i16 = truncate t1
>        t3: i16 = Constant<1001>
>        t4: ch = seteq
>      t5: i1 = setcc t2, t3, t4
>   
>   Type-legalized selection DAG: BB#8 'positive1:entry'
>              t0: i32 = Register %vreg3
>            t1: i32,ch = CopyFromReg t0, t0
>            t15: i32 = Constant<65535>
>          t16: i32 = and t1, t15
>          t10: i32 = Constant<1001>
>          t4: ch = seteq
>        t11: i32 = setcc t16, t10, t4
>   
>
> I think the root problem is that I'm mixing up the legal types specified by the datalayout with the codegen definition of legality based on the register width. So I'm not sure if we should just hack around this limitation here in InstCombine's visitSwitchInst() or shrink as much as possible here without regard for legality and then fix it up in CodeGenPrepare?


On theoretical grounds, I favor the latter. We should shrink aggressively for our canonical form because smaller types convey the most information at the IR level; we tend to lose information when we promote. If we can do this, and then fix it up later based on actual target-type-legality information later, I think that's better.


http://reviews.llvm.org/D12965