RFA: Provide a default definition of CONSTANT_ADDRESS_P

[Nick Clifton]

Hi Guys,

  This is a follow up to a comment from Jeff Law about a patch I
  created for the MN10300 port.  We found that for that port that its
  definition of CONSTANT_ADDRESS_P() as just CONSTANT_P() was a little
  bit too permissive.  In particular it was allowing CONST_DOUBLEs to
  be treated as constant addresses which tended to confuse other parts
  of the compiler.

  It turns out that quite a few backends use this definition of
  CONSTANT_ADDRESS_P.  It is suggested in the tm.texi documentation
  which is probably why it has happened.  So I am seeking approval for
  the attached patch to address the problem.  It does two things:
  Firstly it provides a default definition of CONSTANT_ADDRESS_P in
  defaults.h which includes a test to make sure that CONST_DOUBLEs are
  not allowed.  Secondly it updates all of the backends which used to
  define CONSTANT_ADDRESS_P as CONSTANT_P so that they now use this
  default definition.

  Tested by building the following targets: avr-elf, bfin-elf,
  cris-elf, fr30-elf, frv-elf, m32c-elf, m68hc12-elf, mep-elf,
  mn10300-elf, moxie-elf, pdp11-aout, picochip-elf, score-elf,
  xstormy16-elf.

  I also ran an x86 bootstrap just to make sure that the change to
  defaults.h did not break anything - it did not.

  OK to apply ?

Cheers
  Nick

gcc/ChangeLog
2009-10-20  Nick Clifton  <nickc@...>

        * defaults.h (CONSTANT_ADDRESS_P): Provide a default definition.
        Make sure that it does not allow CONST_DOUBLEs.
        * doc/tm.texi (CONSTANT_ADDRESS_P): Update description.
        * config/avr/avr.h (CONSTANT_ADDRESS_P): Delete.
        * config/bfin/bfin.h (CONSTANT_ADDRESS_P): Delete.
        * config/cris/cris.h (CONSTANT_ADDRESS_P): Delete.
        * config/fr30/fr30.h (CONSTANT_ADDRESS_P): Delete.
        * config/frv/frv.h (CONSTANT_ADDRESS_P): Delete.
        * config/m32c/m32c.h (CONSTANT_ADDRESS_P): Delete.
        * config/m68hc11/m68hc11.h (CONSTANT_ADDRESS_P): Delete.
        * config/mep/mep.h (CONSTANT_ADDRESS_P): Delete.
        * config/mn10300/mn10300.h (CONSTANT_ADDRESS_P): Delete.
        * config/moxie/moxie.h (CONSTANT_ADDRESS_P): Delete.
        * config/pdp11/pdp11.h (CONSTANT_ADDRESS_P): Delete.
        * config/picochip/picochip.h (CONSTANT_ADDRESS_P): Delete.
        * config/score/score.h (CONSTANT_ADDRESS_P): Delete.
        * config/stormy16/stormy16.h (CONSTANT_ADDRESS_P): Delete.


Index: gcc/doc/tm.texi
===================================================================
--- gcc/doc/tm.texi (revision 152972)
+++ gcc/doc/tm.texi (working copy)
@@ -5370,9 +5369,10 @@
 
 @defmac CONSTANT_ADDRESS_P (@var{x})
 A C expression that is 1 if the RTX @var{x} is a constant which
-is a valid address.  On most machines, this can be defined as
-@code{CONSTANT_P (@var{x})}, but a few machines are more restrictive
-in which constant addresses are supported.
+is a valid address.  On most machines the default definition of
+@code{(CONSTANT_P (@var{x}) && GET_CODE (@var{x}) != CONST_DOUBLE)}
+is acceptable, but a few machines are more restrictive as to which
+constant addresses are supported.
 @end defmac
 
 @defmac CONSTANT_P (@var{x})
Index: gcc/defaults.h
===================================================================
--- gcc/defaults.h (revision 152972)
+++ gcc/defaults.h (working copy)
@@ -1154,4 +1154,10 @@
 #define GO_IF_MODE_DEPENDENT_ADDRESS(X, WIN)
 #endif
 
+/* For most ports anything that evaluates to a constant symbolic
+   or integer value is acceptable as a constant address.  */
+#ifndef CONSTANT_ADDRESS_P
+#define CONSTANT_ADDRESS_P(X)   (CONSTANT_P (X) && GET_CODE (X) != CONST_DOUBLE)
+#endif
+
 #endif  /* ! GCC_DEFAULTS_H */
Index: gcc/config/frv/frv.h
===================================================================
--- gcc/config/frv/frv.h (revision 152972)
+++ gcc/config/frv/frv.h (working copy)
@@ -1968,16 +1968,6 @@
 
 /* Addressing Modes.  */
 
-/* A C expression that is 1 if the RTX X is a constant which is a valid
-   address.  On most machines, this can be defined as `CONSTANT_P (X)', but a
-   few machines are more restrictive in which constant addresses are supported.
-
-   `CONSTANT_P' accepts integer-values expressions whose values are not
-   explicitly known, such as `symbol_ref', `label_ref', and `high' expressions
-   and `const' arithmetic expressions, in addition to `const_int' and
-   `const_double' expressions.  */
-#define CONSTANT_ADDRESS_P(X) CONSTANT_P (X)
-
 /* A number, the maximum number of registers that can appear in a valid memory
    address.  Note that it is up to you to specify a value equal to the maximum
    number that `TARGET_LEGITIMATE_ADDRESS_P' would ever accept.  */
Index: gcc/config/m32c/m32c.h
===================================================================
--- gcc/config/m32c/m32c.h (revision 152972)
+++ gcc/config/m32c/m32c.h (working copy)
@@ -563,7 +563,6 @@
 
 #define HAVE_PRE_DECREMENT 1
 #define HAVE_POST_INCREMENT 1
-#define CONSTANT_ADDRESS_P(X) CONSTANT_P(X)
 #define MAX_REGS_PER_ADDRESS 1
 
 /* This is passed to the macros below, so that they can be implemented
Index: gcc/config/mep/mep.h
===================================================================
--- gcc/config/mep/mep.h (revision 152972)
+++ gcc/config/mep/mep.h (working copy)
@@ -567,8 +567,6 @@
 #define TRAMPOLINE_SIZE 20
 
 
-#define CONSTANT_ADDRESS_P(X) CONSTANT_P (X)
-
 #define MAX_REGS_PER_ADDRESS 1
 
 #ifdef REG_OK_STRICT
Index: gcc/config/pdp11/pdp11.h
===================================================================
--- gcc/config/pdp11/pdp11.h (revision 152972)
+++ gcc/config/pdp11/pdp11.h (working copy)
@@ -594,10 +594,6 @@
 
 #define MAX_REGS_PER_ADDRESS 1
 
-/* Recognize any constant value that is a valid address.  */
-
-#define CONSTANT_ADDRESS_P(X)  CONSTANT_P (X)
-
 /* Nonzero if the constant value X is a legitimate general operand.
    It is given that X satisfies CONSTANT_P or is a CONST_DOUBLE.  */
 
Index: gcc/config/avr/avr.h
===================================================================
--- gcc/config/avr/avr.h (revision 152972)
+++ gcc/config/avr/avr.h (working copy)
@@ -406,8 +406,6 @@
 #define HAVE_POST_INCREMENT 1
 #define HAVE_PRE_DECREMENT 1
 
-#define CONSTANT_ADDRESS_P(X) CONSTANT_P (X)
-
 #define MAX_REGS_PER_ADDRESS 1
 
 #define REG_OK_FOR_BASE_NOSTRICT_P(X) \
Index: gcc/config/stormy16/stormy16.h
===================================================================
--- gcc/config/stormy16/stormy16.h (revision 152972)
+++ gcc/config/stormy16/stormy16.h (working copy)
@@ -522,8 +522,6 @@
 
 #define HAVE_PRE_DECREMENT 1
 
-#define CONSTANT_ADDRESS_P(X) CONSTANT_P (X)
-
 #define MAX_REGS_PER_ADDRESS 1
 
 #ifdef REG_OK_STRICT
Index: gcc/config/fr30/fr30.h
===================================================================
--- gcc/config/fr30/fr30.h (revision 152972)
+++ gcc/config/fr30/fr30.h (working copy)
@@ -741,16 +741,6 @@
 /*}}}*/ 
 /*{{{  Addressing Modes.  */
 
-/* A C expression that is 1 if the RTX X is a constant which is a valid
-   address.  On most machines, this can be defined as `CONSTANT_P (X)', but a
-   few machines are more restrictive in which constant addresses are supported.
-
-   `CONSTANT_P' accepts integer-values expressions whose values are not
-   explicitly known, such as `symbol_ref', `label_ref', and `high' expressions
-   and `const' arithmetic expressions, in addition to `const_int' and
-   `const_double' expressions.  */
-#define CONSTANT_ADDRESS_P(X) CONSTANT_P (X)
-
 /* A number, the maximum number of registers that can appear in a valid memory
    address.  Note that it is up to you to specify a value equal to the maximum
    number that `GO_IF_LEGITIMATE_ADDRESS' would ever accept.  */
Index: gcc/config/moxie/moxie.h
===================================================================
--- gcc/config/moxie/moxie.h (revision 152972)
+++ gcc/config/moxie/moxie.h (working copy)
@@ -475,10 +475,6 @@
    an immediate operand on the target machine.  */
 #define LEGITIMATE_CONSTANT_P(X) 1
 
-/* A C expression that is 1 if the RTX X is a constant which is a
-   valid address.  */
-#define CONSTANT_ADDRESS_P(X) CONSTANT_P(X)
-
 /* A number, the maximum number of registers that can appear in a
    valid memory address.  */
 #define MAX_REGS_PER_ADDRESS 1
Index: gcc/config/m68hc11/m68hc11.h
===================================================================
--- gcc/config/m68hc11/m68hc11.h (revision 152972)
+++ gcc/config/m68hc11/m68hc11.h (working copy)
@@ -1108,9 +1108,6 @@
      && (GET_CODE (XEXP (operand, 0)) == POST_INC) \
      && (SP_REG_P (XEXP (XEXP (operand, 0), 0))))
 
-/* 1 if X is an rtx for a constant that is a valid address.  */
-#define CONSTANT_ADDRESS_P(X) (CONSTANT_P (X))
-
 /* Maximum number of registers that can appear in a valid memory address */
 #define MAX_REGS_PER_ADDRESS 2
 
Index: gcc/config/cris/cris.h
===================================================================
--- gcc/config/cris/cris.h (revision 152972)
+++ gcc/config/cris/cris.h (working copy)
@@ -956,8 +956,6 @@
 
 #define HAVE_POST_INCREMENT 1
 
-#define CONSTANT_ADDRESS_P(X) CONSTANT_P (X)
-
 /* Must be a compile-time constant, so we go with the highest value
    among all CRIS variants.  */
 #define MAX_REGS_PER_ADDRESS 2
Index: gcc/config/mn10300/mn10300.h
===================================================================
--- gcc/config/mn10300/mn10300.h (revision 152972)
+++ gcc/config/mn10300/mn10300.h (working copy)
@@ -618,10 +618,6 @@
    ? gen_rtx_MEM (Pmode, arg_pointer_rtx) \
    : (rtx) 0)
 
-/* 1 if X is an rtx for a constant that is a valid address.  */
-
-#define CONSTANT_ADDRESS_P(X)   (CONSTANT_P (X) && GET_CODE (X) != CONST_DOUBLE)
-
 /* Maximum number of registers that can appear in a valid memory address.  */
 
 #define MAX_REGS_PER_ADDRESS 2
Index: gcc/config/picochip/picochip.h
===================================================================
--- gcc/config/picochip/picochip.h (revision 152972)
+++ gcc/config/picochip/picochip.h (working copy)
@@ -471,8 +471,6 @@
 
 /* Addressing Modes  */
 
-#define CONSTANT_ADDRESS_P(X) CONSTANT_P(X)
-
 #define MAX_REGS_PER_ADDRESS 1
 
 /* Legitimize reload address tries machine dependent means of
Index: gcc/config/score/score.h
===================================================================
--- gcc/config/score/score.h (revision 152972)
+++ gcc/config/score/score.h (working copy)
@@ -688,9 +688,6 @@
 #define HAVE_PRE_MODIFY_REG             0
 #define HAVE_POST_MODIFY_REG            0
 
-/* Recognize any constant value that is a valid address.  */
-#define CONSTANT_ADDRESS_P(X)           CONSTANT_P (X)
-
 /* Maximum number of registers that can appear in a valid memory address.  */
 #define MAX_REGS_PER_ADDRESS            1
 
Index: gcc/config/bfin/bfin.h
===================================================================
--- gcc/config/bfin/bfin.h (revision 152972)
+++ gcc/config/bfin/bfin.h (working copy)
@@ -911,9 +911,6 @@
 
 /* Addressing Modes */
 
-/* Recognize any constant value that is a valid address.  */
-#define CONSTANT_ADDRESS_P(X) (CONSTANT_P (X))
-
 /* Nonzero if the constant value X is a legitimate general operand.
    symbol_ref are not legitimate and will be put into constant pool.
    See force_const_mem().

[Hans-Peter Nilsson]

> From: Nick Clifton <nickc@...>
> Date: Tue, 20 Oct 2009 09:46:09 +0100

>   Tested by building the following targets: avr-elf, bfin-elf,
>   cris-elf, fr30-elf, frv-elf, m32c-elf, m68hc12-elf, mep-elf,
>   mn10300-elf, moxie-elf, pdp11-aout, picochip-elf, score-elf,
>   xstormy16-elf.

> * defaults.h (CONSTANT_ADDRESS_P): Provide a default definition.
> Make sure that it does not allow CONST_DOUBLEs.
> * config/cris/cris.h (CONSTANT_ADDRESS_P): Delete.

I was going to suggest something more substantial than "make
all-gcc" (which I assume you mean by "build"), like running the
test-suite, all the bits being in place and all that, but...

> * defaults.h (CONSTANT_ADDRESS_P): Provide a default definition.
> * config/cris/cris.h (CONSTANT_ADDRESS_P): Delete.

...as this is functionally nothing more than a move of the
macro: ok, no worries.

brgds, H-P

[Hans-Peter Nilsson]

> From: Nick Clifton <nickc@...>
> Date: Tue, 20 Oct 2009 09:46:09 +0100

> Index: gcc/defaults.h
> ===================================================================
> --- gcc/defaults.h (revision 152972)
> +++ gcc/defaults.h (working copy)
> @@ -1154,4 +1154,10 @@
>  #define GO_IF_MODE_DEPENDENT_ADDRESS(X, WIN)
>  #endif
>  
> +/* For most ports anything that evaluates to a constant symbolic
> +   or integer value is acceptable as a constant address.  */
> +#ifndef CONSTANT_ADDRESS_P
> +#define CONSTANT_ADDRESS_P(X)   (CONSTANT_P (X) && GET_CODE (X) != CONST_DOUBLE)
> +#endif
> +

Wait, shouldn't that be
 #define CONSTANT_ADDRESS_P(X) \
 (CONSTANT_P (X) && LEGITIMATE_CONSTANT_P (X) && GET_CODE (X) != CONST_DOUBLE)
?  Beware, you're going to have to actually test that.

At some point I have to submit the gcc TLS-patches for CRIS;
they're for CRIS v32 only at the moment.  There, I have to
define CONSTANT_ADDRESS_P so as not to allow thread variables
that haven't been UNSPEC-adorned (through the movsi-expander).
But, when doing that and other needs to define
CONSTANT_ADDRESS_P, it'd help being able to include the default
definition, in case someone in the future finds out that it's
probably a good idea to also exclude, uh... (looking),
CONST_VECTOR and CONST_FIXED. ;)

Is it too late to ask for some variant of:

#define DEFAULT_CONSTANT_ADDRESS_P(X) \
 (CONSTANT_P (X) && LEGITIMATE_CONSTANT_P (X) && GET_CODE (X) != CONST_DOUBLE)
#ifndef CONSTANT_ADDRESS_P
#define CONSTANT_ADDRESS_P(X) DEFAULT_CONSTANT_ADDRESS_P (X)
#endif
?

PS. The documentation for LEGITIMATE_CONSTANT_P lies; you *have*
to define it to other than 1 in order to implement TLS.  But
that's another story.

brgds, H-P

[Hariharan Sandanagobalane]

This is okay (for picochip port).

Cheers
Hari

Nick Clifton wrote:

> Hi Guys,
>
>   This is a follow up to a comment from Jeff Law about a patch I
>   created for the MN10300 port.  We found that for that port that its
>   definition of CONSTANT_ADDRESS_P() as just CONSTANT_P() was a little
>   bit too permissive.  In particular it was allowing CONST_DOUBLEs to
>   be treated as constant addresses which tended to confuse other parts
>   of the compiler.
>
>   It turns out that quite a few backends use this definition of
>   CONSTANT_ADDRESS_P.  It is suggested in the tm.texi documentation
>   which is probably why it has happened.  So I am seeking approval for
>   the attached patch to address the problem.  It does two things:
>   Firstly it provides a default definition of CONSTANT_ADDRESS_P in
>   defaults.h which includes a test to make sure that CONST_DOUBLEs are
>   not allowed.  Secondly it updates all of the backends which used to
>   define CONSTANT_ADDRESS_P as CONSTANT_P so that they now use this
>   default definition.
>
>   Tested by building the following targets: avr-elf, bfin-elf,
>   cris-elf, fr30-elf, frv-elf, m32c-elf, m68hc12-elf, mep-elf,
>   mn10300-elf, moxie-elf, pdp11-aout, picochip-elf, score-elf,
>   xstormy16-elf.
>
>   I also ran an x86 bootstrap just to make sure that the change to
>   defaults.h did not break anything - it did not.
>
>   OK to apply ?
>
> Cheers
>   Nick
>
> gcc/ChangeLog
> 2009-10-20  Nick Clifton  <nickc@...>
>
> * defaults.h (CONSTANT_ADDRESS_P): Provide a default definition.
> Make sure that it does not allow CONST_DOUBLEs.
> * doc/tm.texi (CONSTANT_ADDRESS_P): Update description.
> * config/avr/avr.h (CONSTANT_ADDRESS_P): Delete.
> * config/bfin/bfin.h (CONSTANT_ADDRESS_P): Delete.
> * config/cris/cris.h (CONSTANT_ADDRESS_P): Delete.
> * config/fr30/fr30.h (CONSTANT_ADDRESS_P): Delete.
> * config/frv/frv.h (CONSTANT_ADDRESS_P): Delete.
> * config/m32c/m32c.h (CONSTANT_ADDRESS_P): Delete.
> * config/m68hc11/m68hc11.h (CONSTANT_ADDRESS_P): Delete.
> * config/mep/mep.h (CONSTANT_ADDRESS_P): Delete.
> * config/mn10300/mn10300.h (CONSTANT_ADDRESS_P): Delete.
> * config/moxie/moxie.h (CONSTANT_ADDRESS_P): Delete.
> * config/pdp11/pdp11.h (CONSTANT_ADDRESS_P): Delete.
> * config/picochip/picochip.h (CONSTANT_ADDRESS_P): Delete.
> * config/score/score.h (CONSTANT_ADDRESS_P): Delete.
> * config/stormy16/stormy16.h (CONSTANT_ADDRESS_P): Delete.
>
>  

[Jeff Law]

On 10/20/09 02:46, Nick Clifton wrote:

> Hi Guys,
>
>    This is a follow up to a comment from Jeff Law about a patch I
>    created for the MN10300 port.  We found that for that port that its
>    definition of CONSTANT_ADDRESS_P() as just CONSTANT_P() was a little
>    bit too permissive.  In particular it was allowing CONST_DOUBLEs to
>    be treated as constant addresses which tended to confuse other parts
>    of the compiler.
>
>    It turns out that quite a few backends use this definition of
>    CONSTANT_ADDRESS_P.  It is suggested in the tm.texi documentation
>    which is probably why it has happened.  So I am seeking approval for
>    the attached patch to address the problem.  It does two things:
>    Firstly it provides a default definition of CONSTANT_ADDRESS_P in
>    defaults.h which includes a test to make sure that CONST_DOUBLEs are
>    not allowed.  Secondly it updates all of the backends which used to
>    define CONSTANT_ADDRESS_P as CONSTANT_P so that they now use this
>    default definition.
>
>    Tested by building the following targets: avr-elf, bfin-elf,
>    cris-elf, fr30-elf, frv-elf, m32c-elf, m68hc12-elf, mep-elf,
>    mn10300-elf, moxie-elf, pdp11-aout, picochip-elf, score-elf,
>    xstormy16-elf.
>
>    I also ran an x86 bootstrap just to make sure that the change to
>    defaults.h did not break anything - it did not.
>
>    OK to apply ?
>
> Cheers
>    Nick
>
> gcc/ChangeLog
> 2009-10-20  Nick Clifton<nickc@...>
>
> * defaults.h (CONSTANT_ADDRESS_P): Provide a default definition.
> Make sure that it does not allow CONST_DOUBLEs.
> * doc/tm.texi (CONSTANT_ADDRESS_P): Update description.
> * config/avr/avr.h (CONSTANT_ADDRESS_P): Delete.
> * config/bfin/bfin.h (CONSTANT_ADDRESS_P): Delete.
> * config/cris/cris.h (CONSTANT_ADDRESS_P): Delete.
> * config/fr30/fr30.h (CONSTANT_ADDRESS_P): Delete.
> * config/frv/frv.h (CONSTANT_ADDRESS_P): Delete.
> * config/m32c/m32c.h (CONSTANT_ADDRESS_P): Delete.
> * config/m68hc11/m68hc11.h (CONSTANT_ADDRESS_P): Delete.
> * config/mep/mep.h (CONSTANT_ADDRESS_P): Delete.
> * config/mn10300/mn10300.h (CONSTANT_ADDRESS_P): Delete.
> * config/moxie/moxie.h (CONSTANT_ADDRESS_P): Delete.
> * config/pdp11/pdp11.h (CONSTANT_ADDRESS_P): Delete.
> * config/picochip/picochip.h (CONSTANT_ADDRESS_P): Delete.
> * config/score/score.h (CONSTANT_ADDRESS_P): Delete.
> * config/stormy16/stormy16.h (CONSTANT_ADDRESS_P): Delete.
>
>    

OK

[Nick Clifton]

Hi Hans-Peter,

> Wait, shouldn't that be
>  #define CONSTANT_ADDRESS_P(X) \
>  (CONSTANT_P (X) && LEGITIMATE_CONSTANT_P (X) && GET_CODE (X) != CONST_DOUBLE)
> ?  Beware, you're going to have to actually test that.

Yes it probably should.  I didn't to risk breaking things though, so I
just the "default" definition of CONSTANT_P and added the check for
CONST_DOUBLES.  At some point, in my copious free time, I will have a go
at adding the LEGITIMATE_CONSTANT_P and see if that breaks (or fixes)
anything.

Cheers
   Nick