[LLVMdev] InstCombine adds bit masks, confuses self, others

[Chandler Carruth]

On Tue, Apr 17, 2012 at 1:36 PM, Rafael Espíndola <
rafael.espindola at gmail.com> wrote:

> > I am not sure how best to fix this. If possible, InstCombine's
> canonicalization shouldn't hide arithmetic progressions behind bit masks.
> At least, it seems these transformations should be disabled unless (X >>
> C).hasOneUse(). They aren't exactly optimizations.
> >
> > This:
> >
> >  %div = lshr i32 %a, 2
> >  store i32 %div, i32* %p, align 4, !tbaa !0
> >  %add = shl nuw nsw i32 %div, 1
> >
> > is better than this:
> >
> >  %div = lshr i32 %a, 2
> >  store i32 %div, i32* %p, align 4, !tbaa !0
> >  %0 = lshr i32 %a, 1
> >  %add = and i32 %0, 2147483646
>
> I think we could try your hasOneUse idea. If we are going to keep
> %div, we may as well keep using it and save one instruction in the
> canonical form.


I really dislike hasOneUse-based "solutions" at the IR / InstCombine layer.
They result in strange artifacts during optimization: places where adding
similar code turns off optimizations because we fold the similar bits
together and reuse parts of the computation.

I would much rather see us devise a reasonable set of canonicalization
rules at the IR level that don't block optimization. Tricks like saving
instructions, using bit masks, etc., should all be reserved for the codegen
layer / DAGCombine. There, hasOneUse makes perfect sense because you're
trying to peephole through architectural gymnastics, not trying to get
systematic scalar optimizations to fire.

Let's see if we have any evidence that reserving this transform for the
DAGCombiner hurts things, how it hurts things, and what we can do about it
first?
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20120417/0b5e205d/attachment.html>