This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Fix up wi::lrshift (PR c++/69399)


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]