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

[Richard Sandiford <rdsandiford@googlemail.com>]

Jeff Law <law@redhat.com> writes:
> On 05/31/14 03:02, Richard Sandiford wrote:
>> 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.
> Sounds good.  Do you think prebuilding those arrays once at build time 
> is likely to have a measurable impact?  I realize you probably haven't 
> done measurements, just looking for your gut instinct here.

I have a local patch that does it, but unfortunately it doesn't seem to
make a measurable difference.  I'll try measuring it again once other
lower-hanging fruit are out of the way.

>> 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).
> Funny thing is I suspect the n_alternatives == 0 case is rare in 
> practice though.

Yeah, probably. :-)

Thanks,
Richard