public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* Re: [PATCH 0/9] start converting POINTER_SIZE to a hook
@ 2015-07-30 13:42 David Edelsohn
  2015-07-30 22:26 ` Richard Sandiford
  0 siblings, 1 reply; 15+ messages in thread
From: David Edelsohn @ 2015-07-30 13:42 UTC (permalink / raw)
  To: Richard Earnshaw, tbsaunde+gcc; +Cc: GCC Patches

On Wed, Jul 29, 2015 at 11:16:40AM +0100, Richard Earnshaw wrote:
> I'm getting a bit worried about the potential performance impact from
> all these indirect function call hooks.  This is a good example of when
> it's probably somewhat unnecessary.  I doubt that the compiler could
> function correctly if this ever changed in the middle of a function.

+1

- David

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/9] start converting POINTER_SIZE to a hook
  2015-07-30 13:42 [PATCH 0/9] start converting POINTER_SIZE to a hook David Edelsohn
@ 2015-07-30 22:26 ` Richard Sandiford
  0 siblings, 0 replies; 15+ messages in thread
From: Richard Sandiford @ 2015-07-30 22:26 UTC (permalink / raw)
  To: David Edelsohn; +Cc: Richard Earnshaw, tbsaunde+gcc, GCC Patches

David Edelsohn <dje.gcc@gmail.com> writes:
> On Wed, Jul 29, 2015 at 11:16:40AM +0100, Richard Earnshaw wrote:
>> I'm getting a bit worried about the potential performance impact from
>> all these indirect function call hooks.  This is a good example of when
>> it's probably somewhat unnecessary.  I doubt that the compiler could
>> function correctly if this ever changed in the middle of a function.
>
> +1

FWIW, the thread has already moved on from the idea of calling this hook
each time POINTER_SIZE is used.

The idea of caching POINTER_SIZE should if anything make things quicker,
since on many modern targets POINTER_SIZE depends on command-line options
and the values of those options are tested each time POINTER_SIZE is used.

Thanks,
Richard

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/9] start converting POINTER_SIZE to a hook
  2015-07-30  8:13               ` Trevor Saunders
@ 2015-07-30 22:30                 ` Richard Sandiford
  0 siblings, 0 replies; 15+ messages in thread
From: Richard Sandiford @ 2015-07-30 22:30 UTC (permalink / raw)
  To: Trevor Saunders
  Cc: Jeff Law, Richard Biener, tbsaunde+gcc, GCC Patches, richard.sandiford

Trevor Saunders <tbsaunde@tbsaunde.org> writes:
> On Wed, Jul 29, 2015 at 09:11:21AM +0100, Richard Sandiford wrote:
>> Trevor Saunders <tbsaunde@tbsaunde.org> writes:
>> > On Tue, Jul 28, 2015 at 09:24:17PM +0100, Richard Sandiford wrote:
>> >> Trevor Saunders <tbsaunde@tbsaunde.org> writes:
>> >> > On Mon, Jul 27, 2015 at 09:05:08PM +0100, Richard Sandiford wrote:
>> >> >> Alternatively we could have a new target_globals structure that is
>> >> >> initialised with the result of calling the hook.  If we do that though,
>> >> >> it might make sense to consolidate the hooks rather than have one for
>> >> >> every value.  E.g. having one function for UNITS_PER_WORD, one for
>> >> >> POINTER_SIZE, one for Pmode, etc., would lead to some very verbose
>> >> >> target code.
>> >> >
>> >> > so something like
>> >> >
>> >> > struct target_types
>> >> > {
>> >> >   unsigned long pointer_size;
>> >> >   ...
>> >> > };
>> >> >
>> >> > const target_types &targetm.get_type_data ()
>> >> >
>> >> > ? that seems pretty reasonable, and I wouldn't expect too many ordering
>> >> > issues, but who knows.  Its too bad nobody has taken on the big job of
>> >> > turning targetm into a class so we can hope for some devirt help from
>> >> > the compiler.
>> >> 
>> >> I was thinking more:
>> >> 
>> >>   void targetm.get_type_data (target_types *);
>> >> 
>> >> The caller could then initialise or post-process the defaults.  The
>> >> target_types would eventually end up in some target_globals structure.
>> >
>> > but wouldn't that mean the hook would need to initialize all the fields
>> > every time the hook was called?
>> 
>> Yeah, but the idea was that the hook would only be called once per
>> target initialisation and the result would be stored in a target_globals
>> structure.  Then places that use POINTER_SIZE would instead use the
>> cached target_globals structure rather than targetm.
>
> ok, personally I always found the have global state and update it
> appropriately approach a little distasteful, but I have to admitt it
> makes getting values faster than anything else, and otherwise works.  I
> guess if we ever care about threaded compilation for the jit or whatever
> that's not true, but that's not a bridge we need to cross now.

Yeah, and targetm isn't really any less global than the target_globals.
(The targetm fields aren't fixed.)

If we make GCC thread-safe or multi-target, I assume/hope we'd apply the
same approach to both targetm and the target_globals data.

Thanks,
Richard

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/9] start converting POINTER_SIZE to a hook
  2015-07-29  8:32             ` Richard Sandiford
@ 2015-07-30  8:13               ` Trevor Saunders
  2015-07-30 22:30                 ` Richard Sandiford
  0 siblings, 1 reply; 15+ messages in thread
From: Trevor Saunders @ 2015-07-30  8:13 UTC (permalink / raw)
  To: Jeff Law, Richard Biener, tbsaunde+gcc, GCC Patches, richard.sandiford

On Wed, Jul 29, 2015 at 09:11:21AM +0100, Richard Sandiford wrote:
> Trevor Saunders <tbsaunde@tbsaunde.org> writes:
> > On Tue, Jul 28, 2015 at 09:24:17PM +0100, Richard Sandiford wrote:
> >> Trevor Saunders <tbsaunde@tbsaunde.org> writes:
> >> > On Mon, Jul 27, 2015 at 09:05:08PM +0100, Richard Sandiford wrote:
> >> >> Alternatively we could have a new target_globals structure that is
> >> >> initialised with the result of calling the hook.  If we do that though,
> >> >> it might make sense to consolidate the hooks rather than have one for
> >> >> every value.  E.g. having one function for UNITS_PER_WORD, one for
> >> >> POINTER_SIZE, one for Pmode, etc., would lead to some very verbose
> >> >> target code.
> >> >
> >> > so something like
> >> >
> >> > struct target_types
> >> > {
> >> >   unsigned long pointer_size;
> >> >   ...
> >> > };
> >> >
> >> > const target_types &targetm.get_type_data ()
> >> >
> >> > ? that seems pretty reasonable, and I wouldn't expect too many ordering
> >> > issues, but who knows.  Its too bad nobody has taken on the big job of
> >> > turning targetm into a class so we can hope for some devirt help from
> >> > the compiler.
> >> 
> >> I was thinking more:
> >> 
> >>   void targetm.get_type_data (target_types *);
> >> 
> >> The caller could then initialise or post-process the defaults.  The
> >> target_types would eventually end up in some target_globals structure.
> >
> > but wouldn't that mean the hook would need to initialize all the fields
> > every time the hook was called?
> 
> Yeah, but the idea was that the hook would only be called once per
> target initialisation and the result would be stored in a target_globals
> structure.  Then places that use POINTER_SIZE would instead use the
> cached target_globals structure rather than targetm.

ok, personally I always found the have global state and update it
appropriately approach a little distasteful, but I have to admitt it
makes getting values faster than anything else, and otherwise works.  I
guess if we ever care about threaded compilation for the jit or whatever
that's not true, but that's not a bridge we need to cross now.

> For SWITCHABLE_TARGETs, the hook would be called only once for each
> subtarget used by the TU.  For other targets it would be called
> once for each change in subtarget (which is already very expensive without
> SWITCHABLE_TARGET -- targets that want it to be fast should move to
> SWITCHABLE_TARGET).
> 
> The disadvantage of:
> 
>    const target_types &targetm.get_type_data ()
> 
> is that it pushes the caching logic into targetm rather than sharing
> it between all ports.  This could be a particular problem for targets
> like MIPS that support a lot of variations.

true, though I was expecting for most targets you'd just have a couple
static const structs and just choose which one to return based onthe
subtarget which wouldn't be too bad.

Trev

> 
> Thanks,
> Richard
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/9] start converting POINTER_SIZE to a hook
  2015-07-29 10:40 ` Richard Earnshaw
@ 2015-07-30  4:55   ` Segher Boessenkool
  0 siblings, 0 replies; 15+ messages in thread
From: Segher Boessenkool @ 2015-07-30  4:55 UTC (permalink / raw)
  To: Richard Earnshaw; +Cc: tbsaunde+gcc, gcc-patches

On Wed, Jul 29, 2015 at 11:16:40AM +0100, Richard Earnshaw wrote:
> I'm getting a bit worried about the potential performance impact from
> all these indirect function call hooks.  This is a good example of when
> it's probably somewhat unnecessary.  I doubt that the compiler could
> function correctly if this ever changed in the middle of a function.

It is also very ugly and much harder to read: it is longer, with more
useless interpunction, and there is nothing that makes clear it is a
constant.


Segher

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/9] start converting POINTER_SIZE to a hook
  2015-07-27  3:10 tbsaunde+gcc
  2015-07-27  9:26 ` Richard Biener
@ 2015-07-29 10:40 ` Richard Earnshaw
  2015-07-30  4:55   ` Segher Boessenkool
  1 sibling, 1 reply; 15+ messages in thread
From: Richard Earnshaw @ 2015-07-29 10:40 UTC (permalink / raw)
  To: tbsaunde+gcc, gcc-patches

On 27/07/15 04:10, tbsaunde+gcc@tbsaunde.org wrote:
> From: Trevor Saunders <tbsaunde+gcc@tbsaunde.org>
> 
> Hi,
> 
> $subject.
> 
> patches individually bootstrapped + regtested on x86_64-linux-gnu, and run
> through config-list.mk with more patches removing usage of the macro.  Ok?
> 
> Trev
> 
> Trevor Saunders (9):
>   remove POINTER_SIZE_UNITS macro
>   add pointer_size target hook
>   target.h: change to use targetm.pointer_size instead of POINTER_SIZE
>   varasm.c: switch from POINTER_SIZE to targetm.pointer_size ()
>   ubsan.c: switch from POINTER_SIZE to targetm.pointer_size ()
>   tree-chkp.c: switch to targetm.pointer_size ()
>   stor-layout.c: switch to targetm.pointer_size ()
>   tree.c: switch to targetm.pointer_size ()
>   emit-rtl.c: switch to targetm.pointer_size ()
> 
>  gcc/c-family/c-cppbuiltin.c |  2 +-
>  gcc/defaults.h              |  3 ---
>  gcc/doc/tm.texi             |  7 +++++++
>  gcc/doc/tm.texi.in          |  2 ++
>  gcc/dwarf2asm.c             |  4 ++--
>  gcc/emit-rtl.c              |  5 +++--
>  gcc/lto/lto-object.c        |  3 ++-
>  gcc/stor-layout.c           |  9 +++++----
>  gcc/target.def              |  8 ++++++++
>  gcc/target.h                |  8 ++++++++
>  gcc/targhooks.c             |  8 ++++++++
>  gcc/targhooks.h             |  1 +
>  gcc/tree-chkp.c             | 14 ++++++++------
>  gcc/tree.c                  |  3 ++-
>  gcc/ubsan.c                 |  3 ++-
>  gcc/varasm.c                | 12 ++++++------
>  16 files changed, 65 insertions(+), 27 deletions(-)
> 

I'm getting a bit worried about the potential performance impact from
all these indirect function call hooks.  This is a good example of when
it's probably somewhat unnecessary.  I doubt that the compiler could
function correctly if this ever changed in the middle of a function.

It would be much better if targetm.pointer_size was a variable that
could be modified by back-end code on those few occasions when that
might be really necessary.

R.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/9] start converting POINTER_SIZE to a hook
  2015-07-29  4:44           ` Trevor Saunders
@ 2015-07-29  8:32             ` Richard Sandiford
  2015-07-30  8:13               ` Trevor Saunders
  0 siblings, 1 reply; 15+ messages in thread
From: Richard Sandiford @ 2015-07-29  8:32 UTC (permalink / raw)
  To: Trevor Saunders; +Cc: Jeff Law, Richard Biener, tbsaunde+gcc, GCC Patches

Trevor Saunders <tbsaunde@tbsaunde.org> writes:
> On Tue, Jul 28, 2015 at 09:24:17PM +0100, Richard Sandiford wrote:
>> Trevor Saunders <tbsaunde@tbsaunde.org> writes:
>> > On Mon, Jul 27, 2015 at 09:05:08PM +0100, Richard Sandiford wrote:
>> >> Alternatively we could have a new target_globals structure that is
>> >> initialised with the result of calling the hook.  If we do that though,
>> >> it might make sense to consolidate the hooks rather than have one for
>> >> every value.  E.g. having one function for UNITS_PER_WORD, one for
>> >> POINTER_SIZE, one for Pmode, etc., would lead to some very verbose
>> >> target code.
>> >
>> > so something like
>> >
>> > struct target_types
>> > {
>> >   unsigned long pointer_size;
>> >   ...
>> > };
>> >
>> > const target_types &targetm.get_type_data ()
>> >
>> > ? that seems pretty reasonable, and I wouldn't expect too many ordering
>> > issues, but who knows.  Its too bad nobody has taken on the big job of
>> > turning targetm into a class so we can hope for some devirt help from
>> > the compiler.
>> 
>> I was thinking more:
>> 
>>   void targetm.get_type_data (target_types *);
>> 
>> The caller could then initialise or post-process the defaults.  The
>> target_types would eventually end up in some target_globals structure.
>
> but wouldn't that mean the hook would need to initialize all the fields
> every time the hook was called?

Yeah, but the idea was that the hook would only be called once per
target initialisation and the result would be stored in a target_globals
structure.  Then places that use POINTER_SIZE would instead use the
cached target_globals structure rather than targetm.

For SWITCHABLE_TARGETs, the hook would be called only once for each
subtarget used by the TU.  For other targets it would be called
once for each change in subtarget (which is already very expensive without
SWITCHABLE_TARGET -- targets that want it to be fast should move to
SWITCHABLE_TARGET).

The disadvantage of:

   const target_types &targetm.get_type_data ()

is that it pushes the caching logic into targetm rather than sharing
it between all ports.  This could be a particular problem for targets
like MIPS that support a lot of variations.

Thanks,
Richard

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/9] start converting POINTER_SIZE to a hook
  2015-07-28 20:36         ` Richard Sandiford
@ 2015-07-29  4:44           ` Trevor Saunders
  2015-07-29  8:32             ` Richard Sandiford
  0 siblings, 1 reply; 15+ messages in thread
From: Trevor Saunders @ 2015-07-29  4:44 UTC (permalink / raw)
  To: Jeff Law, Richard Biener, tbsaunde+gcc, GCC Patches, rdsandiford

On Tue, Jul 28, 2015 at 09:24:17PM +0100, Richard Sandiford wrote:
> Trevor Saunders <tbsaunde@tbsaunde.org> writes:
> > On Mon, Jul 27, 2015 at 09:05:08PM +0100, Richard Sandiford wrote:
> >> Alternatively we could have a new target_globals structure that is
> >> initialised with the result of calling the hook.  If we do that though,
> >> it might make sense to consolidate the hooks rather than have one for
> >> every value.  E.g. having one function for UNITS_PER_WORD, one for
> >> POINTER_SIZE, one for Pmode, etc., would lead to some very verbose
> >> target code.
> >
> > so something like
> >
> > struct target_types
> > {
> >   unsigned long pointer_size;
> >   ...
> > };
> >
> > const target_types &targetm.get_type_data ()
> >
> > ? that seems pretty reasonable, and I wouldn't expect too many ordering
> > issues, but who knows.  Its too bad nobody has taken on the big job of
> > turning targetm into a class so we can hope for some devirt help from
> > the compiler.
> 
> I was thinking more:
> 
>   void targetm.get_type_data (target_types *);
> 
> The caller could then initialise or post-process the defaults.  The
> target_types would eventually end up in some target_globals structure.

but wouldn't that mean the hook would need to initialize all the fields
every time the hook was called?  I'd think you'd want to avoid that
work, and have a global or set of global constant  target_types structs,
and based on the target in use you could return the appropriate one.  a
target could also just have global one, but change its values when the
sub target changes, but having it be constant seems like a nicer design
for the target.  I guess that has the disadvantage that if the target
isn't a switchable target you need to reinitialize the whole struct
every time the hook is called on the off chance something has changed,
but that seems like the same thing that happens in your proposal?

Trev

> 
> Thanks,
> Richard

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/9] start converting POINTER_SIZE to a hook
  2015-07-28  5:52       ` Trevor Saunders
@ 2015-07-28 20:36         ` Richard Sandiford
  2015-07-29  4:44           ` Trevor Saunders
  0 siblings, 1 reply; 15+ messages in thread
From: Richard Sandiford @ 2015-07-28 20:36 UTC (permalink / raw)
  To: Trevor Saunders; +Cc: Jeff Law, Richard Biener, tbsaunde+gcc, GCC Patches

Trevor Saunders <tbsaunde@tbsaunde.org> writes:
> On Mon, Jul 27, 2015 at 09:05:08PM +0100, Richard Sandiford wrote:
>> Alternatively we could have a new target_globals structure that is
>> initialised with the result of calling the hook.  If we do that though,
>> it might make sense to consolidate the hooks rather than have one for
>> every value.  E.g. having one function for UNITS_PER_WORD, one for
>> POINTER_SIZE, one for Pmode, etc., would lead to some very verbose
>> target code.
>
> so something like
>
> struct target_types
> {
>   unsigned long pointer_size;
>   ...
> };
>
> const target_types &targetm.get_type_data ()
>
> ? that seems pretty reasonable, and I wouldn't expect too many ordering
> issues, but who knows.  Its too bad nobody has taken on the big job of
> turning targetm into a class so we can hope for some devirt help from
> the compiler.

I was thinking more:

  void targetm.get_type_data (target_types *);

The caller could then initialise or post-process the defaults.  The
target_types would eventually end up in some target_globals structure.

Thanks,
Richard

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/9] start converting POINTER_SIZE to a hook
  2015-07-27 20:15     ` Richard Sandiford
@ 2015-07-28  5:52       ` Trevor Saunders
  2015-07-28 20:36         ` Richard Sandiford
  0 siblings, 1 reply; 15+ messages in thread
From: Trevor Saunders @ 2015-07-28  5:52 UTC (permalink / raw)
  To: Jeff Law, Richard Biener, tbsaunde+gcc, GCC Patches, rdsandiford

On Mon, Jul 27, 2015 at 09:05:08PM +0100, Richard Sandiford wrote:
> Jeff Law <law@redhat.com> writes:
> > On 07/27/2015 03:17 AM, Richard Biener wrote:
> >> On Mon, Jul 27, 2015 at 5:10 AM,  <tbsaunde+gcc@tbsaunde.org> wrote:
> >>> From: Trevor Saunders <tbsaunde+gcc@tbsaunde.org>
> >>>
> >>> Hi,
> >>>
> >>> $subject.
> >>>
> >>> patches individually bootstrapped + regtested on x86_64-linux-gnu, and run
> >>> through config-list.mk with more patches removing usage of the macro.  Ok?
> >>
> >> With POINTER_SIZE now being expensive (target hook) you might consider
> >> moving most users to use pointer_sized_int_node or some other global
> >> derived from POINTER_SIZE.
> >>
> >> Which of course raises the question of why we are hookizing this...  if you'd
> >> want a truly switchable target you'd have to switch all global trees as well
> >> (or hookize them individually).
> > Not sure -- it doesn't remove any conditionally compiled code...
> >
> > One could easily argue that it's just another step on the path towards a 
> > switchable target -- which in and of itself is a reasonable design goal.
> >
> > Trevor, maybe a quick note on the motivation would help here...
> 
> I think at least we should use data hooks rather than function hooks,
> since this value should a constant within a subtarget.  It should only
> change for target_reinit.

I agree in principal, but I wasn't sure where all I might need to change
the values of the hooks, and of course I wondered if there might be some
crazy target where that's not good enough.

> Alternatively we could have a new target_globals structure that is
> initialised with the result of calling the hook.  If we do that though,
> it might make sense to consolidate the hooks rather than have one for
> every value.  E.g. having one function for UNITS_PER_WORD, one for
> POINTER_SIZE, one for Pmode, etc., would lead to some very verbose
> target code.

so something like

struct target_types
{
  unsigned long pointer_size;
  ...
};

const target_types &targetm.get_type_data ()

? that seems pretty reasonable, and I wouldn't expect too many ordering
issues, but who knows.  Its too bad nobody has taken on the big job of
turning targetm into a class so we can hope for some devirt help from
the compiler.

thanks!

Trev

> 
> Perhaps the main problem with these approaches is ensuring that the
> value is set up early enough.
> 
> Thanks,
> Richard

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/9] start converting POINTER_SIZE to a hook
  2015-07-27 16:20   ` Jeff Law
  2015-07-27 20:15     ` Richard Sandiford
@ 2015-07-28  3:47     ` Trevor Saunders
  1 sibling, 0 replies; 15+ messages in thread
From: Trevor Saunders @ 2015-07-28  3:47 UTC (permalink / raw)
  To: Jeff Law; +Cc: Richard Biener, tbsaunde+gcc, GCC Patches

On Mon, Jul 27, 2015 at 10:14:41AM -0600, Jeff Law wrote:
> On 07/27/2015 03:17 AM, Richard Biener wrote:
> >On Mon, Jul 27, 2015 at 5:10 AM,  <tbsaunde+gcc@tbsaunde.org> wrote:
> >>From: Trevor Saunders <tbsaunde+gcc@tbsaunde.org>
> >>
> >>Hi,
> >>
> >>$subject.
> >>
> >>patches individually bootstrapped + regtested on x86_64-linux-gnu, and run
> >>through config-list.mk with more patches removing usage of the macro.  Ok?
> >
> >With POINTER_SIZE now being expensive (target hook) you might consider
> >moving most users to use pointer_sized_int_node or some other global
> >derived from POINTER_SIZE.
> >
> >Which of course raises the question of why we are hookizing this...  if you'd
> >want a truly switchable target you'd have to switch all global trees as well
> >(or hookize them individually).
> Not sure -- it doesn't remove any conditionally compiled code...
> 
> One could easily argue that it's just another step on the path towards a
> switchable target -- which in and of itself is a reasonable design goal.

So my some what more intermediate goal was to have less files including
target specific headers, eventually getting to the point we might be
able to build a large part of the compiler independent of the
target.  Of course I think in the end really switchable targets would be
nice.

Trev

> 
> Trevor, maybe a quick note on the motivation would help here...
> 
> jeff
> 
> 
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/9] start converting POINTER_SIZE to a hook
  2015-07-27 16:20   ` Jeff Law
@ 2015-07-27 20:15     ` Richard Sandiford
  2015-07-28  5:52       ` Trevor Saunders
  2015-07-28  3:47     ` Trevor Saunders
  1 sibling, 1 reply; 15+ messages in thread
From: Richard Sandiford @ 2015-07-27 20:15 UTC (permalink / raw)
  To: Jeff Law; +Cc: Richard Biener, tbsaunde+gcc, GCC Patches

Jeff Law <law@redhat.com> writes:
> On 07/27/2015 03:17 AM, Richard Biener wrote:
>> On Mon, Jul 27, 2015 at 5:10 AM,  <tbsaunde+gcc@tbsaunde.org> wrote:
>>> From: Trevor Saunders <tbsaunde+gcc@tbsaunde.org>
>>>
>>> Hi,
>>>
>>> $subject.
>>>
>>> patches individually bootstrapped + regtested on x86_64-linux-gnu, and run
>>> through config-list.mk with more patches removing usage of the macro.  Ok?
>>
>> With POINTER_SIZE now being expensive (target hook) you might consider
>> moving most users to use pointer_sized_int_node or some other global
>> derived from POINTER_SIZE.
>>
>> Which of course raises the question of why we are hookizing this...  if you'd
>> want a truly switchable target you'd have to switch all global trees as well
>> (or hookize them individually).
> Not sure -- it doesn't remove any conditionally compiled code...
>
> One could easily argue that it's just another step on the path towards a 
> switchable target -- which in and of itself is a reasonable design goal.
>
> Trevor, maybe a quick note on the motivation would help here...

I think at least we should use data hooks rather than function hooks,
since this value should a constant within a subtarget.  It should only
change for target_reinit.

Alternatively we could have a new target_globals structure that is
initialised with the result of calling the hook.  If we do that though,
it might make sense to consolidate the hooks rather than have one for
every value.  E.g. having one function for UNITS_PER_WORD, one for
POINTER_SIZE, one for Pmode, etc., would lead to some very verbose
target code.

Perhaps the main problem with these approaches is ensuring that the
value is set up early enough.

Thanks,
Richard

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/9] start converting POINTER_SIZE to a hook
  2015-07-27  9:26 ` Richard Biener
@ 2015-07-27 16:20   ` Jeff Law
  2015-07-27 20:15     ` Richard Sandiford
  2015-07-28  3:47     ` Trevor Saunders
  0 siblings, 2 replies; 15+ messages in thread
From: Jeff Law @ 2015-07-27 16:20 UTC (permalink / raw)
  To: Richard Biener, tbsaunde+gcc; +Cc: GCC Patches

On 07/27/2015 03:17 AM, Richard Biener wrote:
> On Mon, Jul 27, 2015 at 5:10 AM,  <tbsaunde+gcc@tbsaunde.org> wrote:
>> From: Trevor Saunders <tbsaunde+gcc@tbsaunde.org>
>>
>> Hi,
>>
>> $subject.
>>
>> patches individually bootstrapped + regtested on x86_64-linux-gnu, and run
>> through config-list.mk with more patches removing usage of the macro.  Ok?
>
> With POINTER_SIZE now being expensive (target hook) you might consider
> moving most users to use pointer_sized_int_node or some other global
> derived from POINTER_SIZE.
>
> Which of course raises the question of why we are hookizing this...  if you'd
> want a truly switchable target you'd have to switch all global trees as well
> (or hookize them individually).
Not sure -- it doesn't remove any conditionally compiled code...

One could easily argue that it's just another step on the path towards a 
switchable target -- which in and of itself is a reasonable design goal.

Trevor, maybe a quick note on the motivation would help here...

jeff



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/9] start converting POINTER_SIZE to a hook
  2015-07-27  3:10 tbsaunde+gcc
@ 2015-07-27  9:26 ` Richard Biener
  2015-07-27 16:20   ` Jeff Law
  2015-07-29 10:40 ` Richard Earnshaw
  1 sibling, 1 reply; 15+ messages in thread
From: Richard Biener @ 2015-07-27  9:26 UTC (permalink / raw)
  To: tbsaunde+gcc; +Cc: GCC Patches

On Mon, Jul 27, 2015 at 5:10 AM,  <tbsaunde+gcc@tbsaunde.org> wrote:
> From: Trevor Saunders <tbsaunde+gcc@tbsaunde.org>
>
> Hi,
>
> $subject.
>
> patches individually bootstrapped + regtested on x86_64-linux-gnu, and run
> through config-list.mk with more patches removing usage of the macro.  Ok?

With POINTER_SIZE now being expensive (target hook) you might consider
moving most users to use pointer_sized_int_node or some other global
derived from POINTER_SIZE.

Which of course raises the question of why we are hookizing this...  if you'd
want a truly switchable target you'd have to switch all global trees as well
(or hookize them individually).

Richard.

> Trev
>
> Trevor Saunders (9):
>   remove POINTER_SIZE_UNITS macro
>   add pointer_size target hook
>   target.h: change to use targetm.pointer_size instead of POINTER_SIZE
>   varasm.c: switch from POINTER_SIZE to targetm.pointer_size ()
>   ubsan.c: switch from POINTER_SIZE to targetm.pointer_size ()
>   tree-chkp.c: switch to targetm.pointer_size ()
>   stor-layout.c: switch to targetm.pointer_size ()
>   tree.c: switch to targetm.pointer_size ()
>   emit-rtl.c: switch to targetm.pointer_size ()
>
>  gcc/c-family/c-cppbuiltin.c |  2 +-
>  gcc/defaults.h              |  3 ---
>  gcc/doc/tm.texi             |  7 +++++++
>  gcc/doc/tm.texi.in          |  2 ++
>  gcc/dwarf2asm.c             |  4 ++--
>  gcc/emit-rtl.c              |  5 +++--
>  gcc/lto/lto-object.c        |  3 ++-
>  gcc/stor-layout.c           |  9 +++++----
>  gcc/target.def              |  8 ++++++++
>  gcc/target.h                |  8 ++++++++
>  gcc/targhooks.c             |  8 ++++++++
>  gcc/targhooks.h             |  1 +
>  gcc/tree-chkp.c             | 14 ++++++++------
>  gcc/tree.c                  |  3 ++-
>  gcc/ubsan.c                 |  3 ++-
>  gcc/varasm.c                | 12 ++++++------
>  16 files changed, 65 insertions(+), 27 deletions(-)
>
> --
> 2.4.0
>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 0/9] start converting POINTER_SIZE to a hook
@ 2015-07-27  3:10 tbsaunde+gcc
  2015-07-27  9:26 ` Richard Biener
  2015-07-29 10:40 ` Richard Earnshaw
  0 siblings, 2 replies; 15+ messages in thread
From: tbsaunde+gcc @ 2015-07-27  3:10 UTC (permalink / raw)
  To: gcc-patches

From: Trevor Saunders <tbsaunde+gcc@tbsaunde.org>

Hi,

$subject.

patches individually bootstrapped + regtested on x86_64-linux-gnu, and run
through config-list.mk with more patches removing usage of the macro.  Ok?

Trev

Trevor Saunders (9):
  remove POINTER_SIZE_UNITS macro
  add pointer_size target hook
  target.h: change to use targetm.pointer_size instead of POINTER_SIZE
  varasm.c: switch from POINTER_SIZE to targetm.pointer_size ()
  ubsan.c: switch from POINTER_SIZE to targetm.pointer_size ()
  tree-chkp.c: switch to targetm.pointer_size ()
  stor-layout.c: switch to targetm.pointer_size ()
  tree.c: switch to targetm.pointer_size ()
  emit-rtl.c: switch to targetm.pointer_size ()

 gcc/c-family/c-cppbuiltin.c |  2 +-
 gcc/defaults.h              |  3 ---
 gcc/doc/tm.texi             |  7 +++++++
 gcc/doc/tm.texi.in          |  2 ++
 gcc/dwarf2asm.c             |  4 ++--
 gcc/emit-rtl.c              |  5 +++--
 gcc/lto/lto-object.c        |  3 ++-
 gcc/stor-layout.c           |  9 +++++----
 gcc/target.def              |  8 ++++++++
 gcc/target.h                |  8 ++++++++
 gcc/targhooks.c             |  8 ++++++++
 gcc/targhooks.h             |  1 +
 gcc/tree-chkp.c             | 14 ++++++++------
 gcc/tree.c                  |  3 ++-
 gcc/ubsan.c                 |  3 ++-
 gcc/varasm.c                | 12 ++++++------
 16 files changed, 65 insertions(+), 27 deletions(-)

-- 
2.4.0

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2015-07-30 21:37 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-07-30 13:42 [PATCH 0/9] start converting POINTER_SIZE to a hook David Edelsohn
2015-07-30 22:26 ` Richard Sandiford
  -- strict thread matches above, loose matches on Subject: below --
2015-07-27  3:10 tbsaunde+gcc
2015-07-27  9:26 ` Richard Biener
2015-07-27 16:20   ` Jeff Law
2015-07-27 20:15     ` Richard Sandiford
2015-07-28  5:52       ` Trevor Saunders
2015-07-28 20:36         ` Richard Sandiford
2015-07-29  4:44           ` Trevor Saunders
2015-07-29  8:32             ` Richard Sandiford
2015-07-30  8:13               ` Trevor Saunders
2015-07-30 22:30                 ` Richard Sandiford
2015-07-28  3:47     ` Trevor Saunders
2015-07-29 10:40 ` Richard Earnshaw
2015-07-30  4:55   ` Segher Boessenkool

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).