This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [Patch,AVR]: Solve PR42210
Richard Henderson schrieb:
> On 04/26/2011 04:51 AM, Georg-Johann Lay wrote:
>> As the SFRs are volatile, insv expander skips
>> them and a combine pattern must care of them. Omitting the complicated
>> bit-plethora patterns I still see long, slow shift loops for HI. So
>> the patch still includes them.
>
> Why don't we focus on doing the right thing for plain registers first,
> and worry about volatile SFRs second?
The shift loops occur on registers. The insv now covers registers.
Besides that, in many places these operations are actually needed to
set SFR bits. I see combine try many insv for HImode; I don't know if
it is wise to support them without actually knowing what they are used
for. Most of this stuff will actually just use one byte of the values,
but that can't be seen at that point.
> In particular, combine *should* be doing the right thing for registers,
> putting those AND/OR/SHIFT whatsits back into insv/extv form, via
> make_compound_operation.
Yes it should. Bit combine only looks into thins up to some depth, and
intermediate patterns can help combine find the way. In the particular
case the combine bridges do no harm because they map to efficient
instruction sequences in all cases.
>> The snip "INTVAL(operands[...])" occurs more than 50 times in the
>> machine description. Wouln't it be good to have an abbreviation like
>> #define OPVAL(n) INTVAL(operands[n])) defined in, e.g. avr-protos.h?
>
> No, what would be better is to define more predicates. Though in the
> case of comparing op2 vs op4, there's probably no helping it.
Was just some a suggestion. I don't see how predicate would help in
switch (INTVAL(operands[n])) or similar places in all the C snipps...
The predicates you indicated are already included in the last patch.
> Why are you adding "optimize" to all these insns? None of them will
> be matched unless combine is run, which implies optimization.
Similar pattern like *sbi, *cbi etc use that insn condition. I don't
know what the rationale behind that is/was, I just followed that.
Johann