On Tue, 3 Oct 2017, Jeff Law wrote:
> On 10/03/2017 03:00 PM, Marc Glisse wrote:
> > On Tue, 3 Oct 2017, Jakub Jelinek wrote:
> >
> >> On Tue, Oct 03, 2017 at 12:54:39PM -0700, Prathamesh Kulkarni wrote:
> >>> Hi,
> >>> This follow-up patch implements the patterns mentioned in $subject.
> >>> Bootstrap+test in progress on x86_64-unknown-linux-gnu and
> >>> aarch64-linux-gnu.
> >>> OK to commit if passes ?
> >>>
> >>> Thanks,
> >>> Prathamesh
> >>
> >>> 2017-10-03 Prathamesh Kulkarni <prathamesh.kulka...@linaro.org>
> >>>
> >>> * match.pd ((X >> CST) == 0 -> X < (1 << CST)): New pattern.
> >>> ((X >> CST) != 0 -> X >= (1 << CST)): Likewise.
> >
> > build_int_cstu doesn't work for vectors, you want build_one_cst. I never
> > know if we should check single_use or not :-(
> >
> >>> testsuite/
> >>> * gcc.dg/tree-ssa/cmpdiv.c: Add test-cases f3 and f4.
> >>
> >> Why this way and not the other way around?
> >
> > For high level gimple optimizations, X < CST is more convenient (and
> > smaller, just one insn) than (X >> CST) == 0.
> Right. One could easily argue that Prathamesh's form should be the
> preferred form because it is simpler at the gimple level -- and that
> x86-isms should be dealt with later in the pipeline.
>
>
> >
> >> E.g. i?86/x86_64 and various other targets have shift instructions which
> >> set condition codes, so (X >> 51) == 0 is certainly smaller
> >> smaller and I believe cheaper than the latter.
> >> Try:
> >> void foo (void);
> >>
> >> void
> >> bar (unsigned long long x)
> >> {
> >> if ((x >> 51) == 0)
> >> foo ();
> >> }
> >>
> >> void
> >> baz (unsigned long long x)
> >> {
> >> if (x < (1LL << 51))
> >> foo ();
> >> }
> >> with -Os on x86_64, the first function is 4 insns, 12 bytes,
> >> the second one 5 insns, 21 bytes.
> >>
> >> I wonder if this kind of instruction selection stuff shouldn't be
> >> done in target.pd instead, with input from the target.
> Right, but I think that argues that Prathamesh's patch is the right
> direction and that to move forward what needs to happen is something
> needs to be fixed at the gimple/rtl border to ensure we get good x86 code.
For this case it should be even "easy" within the current RTL expansion
framework since all we need is to look at a single comparison and its
operands.
So I'd suggest to go forward with the canonicalization as proposed
and address the expansion issue as followup. It's currently a missed
optimization for baz anyway.
Richard.
> >
> > At a late stage, maybe during an RTL pass or expansion (or just before
> > expansion) it would indeed be good to generate a shift for such
> > comparisons, on targets where that sets a cc. The lack of this
> > transformation could be considered a blocker for the other one, to avoid
> > regressing on bar.
> Right. In fact, I think Jakub's test ought to be added to this work as
> part of its basic testing.
>
> jeff
>
>
--
Richard Biener <rguent...@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB
21284 (AG Nuernberg)
