Undocumented anomaly with EXTRA_CONSTRAINTS

Undocumented anomaly with EXTRA_CONSTRAINTS

[David S. Miller]

As per the documentation and usage, EXTRA_CONSTRANT is for
"Special cases of registers or memory."

Fine so far, but one has to be really careful when using
this for memory operands.  Specifically I just fixed a bug
(patch to gcc-patches and commit to come after my regressions
run) in the Sparc port that appears to be present in other
ports as well.

Consider how constrain_operands works, and also how reload
checks the constraints.  In particular, look at how the 'm'
works.  In constrain_operands it passes if:

	GET_CODE (op) == MEM
	/* Before reload, accept what reload can turn into mem.  */
	|| (strict < 0 && CONSTANT_P (op))
	/* During reload, accept a pseudo  */
	|| (reload_in_progress && GET_CODE (op) == REG
	    && REGNO (op) >= FIRST_PSEUDO_REGISTER)

In the reload case the test is (let us call this the
REG_OK_STRICT case, since that is what it is):

	GET_CODE (operand) == MEM
	|| (GET_CODE (operand) == REG
	    && REGNO (operand) >= FIRST_PSEUDO_REGISTER
	    && reg_renumber[REGNO (operand)] < 0)

These extra:

	1) accept any pseudo if !REG_OK_STRICT
	2) accept only non-renumbered pseudos if REG_OK_STRICT

checks in their memory EXTRA_CONTRAINT entries aren't documented.

Which means that anyone making a mistake here can end up with "severe
tire damage" in reload.

I note that the PA port specifically makes note of this issue with
it's IS_RELOADING_PSEUDO_P macro.  Hi Jeff. :-)  In fact I want to
quote Jeff's comments in the PA headers above the EXTRA_CONSTRAINT
definition because it sums up my email:

   Note that an unassigned pseudo register is such a memory operand.
   Needed because reload will generate these things in insns and then
   not re-recognize the insns, causing constrain_operands to fail.

Anyways, the gist of my email is:

1) Am I right?
2) Once decided, we should document this and perhaps even
   "help" the port somehow.

By "help" the port, I mean we can somehow hide these recog/reload
details about accepting pseudo-registers for memory constraints.

Re: Undocumented anomaly with EXTRA_CONSTRAINTS

[Michael Matz]

Hi,

On Tue, 9 Apr 2002, David S. Miller wrote:

> These extra:
>
> 	1) accept any pseudo if !REG_OK_STRICT
> 	2) accept only non-renumbered pseudos if REG_OK_STRICT
>
> checks in their memory EXTRA_CONTRAINT entries aren't documented.
>
> Anyways, the gist of my email is:
>
> 1) Am I right?

I'm not quite sure what this has to do with only EXTRA_CONSTRAINT.  Any
'm' constraint accepts pseudos before reload and unassigned pseudos while
reloading.  Exactly because unassigned pseudos _are_ memory slots.

> 2) Once decided, we should document this and perhaps even
>    "help" the port somehow.
>
> By "help" the port, I mean we can somehow hide these recog/reload
> details about accepting pseudo-registers for memory constraints.

Hide from what?  From the .md files?  I.e. that patterns having 'm' in the
constraints not seeing REG rtx's?  This would mean to substitute
stackslots into those REG rtx before calling any recognizing things, which
is not what reload want to do.  In a later pass it might get a hardreg, so
replacing a pseudo with a mem too soon would be bad.


Ciao,
Michael.

Re: Undocumented anomaly with EXTRA_CONSTRAINTS

[David S. Miller]

   From: Michael Matz <matzmich@cs.tu-berlin.de>
   Date: Tue, 9 Apr 2002 15:14:43 +0200 (MET DST)

   > 2) Once decided, we should document this and perhaps even
   >    "help" the port somehow.
   >
   > By "help" the port, I mean we can somehow hide these recog/reload
   > details about accepting pseudo-registers for memory constraints.
   
   Hide from what?  From the .md files?  I.e. that patterns having 'm' in the
   constraints not seeing REG rtx's?  This would mean to substitute
   stackslots into those REG rtx before calling any recognizing things, which
   is not what reload want to do.  In a later pass it might get a hardreg, so
   replacing a pseudo with a mem too soon would be bad.
   
No no no, by making something like the pa.h's IS_RELOADING_PSEUDO
public so every port can use it and we only need to fix bugs
in it's definition in one place.
   

Re: Undocumented anomaly with EXTRA_CONSTRAINTS

[David S. Miller]

   From: Michael Matz <matzmich@cs.tu-berlin.de>
   Date: Tue, 9 Apr 2002 15:14:43 +0200 (MET DST)

   I'm not quite sure what this has to do with only EXTRA_CONSTRAINT.  Any
   'm' constraint accepts pseudos before reload and unassigned pseudos while
   reloading.  Exactly because unassigned pseudos _are_ memory slots.

Where is this documented?  :-)  That's the point of my entire email.
If all one did was read md.texi, upon implementing EXTRA_CONSTRAINT
for MEM objects this issue would not be apparent until the compiler
started to fail.

Re: Undocumented anomaly with EXTRA_CONSTRAINTS

[Michael Matz]

Hi,

On Tue, 9 Apr 2002, David S. Miller wrote:

>    I'm not quite sure what this has to do with only EXTRA_CONSTRAINT.  Any
>    'm' constraint accepts pseudos before reload and unassigned pseudos while
>    reloading.  Exactly because unassigned pseudos _are_ memory slots.
>
> Where is this documented?  :-)

Ohh I see.  Guess I stared too long at reload ;)  Indeed documenting this
might be usefull (if not already done so), like would be documenting
reload at all ;)  That pa.h macro probably is usefull globally, I'm just
not sure, how finegraned it should be.  I could think of the usefullness
of a POSSIBLY_MEM_P(rtx) (checking for MEM and before reload also pseudo,
while reload unassigned pseudos).  The pa.h macro as is is rather limited,
as it only does anything while reload.  But before reload pseudos are
valid memory too (constrain-wise at least).  After reload, of course only
real MEMs are valid.  Just to be complete: Also 'o' (offsetable mems)
accepts such pseudos in certain circumstances (although this remark
doesn't relate to any self-written extra constraints accepting mem's).

> That's the point of my entire email. If all one did was read md.texi,
> upon implementing EXTRA_CONSTRAINT for MEM objects this issue would
> not be apparent until the compiler started to fail.

Hmm, but you can define those EXTRA_CONSTRAINTS any way you like, or can't
you?  I.e. if you define one which accepts only a MEM rtx, are you saying
that this breaks, and it really also should accept a pseudo?  This should
only happen, if the pattern itself accepts that pseudo, which would be a
bug in the .md file, if there's no alternative whose constraints accept
the pseudo.


Ciao,
Michael.