diff mbox series

GGC: Remove 'const char *' 'gt_ggc_mx', 'gt_pch_nx' variants (was: [PATCH] support ggc hash_map and hash_set)

Message ID 875y6wub1r.fsf@euler.schwinge.homeip.net
State New
Headers show
Series GGC: Remove 'const char *' 'gt_ggc_mx', 'gt_pch_nx' variants (was: [PATCH] support ggc hash_map and hash_set) | expand

Commit Message

Thomas Schwinge July 6, 2023, 6:53 p.m. UTC
Hi!

On 2014-09-01T21:56:28-0400, tsaunders@mozilla.com wrote:
> [...] this part [...]

... became commit b086d5308de0d25444243f482f2f3d1dfd3a9a62
(Subversion r214834), which added GGC support to 'hash_map', 'hash_set',
and converted to those a number of 'htab' instances.

It doesn't really interfere with my ongoing work, but I have doubts about
two functions that were added here:

> --- a/gcc/ggc.h
> +++ b/gcc/ggc.h

> +static inline void
> +gt_ggc_mx (const char *s)
> +{
> +  ggc_test_and_set_mark (const_cast<char *> (s));
> +}
> +
> +static inline void
> +gt_pch_nx (const char *)
> +{
> +}

If (in current sources) I put '__builtin_abort' calls into these
functions, those don't trigger, so the functions are (currently) unused,
at least in my configuration.  Moreover, comparing these two to other
string-related 'gt_ggc_mx' functions in (nowadays) 'gcc/ggc-page.cc', and
string-related 'gt_pch_nx' functions in (nowadays) 'gcc/stringpool.cc'
(..., which already did exist back then in 2014), we find that this
'gt_ggc_mx' doesn't call 'gt_ggc_m_S', so doesn't get the special string
handling, and this 'gt_pch_nx' doesn't call 'gt_pch_n_S' and also doesn't
'gt_pch_note_object' manually, so I wonder how that ever worked?  So
maybe these two in fact never were used?  Should we dare to put in the
attached "GGC: Remove 'const char *' 'gt_ggc_mx', 'gt_pch_nx' variants"?


Grüße
 Thomas


-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955

Comments

Richard Biener July 7, 2023, 6:20 a.m. UTC | #1
On Thu, Jul 6, 2023 at 8:53 PM Thomas Schwinge <thomas@codesourcery.com> wrote:
>
> Hi!
>
> On 2014-09-01T21:56:28-0400, tsaunders@mozilla.com wrote:
> > [...] this part [...]
>
> ... became commit b086d5308de0d25444243f482f2f3d1dfd3a9a62
> (Subversion r214834), which added GGC support to 'hash_map', 'hash_set',
> and converted to those a number of 'htab' instances.
>
> It doesn't really interfere with my ongoing work, but I have doubts about
> two functions that were added here:
>
> > --- a/gcc/ggc.h
> > +++ b/gcc/ggc.h
>
> > +static inline void
> > +gt_ggc_mx (const char *s)
> > +{
> > +  ggc_test_and_set_mark (const_cast<char *> (s));
> > +}
> > +
> > +static inline void
> > +gt_pch_nx (const char *)
> > +{
> > +}
>
> If (in current sources) I put '__builtin_abort' calls into these
> functions, those don't trigger, so the functions are (currently) unused,
> at least in my configuration.  Moreover, comparing these two to other
> string-related 'gt_ggc_mx' functions in (nowadays) 'gcc/ggc-page.cc', and
> string-related 'gt_pch_nx' functions in (nowadays) 'gcc/stringpool.cc'
> (..., which already did exist back then in 2014), we find that this
> 'gt_ggc_mx' doesn't call 'gt_ggc_m_S', so doesn't get the special string
> handling, and this 'gt_pch_nx' doesn't call 'gt_pch_n_S' and also doesn't
> 'gt_pch_note_object' manually, so I wonder how that ever worked?  So
> maybe these two in fact never were used?  Should we dare to put in the
> attached "GGC: Remove 'const char *' 'gt_ggc_mx', 'gt_pch_nx' variants"?

Are the variants in ggc-page.c/stringpool.cc used?  They don't seem to be
declared anywhere.

I notice that one is for a reference of const char * and one for the value.

But yes, I think we should remove the inlines if they are not needed.

Thanks,
Richard.

>
> Grüße
>  Thomas
>
>
> -----------------
> Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955
diff mbox series

Patch

From a1341d0e75ab20ee9ba09a1a8428c9d3dd2fd54a Mon Sep 17 00:00:00 2001
From: Thomas Schwinge <thomas@codesourcery.com>
Date: Thu, 6 Jul 2023 17:44:35 +0200
Subject: [PATCH] GGC: Remove 'const char *' 'gt_ggc_mx', 'gt_pch_nx' variants

Those were added in 2014 commit b086d5308de0d25444243f482f2f3d1dfd3a9a62
(Subversion r214834) "support ggc hash_map  and hash_set".

If (in current sources) I put '__builtin_abort' calls into these functions,
those don't trigger, so the functions are (currently) unused, at least in my
configuration.  Moreover, comparing these two to other string-related
'gt_ggc_mx' functions in (nowadays) 'gcc/ggc-page.cc', and string-related
'gt_pch_nx' functions in (nowadays) 'gcc/stringpool.cc' (..., which already did
exist back then in 2014), we find that this 'gt_ggc_mx' doesn't call
'gt_ggc_m_S', so doesn't get the special string handling, and this 'gt_pch_nx'
doesn't call 'gt_pch_n_S' and also doesn't 'gt_pch_note_object' manually, so I
wonder how that ever worked?  So maybe these two in fact never were used?

	gcc/
	* ggc.h (gt_ggc_mx (const char *s), gt_pch_nx (const char *)):
	Remove.
---
 gcc/ggc.h | 11 -----------
 1 file changed, 11 deletions(-)

diff --git a/gcc/ggc.h b/gcc/ggc.h
index 78eab7eaba6..1f3d665fc57 100644
--- a/gcc/ggc.h
+++ b/gcc/ggc.h
@@ -331,17 +331,6 @@  ggc_alloc_cleared_gimple_statement_stat (size_t s CXX_MEM_STAT_INFO)
   return (gimple *) ggc_internal_cleared_alloc (s PASS_MEM_STAT);
 }
 
-inline void
-gt_ggc_mx (const char *s)
-{
-  ggc_test_and_set_mark (const_cast<char *> (s));
-}
-
-inline void
-gt_pch_nx (const char *)
-{
-}
-
 inline void gt_pch_nx (bool) { }
 inline void gt_pch_nx (char) { }
 inline void gt_pch_nx (signed char) { }
-- 
2.34.1