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] Partial fix for PR 18048


Zdenek Dvorak <rakdver@atrey.karlin.mff.cuni.cz> writes:

> 3) A small improvement in number_of_iterations_cond.
>    build_int_cst (type, -1) is not suitable for producing unsigned
>    constant with all bits one, since the bits outside of the precision
>    of the type are set to 1 as well, which makes fold to fail to
>    perform some of the optimizations with such constants.
...

> !       bound = EXEC_UNARY (BIT_NOT_EXPR, niter_type,
> ! 			  build_int_cst_type (niter_type, 0));
> !       bound = EXEC_BINARY (RSHIFT_EXPR, niter_type, bound, bits);

While this is a good conceptual change, this is not the appropriate
way to do it.  You are creating trees just to get fold() to do
arithmetic for you, which wastes memory, and you risk having overflow
bits set on the constants, which will also inhibit optimization.

It would be nice if force_fit_type could be used for this purpose, but
it also creates junk trees and sets overflow bits.  The most efficient
way to code this logic is like so:

  /* Construct an INTEGER_CST with type niter_type, whose low
     (TYPE_PRECISION (niter_type) - bits) bits are all 1 and whose
     remaining bits are all 0.  */
  {
    unsigned int precision = TYPE_PRECISION (niter_type) - bits;

    unsigned HOST_WIDE_INT blow;
    HOST_WIDE_INT bhigh;

    if (precision < HOST_BITS_PER_WIDE_INT)
      {
        bhigh = 0;
        blow = (((unsigned HOST_WIDE_INT) 1) << precision) - 1;
      }
    else
      {
        precision -= HOST_BITS_PER_WIDE_INT;

        blow = ~ (unsigned HOST_WIDE_INT) 0;
        bhigh = (((unsigned HOST_WIDE_INT) 1) << precision) - 1;
      }
    bound = build_int_cst_wide (niter_type, blow, bhigh);
  }

This replaces both the statements quoted from your patch.  It would be
a good idea to do similar things to all calculations where you know
that a call to fold() will always spit out an INTEGER_CST.  The
*_double functions defined in fold-const.c are frequently useful for
this.

(There may be an off-by-one error in the above, I'm not sure.)

(Needing to construct a CONST_INT for a mask value is a common thing,
and the code is tricky.  Maybe we should have

extern tree tree_int_cst_{low,high}_mask (tree type, unsigned int bits);

primitives which produce masks filled from the low or the high end of
the constant.)

zw


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