Alan Modra <amo...@gmail.com> writes:
> On Fri, Jan 04, 2019 at 12:18:03PM +0000, Richard Sandiford wrote:
>> Alan Modra <amo...@gmail.com> writes:
>> > On Thu, Jan 03, 2019 at 07:03:59PM +0000, Richard Sandiford wrote:
>> >> Richard Sandiford <richard.sandif...@arm.com> writes:
>> >> > This still seems risky and isn't what the name and function comment
>> >
>> > OK, how about this delta from the previous patch to ameliorate the
>> > maintenance risk?
>>
>> attr_value_alignment seems clearer, and means that we can handle
>> things like:
>>
>> (mult (symbol_ref "...") (const_int 4))
>
> OK, revised patch as follows, handling MINUS and MULT in the max/min
> value functions too.
>
> * genattrtab.c (max_attr_value, min_attr_value, or_attr_value):
> Delete "unknownp" parameter. Adjust callers. Handle
> CONST_INT, PLUS, MINUS, and MULT.
> (attr_value_aligned): Renamed from or_attr_value.
> (min_attr_value): Return INT_MIN for unhandled rtl case..
> (min_fn): ..and translate to INT_MAX here.
> (write_length_unit_log): Modify to cope without "unknown".
> (write_attr_value): Handle IF_THEN_ELSE.
>
> diff --git a/gcc/genattrtab.c b/gcc/genattrtab.c
> index 2cd04cdb06f..b8adf704009 100644
> --- a/gcc/genattrtab.c
> +++ b/gcc/genattrtab.c
> @@ -266,9 +266,9 @@ static int compares_alternatives_p (rtx);
> static void make_internal_attr (const char *, rtx, int);
> static void insert_insn_ent (struct attr_value *, struct insn_ent *);
> static void walk_attr_value (rtx);
> -static int max_attr_value (rtx, int*);
> -static int min_attr_value (rtx, int*);
> -static int or_attr_value (rtx, int*);
> +static int max_attr_value (rtx);
> +static int min_attr_value (rtx);
> +static unsigned int attr_value_alignment (rtx);
> static rtx simplify_test_exp (rtx, int, int);
> static rtx simplify_test_exp_in_temp (rtx, int, int);
> static rtx copy_rtx_unchanging (rtx);
> @@ -1550,15 +1550,16 @@ one_fn (rtx exp ATTRIBUTE_UNUSED)
> static rtx
> max_fn (rtx exp)
> {
> - int unknown;
> - return make_numeric_value (max_attr_value (exp, &unknown));
> + return make_numeric_value (max_attr_value (exp));
> }
>
> static rtx
> min_fn (rtx exp)
> {
> - int unknown;
> - return make_numeric_value (min_attr_value (exp, &unknown));
> + int val = min_attr_value (exp);
> + if (val < 0)
> + val = INT_MAX;
> + return make_numeric_value (val);
> }
>
> static void
> @@ -1568,24 +1569,21 @@ write_length_unit_log (FILE *outf)
> struct attr_value *av;
> struct insn_ent *ie;
> unsigned int length_unit_log, length_or;
> - int unknown = 0;
>
> if (length_attr)
> {
> - length_or = or_attr_value (length_attr->default_val->value, &unknown);
> + length_or = attr_value_alignment (length_attr->default_val->value);
> for (av = length_attr->first_value; av; av = av->next)
> for (ie = av->first_insn; ie; ie = ie->next)
> - length_or |= or_attr_value (av->value, &unknown);
> - }
> + length_or |= attr_value_alignment (av->value);
>
> - if (length_attr == NULL || unknown)
> - length_unit_log = 0;
> - else
> - {
> length_or = ~length_or;
> for (length_unit_log = 0; length_or & 1; length_or >>= 1)
> length_unit_log++;
> }
> + else
> + length_unit_log = 0;
> +
> fprintf (outf, "EXPORTED_CONST int length_unit_log = %u;\n",
> length_unit_log);
> }
>
> @@ -3753,11 +3751,12 @@ write_test_expr (FILE *outf, rtx exp, unsigned int
> attrs_cached, int flags,
> return attrs_cached;
> }
>
> -/* Given an attribute value, return the maximum CONST_STRING argument
> - encountered. Set *UNKNOWNP and return INT_MAX if the value is unknown.
> */
> +/* Given an attribute value expression, return the maximum value that
> + might be evaluated assuming all conditionals are independent.
> + Return INT_MAX if the value can't be calculated by this function. */
Not sure about "assuming all conditionals are independent". All three
functions should be conservatively correct without any assumptions.
OK without that part if you agree.
Thanks,
Richard
