[PATCH] D26556: [InstCombine] don't widen most selects by hoisting an extend

[Sanjay Patel via llvm-commits]

On 2nd thought, any time we have a transform loophole like this and we
really want to do the opposite transform, we run the risk of infinite
looping. Let me look at adding folds that start with sext/zext, so we know
we're catching the trunc/select/ext cases exactly without needing this
extra check.

On Wed, Nov 16, 2016 at 12:50 AM, Filipe Cabecinhas <
filcab+llvm.phabricator at gmail.com> wrote:

> Yeah, a TODO is enough for now. This patch makes it strictly better,
> so I'm good with it.
>
> Thank you,
>  Filipe
>
> On Wed, Nov 16, 2016 at 12:09 AM, Sanjay Patel <spatel at rotateright.com>
> wrote:
> > spatel added inline comments.
> >
> >
> > ================
> > Comment at: lib/Transforms/InstCombine/InstructionCombining.cpp:805
> > +  auto isNonTruncVariable = [](Value *V) {
> > +    return !isa<Constant>(V) && !match(V, m_Trunc(m_Value()));
> > +  };
> > ----------------
> > filcab wrote:
> >> Do you want to match type sizes, though? Or at least make sure you're
> truncating more (or the same) as you're extending?
> >> Like this:
> >> ```[build-debug]% cat | ./bin/opt -O3 - -o - -S
> >> define <4 x i64> @g3vec(<4 x i32> %_a, <4 x i1> %cmp) {
> >>   %a = trunc <4 x i32> %_a to <4 x i24>
> >>   %sel = select <4 x i1> %cmp, <4 x i24> %a, <4 x i24> <i24 42, i24 42,
> i24 42, i24 42>
> >>   %ext = zext <4 x i24> %sel to <4 x i64>
> >>   ret <4 x i64> %ext
> >> }
> >>
> >>
> >> ; ModuleID = '<stdin>'
> >> source_filename = "<stdin>"
> >>
> >> ; Function Attrs: norecurse nounwind readnone
> >> define <4 x i64> @g3vec(<4 x i32> %_a, <4 x i1> %cmp)
> local_unnamed_addr #0 {
> >>   %1 = and <4 x i32> %_a, <i32 16777215, i32 16777215, i32 16777215,
> i32 16777215>
> >>   %2 = zext <4 x i32> %1 to <4 x i64>
> >>   %ext = select <4 x i1> %cmp, <4 x i64> %2, <4 x i64> <i64 42, i64 42,
> i64 42, i64 42>
> >>   ret <4 x i64> %ext
> >> }
> >>
> >> attributes #0 = { norecurse nounwind readnone }
> >> ```
> >> vs just `select` + `zext` (using `sext` will make it even worse :-)
> >>
> > This case would be another improvement over the current behavior, right?
> Ok if I add a 'TODO' comment in this patch and follow up with another test
> case plus that refinement?
> >
> >
> > https://reviews.llvm.org/D26556
> >
> >
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161116/0efcd245/attachment.html>