On Thu, Jan 03, 2019 at 12:41:52PM +0000, Richard Sandiford wrote:
> Alan Modra <amo...@gmail.com> writes:
> > + case PLUS:
> > + current_or = or_attr_value (XEXP (exp, 0));
> > + if (current_or != -1)
> > + {
> > + int n = current_or;
> > + current_or = or_attr_value (XEXP (exp, 1));
> > + if (current_or != -1)
> > + current_or += n;
> > + }
> > + break;
>
> This doesn't look right. Doing the same for IOR and |= would be OK
> in principle, but write_attr_value doesn't handle IOR yet.
>From what I can see, or_attr_value is used to find the largest power
of two that divides all attr length values for a given insn. So what
should be done for PLUS in an attr length value expression? I can't
simply ignore it as then or_attr_value will return -1 and we'll
calculate length_unit_log as 0 on powerpc when it ought to be 2.
That might matter some time in the future.
If the operands of PLUS are assumed to be insn lengths (reasonable,
since you're likely to use PLUS to sum the machine insn lengths of a
pattern that conditionally emits multiple machine insns), then it
might be better to replace "current_or += n" above with
"current_or |= n". That would be correct for a length expression of
(plus (if_then_else (condA) (const_int 4) (const_int 0))
(if_then_else (condB) (const_int 8) (const_int 4)))
where any answer from or_attr_value that has 3 lsbs of 100b is
sufficently correct. If I keep "+=" then we'd get the wrong answer in
this particular example. However "|=" is wrong if someone is playing
games and writes a silly expression like
(plus (const_int 1) (const_int 3))
since the length is always 4.
> OK with the above dropped, thanks.
Maybe this instead?
case PLUS:
current_or = or_attr_value (XEXP (exp, 0));
current_or |= or_attr_value (XEXP (exp, 1));
break;
> Richard
>
> PS. current write_attr_value doesn't seem to handle operator precedence
> properly, but that's orthogonal.
I hadn't noticed that, but yes that is another problem. I also didn't
implement MINUS, MULT, DIV and MOD in max_attr_value, min_attr_value
and or_attr_value even though write_attr_value handles those cases.
--
Alan Modra
Australia Development Lab, IBM
