Re: RFA: patch to fix a bad code generation for PR64110 -- new constraints addition

[Richard Sandiford <richard dot sandiford at arm dot com>]

Jeff Law <law@redhat.com> writes:
> On 01/24/15 04:29, Richard Sandiford wrote:
>>
>> Yeah.  I expect in practice most people who used "?" and "!" attached
>> them to a particular operand for a reason.  From a quick scan through
>> 386.exp it looked like almost all uses would either want this behaviour
>> or wouldn't care.  An interesting exception is:
>>
>> (define_insn "extendsidi2_1"
>>    [(set (match_operand:DI 0 "nonimmediate_operand" "=*A,r,?r,?*o")
>> 	(sign_extend:DI (match_operand:SI 1 "register_operand" "0,0,r,r")))
>>     (clobber (reg:CC FLAGS_REG))
>>     (clobber (match_scratch:SI 2 "=X,X,X,&r"))]
>>    "!TARGET_64BIT"
>>    "#")
>>
>> I don't know how effective the third alternative is with LRA.  Surely
>> a "r<-0" alternative is by definition a case where "r<-r" is possible
>> only with a "?"-cost reload?  Seems to me we could just delete it.
>> But assuming it does some good, I suppose the "?" really does apply to
>> the alternative as a whole.  If we had to reload operand 1 or operand 0,
>> there's an extra cost if it can't use the same register as the other
>> operand.
>>
>> Wouldn't it be better to make "?" and "!" behave the new way and only
>> add new constraints if it turns out that the old behaviour really is
>> useful in some cases?
>>
>> Maybe stage 4 isn't the time to be making that kind of change.
>> Still, it'd be great if someone who's set up do x86_64 benchmarking
>> could measure the effect of making "?" and "!" behave like the
>> new constraints.
> My worry isn't the x86_64 port, but all the others that folks don't test 
> as regularly.
>
> I'd rather go the other direction, have folks familiar with the port go 
> through it changing the constraints where it makes sense.  That just 
> seems a hell of a lot safer.
>
> A port maintainer could certainly hack something together for testing 
> purposes to guide them as to whether or not there's something to be 
> gained by converting many/most of the ?! to the new constraints.

Yeah, but in practice that's only ever going to be a partial transition.
Many port maintainers won't look at this, so we'll have to support both
versions indefinitely, even if the new behaviour turns out to be the
best for all cases.

I just think we're going to regret having two sets of constraints with
such subtly different meanings.

Looking back at the original PR, Jakub said:

  The ! has been added by me for PR63594, so it isn't there from the era
  when i?86 backend was using reload.  If there is a better way to
  express that RA should prefer to use memory or xmm register and only
  use r constraint if it already is in a r register and doesn't need to
  be reloaded, I can use that.  Whether it is ?, ??? or something else.
  ! description in gcc docs just fitted most what I wanted...

In some ways this seems to match the intention of "*".  Originally I think
it was just an RA-only thing and was ignored by reload, but LRA does take it
into account too (which sounds like progress to me).

If I revert the patch locally and change the *vec_dup<mode> pattern to
use "*", it passes both the test for PR64110 and the tests for PR63594.
Would that be OK as an alternative?

Thanks,
Richard