Re: RFC cse weirdness

[Andreas Krebbel <krebbel1 at de dot ibm dot com>]

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.

> 
> > 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:
(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)"
{ ...

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?

(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)"
  "@ ...

>   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?
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. "

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?!


If there is no other way than splitting the pattern I'll do this. 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.

Bye,

-Andreas-