On Tue, May 23, 2017 at 06:48:15AM -0400, Aldy Hernandez wrote:
> --- a/gcc/tree-ssanames.h
> +++ b/gcc/tree-ssanames.h
> @@ -45,14 +45,12 @@ struct GTY(()) ptr_info_def
> unsigned int misalign;
> };
>
> -/* Value range information for SSA_NAMEs representing non-pointer variables.
> */
> -
> -struct GTY ((variable_size)) range_info_def {
> - /* Minimum, maximum and nonzero bits. */
> - TRAILING_WIDE_INT_ACCESSOR (min, ints, 0)
> - TRAILING_WIDE_INT_ACCESSOR (max, ints, 1)
> - TRAILING_WIDE_INT_ACCESSOR (nonzero_bits, ints, 2)
> - trailing_wide_ints <3> ints;
> +/* Used bits information for SSA_NAMEs representing non-pointer variables.
> */
> +
> +struct GTY ((variable_size)) nonzero_bits_def {
> + /* Mask representing which bits are known to be used in an integer. */
> + TRAILING_WIDE_INT_ACCESSOR (nonzero_bits, ints, 0)
> + trailing_wide_ints <1> ints;
> };
>
>
> --- a/gcc/tree-core.h
> +++ b/gcc/tree-core.h
> @@ -1416,11 +1413,15 @@ struct GTY(()) tree_ssa_name {
> union ssa_name_info_type {
> /* Pointer attributes used for alias analysis. */
> struct GTY ((tag ("0"))) ptr_info_def *ptr_info;
> - /* Value range attributes used for zero/sign extension elimination. */
> - struct GTY ((tag ("1"))) range_info_def *range_info;
> + /* Value range attributes. */
> + class GTY ((tag ("1"))) irange *range_info;
> } GTY ((desc ("%1.typed.type ?" \
> "!POINTER_TYPE_P (TREE_TYPE ((tree)&%1)) : 2"))) info;
>
> + /* For non-pointer types, this specifies a mask for the bits that
> + are known to be set. */
> + struct nonzero_bits_def *nonzero_bits;
> +
> /* Immediate uses list for this SSA_NAME. */
> struct ssa_use_operand_t imm_uses;
> };
I'm worried a lot here about compile time memory usage increase, especially
with EVRP and IPA-VRP and even more so with LTO.
The changes mean that for every SSA_NAME of any kind we now need 8 more
bytes, and for every SSA_NAME that has range info (typically both range info
and nonzero_bits; in the old implementation the two were tied together for a
good reason, many ranges also imply some non-zero bits and from non-zero
bits one can in many cases derive a range) much more than that (through
the nonzero_bits_def you get all the overhead of trailing_wide_ints -
the 3 fields it has, just save on the actual 2 lengths and 2 HWI sets,
but then you add 3x 8 bytes and 6x size of the whole wide_int which is
from what I understood not really meant as the memory storage of wide ints
in data structures, but as something not memory efficient you can work
quickly on (so ideal for automatic variables in functions that work with
those). So it is a 8 byte growth for SSA_NAMEs without ranges and
8+3*8+6*32-2*4-2*8*x growth for SSA_NAMEs with ranges if x is the number
of HWIs needed to represent the integers of that type (so most commonly
x=1). For x=1 8+3*8+6*32-2*4-2*8*x is 200 bytes growth.
With say 10000000 SSA_NAMEs, 5000000 of them with ranges, that will be
already a 1GB difference, dunno how many SSA_NAMEs are there e.g. in firefox
LTO build.
Can the nonzero_bits stuff be folded back into irange (and have code to
maintain nonzero_bits in addition to ranges as before (from setting a range
compute or update nonzero_bits and vice versa)? Can you use
trailing_wide_int for the irange storage of the values? Can you allocate
only as many HWIs as you really need, rather than always 6?
Again, it can be different between a class you use for accessing the
information and manipulating it and for actual underlying storage on
SSA_NAMEs.
> --- /dev/null
> +++ b/gcc/range.h
> +
> +You should have received a copy of the GNU General Public License
> +along with GCC; see the file COPYING3. If not see
> +<http://www.gnu.org/licenses/>. */
> +
> +#ifndef GCC_RANGE_H
> +#define GCC_RANGE_H
> +#define MAX_RANGES 6
> +
> +typedef class irange *irange_p;
> +enum irange_type { RANGE_PLAIN, RANGE_INVERT };
> +
> +class GTY(()) irange
> +{
> + private:
> + bool overflow;
> + size_t n;
> + void prepend (wide_int x, wide_int y);
> + void append (wide_int x, wide_int y);
> + void remove (unsigned i, unsigned j);
> + void canonicalize ();
> + /* This is stupid. These two should be private, but the GTY
> + machinery can't look inside an irange. */
> + public:
> + tree type;
> + wide_int bounds[MAX_RANGES];
> +
> +public:
...
> + void Union (wide_int x, wide_int y);
> + bool Union (const irange &r);
> + bool Union (const irange &r1, const irange &r2);
Do we really want methods starting with capital letters?
I understand why you can't use union, but I don't think we use CamelCase
anywhere.
Jakub
