This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Fix up wi::lrshift (PR c++/69399)
- From: Jakub Jelinek <jakub at redhat dot com>
- To: Richard Biener <richard dot guenther at gmail dot com>
- Cc: Mike Stump <mikestump at comcast dot net>, Richard Sandiford <rdsandiford at googlemail dot com>, "H.J. Lu" <hjl dot tools at gmail dot com>, GCC Patches <gcc-patches at gcc dot gnu dot org>
- Date: Wed, 27 Jan 2016 12:36:37 +0100
- Subject: Re: [PATCH] Fix up wi::lrshift (PR c++/69399)
- Authentication-results: sourceware.org; auth=none
- References: <CAFiYyc3d+rRFi4hOdaDip-c_ohPYOy7keYAWRaE7V-XALnfKwQ at mail dot gmail dot com> <CAMe9rOpAPQendCYodbCC2SjtC3xSGcPz6pfN3LjXNAnq2R2iPQ at mail dot gmail dot com> <CAFiYyc1tGWtw7f5LHKP79v1pKPp8rmJNKRBv52qw_x4Gga=8mg at mail dot gmail dot com> <20160126182148 dot GE3017 at tucnak dot redhat dot com> <4ADD4937-E19F-4CD0-8441-C97935548759 at comcast dot net> <CD95CA3C-11D2-4C9D-A418-BD6DB4DD4B47 at gmail dot com> <20160126212616 dot GK3017 at tucnak dot redhat dot com> <0ADBB08F-103E-4CC7-A7B9-93937E809197 at comcast dot net> <20160126234152 dot GM3017 at tucnak dot redhat dot com> <CAFiYyc1-VLJZrN+poUPRUEa08EbOi3AOM1zaqhpr+imMxHJ-MQ at mail dot gmail dot com>
- Reply-to: Jakub Jelinek <jakub at redhat dot com>
On Wed, Jan 27, 2016 at 11:38:33AM +0100, Richard Biener wrote:
> On Wed, Jan 27, 2016 at 12:41 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> > On Tue, Jan 26, 2016 at 01:55:41PM -0800, Mike Stump wrote:
> >> On Jan 26, 2016, at 1:26 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> >> > will do cc1plus size comparison afterwards.
> >>
> >> We know the dynamic check is larger. You canât tell the advantage of
> >> speed from size. Better would be to time compiling any random large
> >> translation unit.
> >>
> >> Nice to see that only 14 calls remain, thatâs way better than the 34 I
> >> thought.
> >
> > So, it seems probably the PR65656 changes made things actually significantly
> > worse, while I see a (small) difference in the generated code between the two
> > patches if I compile say tree-ssa-ccp.c with g++ 5.x, in the bootstrapped
> > compiler there is no difference at all, the compilers with either patch
> > have identical objdump -dr cc1plus. Already at *.gimple time all the
> > relevant __builtin_constant_p are resolved and it seems all to 0.
> >
> > So please agree on one of the two patches (don't care which), and I'll try
> > to distill a testcase to look at for PR65656.
>
> I don't care if the wi::lshift by LOG2_BITS_PER_UNIT done by
> get_ref_base_and_extent
> are compiled to as good quality as before (I suspect it doesn't matter
> in this case
> as the shift amount is constant, but maybe due to PR65656 the
> non-STATIC_CONSTANT_P
> variant is better).
Ok, I'll commit the non-STATIC_CONSTANT_P variant for now, we can revisit it
later. I have distilled a testcase for Jason in PR65656. It seems the
problematic STATIC_CONSTANT_P is only if it is inside of the condition of
?: expression. So we could work around it by using something like:
- if (STATIC_CONSTANT_P (xi.precision > HOST_BITS_PER_WIDE_INT)
- ? (STATIC_CONSTANT_P (shift < HOST_BITS_PER_WIDE_INT - 1)
- && xi.len == 1
- && xi.val[0] <= (HOST_WIDE_INT) ((unsigned HOST_WIDE_INT)
- HOST_WIDE_INT_MAX >> shift))
- : precision <= HOST_BITS_PER_WIDE_INT)
+ bool fast_path = false;
+ if (STATIC_CONSTANT_P (xi.precision > HOST_BITS_PER_WIDE_INT)
+ {
+ if (STATIC_CONSTANT_P (shift < HOST_BITS_PER_WIDE_INT - 1)
+ && xi.len == 1
+ && xi.val[0] <= (HOST_WIDE_INT) ((unsigned HOST_WIDE_INT)
+ HOST_WIDE_INT_MAX >> shift))
+ fast_path = true;
+ }
+ else if (precision <= HOST_BITS_PER_WIDE_INT)
+ fast_path = true;
+ if (fast_path)
{
val[0] = xi.ulow () << shift;
result.set_len (1);
}
else
result.set_len (lshift_large (val, xi.val, xi.len,
precision, shift));
and similarly in lrshift.
Jakub