[PATCH 0/5] Cache recog_op_alt by insn code, take 2

[Richard Sandiford <rdsandiford@googlemail.com>]

Sorry Jeff, while working on the follow-on LRA patch I came across
a couple of problems, so I need another round on this.

First of all, I mentioned doing a follow-on patch to make LRA and
recog use the same cache for operand_alternatives.  I hadn't realised
until I wrote that patch that there's a significant difference between
the LRA and recog_op_alt versions of the information: recog_op_alt
groups alternatives together by operand whereas LRA groups operands
together by alternative.  So in the cached flat array, recog_op_alt
uses [op * n_alternatives + alt] whereas LRA uses [alt * n_operands + nop].
The two could only share a cache if they used the same order.

I think the LRA ordering makes more sense.  Quite a few places are
only interested in alternative which_alternative, so grouping operands
together by alternative gives a natural subarray.  And there are places
in IRA (which uses recog_op_alt rather than the LRA information) where
the "for each alternative" loop is the outer one.

A second difference was that preprocess_constraints skips disabled
alternatives while LRA's setup_operand_alternative doesn't; LRA just
checks for disabled alternatives when walking the array instead.
That should make no difference in practice, but since the LRA approach
would also make it easier to precompute the array at build time,
I thought it would be a better way to go.  It also gives a cleaner
interface: we can have a preprocess_insn_constraints function that
takes a (define_insn-based) insn and returns the operand_alternative
information without any global state.

Also, it turns out that sel-sched.c, regcprop.c and regrename.c
modify the recog_op_alt structure as a convenient way of handling
matching constraints.  That obviously isn't a good idea if we want
other passes to reuse the data later.  I've fixed that and made the
types "const" so that new instances shouldn't creep in.

Also, I hadn't realised that a define_insn with no constraints
at all has 0 alternatives rather than 1.  Some passes nevertheless
unconditionally access the recog_op_alt information for alternative
which_alternative.

I could have fixed that by making n_alternatives always be >= 1
in the static insn table.  Several places do have fast paths for
n_alternatives == 0 though, so I thought it would be better to
handle the 0 alternative case in preprocess_constraints as well.
All it needs is a MIN (..., 1).

So the patch has now grown to 5 subpatches:

1. make recog_op_alt a flat array and order it in the same way as LRA
2. fix the places that modified recog_op_alt locally
3. don't handle the enabled attribute in preprocess_constraints and make
   sure all consumers check for disabled alternatives explicitly
4. the updated version of the original patch
5. get LRA to use the new cache too (the original follow-on patch)

Jeff Law <law@redhat.com> writes:
> On 05/20/14 02:19, Richard Sandiford wrote:
>> Following on from (and depending on) the last patch, process_constraints
>> also shows up high in the profile.  This patch caches the recog_op_alt
>> information by insn code too.  It also shrinks the size of the structure
>> from 1 pointer + 5 ints to 1 pointer + 2 ints:
>>
>> - no target should have more than 65536 register classes :-)
> That could probably be much lower if we needed more bits for other things.
>
>> - "reject" is based on a cost of 600 for '!', so 16 bits should be plenty
> OK.  Makes sense.
>
>
>> - "matched" and "matches" are operand numbers and so fit in 8 bits
> OK.  This could also be smaller, don't we have an upper limit of 32 (or 
> 30?) on the number of operands appearing in an insn.  It'd be a way to 
> get a few more bits if we need them someday.

Yeah, we could probably squeeze everything into a single int if we needed
to and if we were prepared to have non-byte-sized integer fields.
Going from 2 ints to 1 int wouldn't help LP64 hosts as things stand
though, since we have a pointer at the beginning of the struct.

Boostrapped & regression-tested on x86_64-linux-gnu.  Also tested by
building gcc.dg, g++.dg and gcc.c-torture for one target per config/
directory and making sure that there were no asm differences.

Thanks,
Richard