On 23 May 2006 15:42, Andreas Krebbel wrote:
> Hi,
>
>> Hmm. I note that if you /were/ using match_dups, the problem would be
>> solved because all four changes would go through the 'then' clause of the
>> if...else construct. Maybe it would be more worthwhile for you to have
>> separate patterns after all and find some other way of making reload happy.
> Yes splitting the pattern and using match_dups would fix it. It already has
> been like this some time ago. The patterns were merged to give reload more
> alternatives.
Well, merging these patterns may just not be possible given the other
restrictions you have to meet, but read on....
>>> Btw. this is not a s390 back end invention. You can see the same in the
>>> i386.md file for "anddi3".
>>
>> Eh? I've looked at 3.3.3 and at HEAD and it seems to be doing everything
>> completely differently: there are no constraints at all, let alone matching
>> ones, the pattern in question is an expander not an insn pattern, and the
>> expander routine ix86_expand_binary_operator takes care of making sure only
>> valid combinations of mem operands are used.
>
> More precisely I should have said "*anddi_1_rex64", "*andsi_1" and others:
> For the first 3 alternatives a matching constraint is used and for the last
> one the same is achieved calling ix86_binary_operator_ok. This is exactly
> how we did it on s390 or am I missing something?
Yes, quite definitely. Looking at those patterns:
> (define_insn "*anddi_1_rex64"
> [(set (match_operand:DI 0 "nonimmediate_operand" "=r,rm,r,r")
> (and:DI (match_operand:DI 1 "nonimmediate_operand" "%0,0,0,qm")
> (match_operand:DI 2 "x86_64_szext_general_operand"
> "Z,re,rm,L"))) (clobber (reg:CC FLAGS_REG))]
> "TARGET_64BIT && ix86_binary_operator_ok (AND, DImode, operands)"
> { ...
> (define_insn "*anddi3"
> [(set (match_operand:DI 0 "nonimmediate_operand" "=d,d,d,d,d,d,d,d,AQ,Q")
> (and:DI (match_operand:DI 1 "nonimmediate_operand"
> "%d,o,0,0,0,0,0,0,0,0")
> (match_operand:DI 2 "general_operand"
>
> "M,M,N0HDF,N1HDF,N2HDF,N3HDF,d,m,NxQDF,Q"))) (clobber (reg:CC
> CC_REGNUM))] "TARGET_64BIT && !TARGET_EXTIMM &&
> s390_logical_operator_ok_p (operands)" "@ ...
The x86 pattern is called "anddi_1_rex64", which is not one of the standard
names: that means the compiler will never emit it directly. The only RTL that
matches that pattern will be that which has already been matched by the x86's
"anddi3" expander, which will have called ix86_expand_binary_operator to munge
the operands into a form that the "anddi_1_rex64" pattern can accept without
problems.
In your case, you're defining the named pattern "anddi3" directly as an
insn, and the compiler is going to assume it's fully capable of dealing with
whatever combination of operands are accepted by the calls to
nonimmediate_operand and general_operand. (I don't think that prefixing
"anddi3" with an asterisk will hide it from the compiler and prevent it being
treated as the named pattern for anddi3; IIRC that doesn't work to protect the
well-known names, and so neither can you have, for example, two patterns
called "anddi3" and "*anddi3").
>> Well, hang on. The change that you're complaining of is replacing one
>> pseudo by another. It's fairly fundamental that gcc assumes that all
>> pseudos are equivalent and freely interchangeable; there are no reg
>> classes in pseudos. So if your design places special restrictions on
>> gcc's use of and freedom to swap and substitute pseudos, your design is
>> probably making an invalid assumption about gcc's use of pseudos, or it's
>> trying to do something by placing constraints on pseudo-reg numbers that
>> it should be trying to do by some other means.
> But doesn't show the code in validate_canon_reg already that it isn't always
> applicable to replace one pseudo with another e.g. when match_dups are
> present?
That's not an 'e.g'; it's *only* when match_dups are present (or if the
instruction has /already/ been changed in either mode or code, but replacing
one pseudo by another won't have that effect) that replacing one pseudo by
another isn't necessarily applicable. Your code doesn't have match dups,
meaning that your code claims to the compiler's mid-end that it *is*
legitimate to replace one pseudo with another. Remember, constraints aren't
used in recognition, only in reload, so re-recognizing the RTL with a
different pseudo in place will just match exactly the same insn anyway, and
it's only when reload comes around that we will care about them being
*matching* operands.
> I don't find these kind of restrictions in the gcc internals manual
> describing insn conditions:
> It is restricted for named patterns:
>
> "For a named pattern, the condition (if present) may not depend on the data
> in the insn being matched, but only the target-machine-type flags. The
> compiler needs to test these conditions during initialization in order to
> learn exactly which named instructions are available in a particular run."
>
> A second part applies to nameless patterns:
>
> "For nameless patterns ... For an insn where the condition has once matched,
> it can't be used to control register allocation, for example by excluding
> certain hard registers or hard register combinations. "
You are confusing the *condition* with the *constraints*. Your conditions
are "nonimmediate_operand", which accept pretty much anything. It is only the
conditions that are tested in recog, not the constraints.
Also, you may have confused gcc by creating a pattern with the same name as
a well-known named pattern but trying to make it nameless with using an
asterisk. Don't try to re-use the well-known pattern names; do something
quite different, such as adding a suffix like the x86 does.
> An insn condition forcing two pseudos to be the same would be trivially
> fulfilled after register allocation as well. So I would say the insn
> condition we are using does not try to force an assumption on register
> allocation - right?!
Yes, that's right; there's no way it could because the conditions are
applied to test each operand individually, there's no way nonimmediate_operand
can compare the argument it is given to one of the other operands from the
current pattern. None of the conditions make assumptions on register
allocation, because register allocation has not yet been run! If you need to
impose conditions on your operands that the condition tests can't handle, then
in the body of an expander is the place to do it.
> If there is no other way than splitting the pattern I'll do this.
I'm really very sure this is the way to go. It works for the x86, after
all! So, I would reckon your approach is
1) define expanders for the insns
2) rename the 'nameless' patterns so that you can call emit/gen_XXX from your
expander and not get into an infinite recursion!
> Probably
> somebody then should have a look at the i386 machine description and maybe
> it should be made a bit clearer in the gcc internals manual.
Yes, the manual often does skate over things that could do with more clear
explanations.
cheers,
DaveK
--
Can't think of a witty .sigline today....
