Re: [PATCH], Patch #6, revision 3, Create pc-relative addressing insns

2019-07-18 Thread Segher Boessenkool
On Thu, Jul 18, 2019 at 02:20:43PM -0400, Michael Meissner wrote:
> On Tue, Jul 16, 2019 at 03:58:18PM -0500, Segher Boessenkool wrote:
> > > I did not move the initialization of the TOC_alias_set
> > > elsewhere, because in order to call TOC_alias_set, the code has already 
> > > called
> > > force_const_mem, create_TOC_reference, and gen_const_mem, so I didn't see 
> > > the
> > > point of adding a micro-optimization for this.
> > 
> > It gets rid of a call, but also of the conditional, and that makes this
> > eminently inlinable.  You could remove the getter function completely
> > even, access the variable directly.
> > 
> > But, sure, that's existing code only now.
> 
> We can fix this later.

Yup :-)

> I did rename the TARGET_NO_TOC macro to TARGET_NO_TOC_OR_PCREL which hopefully
> makes it more obvious when it should be true.

It does, thanks!


Segher


Re: [PATCH], Patch #6, revision 3, Create pc-relative addressing insns

2019-07-18 Thread Michael Meissner
On Tue, Jul 16, 2019 at 03:58:18PM -0500, Segher Boessenkool wrote:
> Hi Mike,
> 
> On Tue, Jul 16, 2019 at 02:19:14AM -0400, Michael Meissner wrote:
> > I have changed the TARGET_TOC to be TARGET_HAS_TOC in the aix, darwin, 
> > system
> > V, and Linux 64-bit headers.  Then in rs6000.h, TARGET_TOC is defined in 
> > terms
> > of TARGET_HAS_TOC and not pc-relative referencing.
> 
> Cool, thanks.  Good name, too.
> 
> > I discovered that TARGET_NO_TOC must not be set to be just !TARGET_TOC, 
> > since
> > TARGET_NO_TOC is used to create the elf_high, elf_low insns in 32-bit.
> 
> I don't know if your setting in sysv4.h works.  This file is used on so
> very many platforms, it is hard to predict if it works everywhere :-/

Well it is a mechanical change.

> > I did rename the static variable 'set' that contained the alias set to
> > TOC_alias_set.
> 
> :-)
> 
> > I did not move the initialization of the TOC_alias_set
> > elsewhere, because in order to call TOC_alias_set, the code has already 
> > called
> > force_const_mem, create_TOC_reference, and gen_const_mem, so I didn't see 
> > the
> > point of adding a micro-optimization for this.
> 
> It gets rid of a call, but also of the conditional, and that makes this
> eminently inlinable.  You could remove the getter function completely
> even, access the variable directly.
> 
> But, sure, that's existing code only now.

We can fix this later.

> > Index: gcc/config/rs6000/linux64.h
> > ===
> > --- gcc/config/rs6000/linux64.h (revision 273457)
> > +++ gcc/config/rs6000/linux64.h (working copy)
> > @@ -277,8 +277,8 @@ extern int dot_symbols;
> >  #ifndef RS6000_BI_ARCH
> >  
> >  /* 64-bit PowerPC Linux always has a TOC.  */
> > -#undef  TARGET_TOC
> > -#defineTARGET_TOC  1
> > +#undef  TARGET_HAS_TOC
> > +#defineTARGET_HAS_TOC  1
> 
> Fix the tab while your at it?  Not that it is consistent at all in this
> file, but having the undef and the define in different style... :-)
> 
> 
> So, what does TARGET_NO_TOC mean now?  Maybe a better name would help,
> or some documentation if not?
> 
> Looks good otherwise, okay for trunk.  Thanks!
> 
> (And watch out if it works on AIX and Darwin, please).

David has said the earlier patch works on AIX, and Ian has said he will test it
when he is able to.

I did rename the TARGET_NO_TOC macro to TARGET_NO_TOC_OR_PCREL which hopefully
makes it more obvious when it should be true.

Here is the patch I committed:

2019-07-18  Michael Meissner  

* config/rs6000/aix.h (TARGET_HAS_TOC): Rename TARGET_TOC to
TARGET_HAS_TOC.
(TARGET_TOC): Likewise.
(TARGET_NO_TOC): Delete here, define TARGET_NO_TOC_OR_PCREL in
rs6000.h.
* config/rs6000/darwin.h (TARGET_HAS_TOC): Rename TARGET_TOC to
TARGET_HAS_TOC.
(TARGET_TOC): Likewise.
(TARGET_NO_TOC): Delete here, define TARGET_NO_TOC_OR_PCREL in
rs6000.h.
* config/rs6000/linux64.h (TARGET_HAS_TOC): Rename TARGET_TOC to
TARGET_HAS_TOC.
(TARGET_TOC): Likewise.
* config/rs6000/rs6000.c (rs6000_option_override_internal): Add
check to require -mcmodel=medium for pc-relative addressing.
(create_TOC_reference): Add assertion for TARGET_TOC.
(rs6000_legitimize_address): Use TARGET_NO_TOC_OR_PCREL instead of
TARGET_NO_TOC.
(rs6000_emit_move): Likewise.
(TOC_alias_set): Rename TOC alias set static variable from 'set'
to 'TOC_alias_set'.
(get_TOC_alias_set): Likewise.
(output_toc): Use TARGET_NO_TOC_OR_PCREL instead of
TARGET_NO_TOC.
(rs6000_can_eliminate): Likewise.
* config/rs6000/rs6000.h (TARGET_TOC): Define in terms of
TARGET_HAS_TOC and not pc-relative.
(TARGET_NO_TOC_OR_PCREL): New macro to replace TARGET_NO_TOC.
* config/rs6000/sysv4.h (TARGET_HAS_TOC): Rename TARGET_TOC to
TARGET_HAS_TOC.
(TARGET_TOC): Likewise.
(TARGET_NO_TOC): Delete here, define TARGET_NO_TOC_OR_PCREL in
rs6000.h.

Index: gcc/config/rs6000/aix.h
===
--- gcc/config/rs6000/aix.h (revision 273578)
+++ gcc/config/rs6000/aix.h (working copy)
@@ -32,8 +32,7 @@
 #define TARGET_AIX_OS 1
 
 /* AIX always has a TOC.  */
-#define TARGET_NO_TOC 0
-#define TARGET_TOC 1
+#define TARGET_HAS_TOC 1
 #define FIXED_R2 1
 
 /* AIX allows r13 to be used in 32-bit mode.  */
Index: gcc/config/rs6000/darwin.h
===
--- gcc/config/rs6000/darwin.h  (revision 273578)
+++ gcc/config/rs6000/darwin.h  (working copy)
@@ -43,8 +43,7 @@
 
 /* We're not ever going to do TOCs.  */
 
-#define TARGET_TOC 0
-#define TARGET_NO_TOC 1
+#define TARGET_HAS_TOC 0
 
 /* Override the default rs6000 definition.  */
 #undef  PTRDIFF_TYPE
Index: gcc/config/rs6000/linux64.h

Re: [PATCH], Patch #6, revision 3, Create pc-relative addressing insns

2019-07-16 Thread Segher Boessenkool
Hi Mike,

On Tue, Jul 16, 2019 at 02:19:14AM -0400, Michael Meissner wrote:
> I have changed the TARGET_TOC to be TARGET_HAS_TOC in the aix, darwin, system
> V, and Linux 64-bit headers.  Then in rs6000.h, TARGET_TOC is defined in terms
> of TARGET_HAS_TOC and not pc-relative referencing.

Cool, thanks.  Good name, too.

> I discovered that TARGET_NO_TOC must not be set to be just !TARGET_TOC, since
> TARGET_NO_TOC is used to create the elf_high, elf_low insns in 32-bit.

I don't know if your setting in sysv4.h works.  This file is used on so
very many platforms, it is hard to predict if it works everywhere :-/

> I did rename the static variable 'set' that contained the alias set to
> TOC_alias_set.

:-)

> I did not move the initialization of the TOC_alias_set
> elsewhere, because in order to call TOC_alias_set, the code has already called
> force_const_mem, create_TOC_reference, and gen_const_mem, so I didn't see the
> point of adding a micro-optimization for this.

It gets rid of a call, but also of the conditional, and that makes this
eminently inlinable.  You could remove the getter function completely
even, access the variable directly.

But, sure, that's existing code only now.

> Index: gcc/config/rs6000/linux64.h
> ===
> --- gcc/config/rs6000/linux64.h   (revision 273457)
> +++ gcc/config/rs6000/linux64.h   (working copy)
> @@ -277,8 +277,8 @@ extern int dot_symbols;
>  #ifndef RS6000_BI_ARCH
>  
>  /* 64-bit PowerPC Linux always has a TOC.  */
> -#undef  TARGET_TOC
> -#define  TARGET_TOC  1
> +#undef  TARGET_HAS_TOC
> +#define  TARGET_HAS_TOC  1

Fix the tab while your at it?  Not that it is consistent at all in this
file, but having the undef and the define in different style... :-)


So, what does TARGET_NO_TOC mean now?  Maybe a better name would help,
or some documentation if not?

Looks good otherwise, okay for trunk.  Thanks!

(And watch out if it works on AIX and Darwin, please).


Segher