[v4,2/2] gcc: Drop obsolete INCLUDE_PTHREAD_H

  This is no longer used since 0a62889c7a155f8ed971860d68870dc9c46bb004, so
let's clean it up.

gcc/ChangeLog:
	* system.h: Drop unused INCLUDE_PTHREAD_H.

Signed-off-by: Sam James <sam@gentoo.org>
---
 gcc/system.h | 4 ----
 1 file changed, 4 deletions(-)
  

Thank you for fixing this. I am not familiar with this.
This generator code (genrvv-type-indexer.cc) is written by @kito. 

Kito ? Can you take a look at this?


juzhe.zhong@rivai.ai
 
From: Sam James
Date: 2023-03-14 08:23
To: gcc-patches
CC: Kito Cheng; Palmer Dabbelt; Andrew Waterman; Jim Wilson; Ju-Zhe Zhong; Sam James
Subject: [PATCH v4 2/2] gcc: Drop obsolete INCLUDE_PTHREAD_H
This is no longer used since 0a62889c7a155f8ed971860d68870dc9c46bb004, so
let's clean it up.
 
gcc/ChangeLog:
* system.h: Drop unused INCLUDE_PTHREAD_H.
 
Signed-off-by: Sam James <sam@gentoo.org>
---
gcc/system.h | 4 ----
1 file changed, 4 deletions(-)
 
diff --git a/gcc/system.h b/gcc/system.h
index cf45db3f97e..3fdad7abf1e 100644
--- a/gcc/system.h
+++ b/gcc/system.h
@@ -761,10 +761,6 @@ extern int vsnprintf (char *, size_t, const char *, va_list);
#endif
#endif
-#ifdef INCLUDE_PTHREAD_H
-#include <pthread.h>
-#endif
-
#ifdef INCLUDE_ISL
#ifdef HAVE_isl
#include <isl/options.h>
-- 
2.40.0
  

It's not the RISC-V part, so this requires a global maintainer there I think?

On Tue, Mar 14, 2023 at 8:28 AM juzhe.zhong@rivai.ai
<juzhe.zhong@rivai.ai> wrote:
>
> Thank you for fixing this. I am not familiar with this.
> This generator code (genrvv-type-indexer.cc) is written by @kito.
>
> Kito ? Can you take a look at this?
>
>
> juzhe.zhong@rivai.ai
>
> From: Sam James
> Date: 2023-03-14 08:23
> To: gcc-patches
> CC: Kito Cheng; Palmer Dabbelt; Andrew Waterman; Jim Wilson; Ju-Zhe Zhong; Sam James
> Subject: [PATCH v4 2/2] gcc: Drop obsolete INCLUDE_PTHREAD_H
> This is no longer used since 0a62889c7a155f8ed971860d68870dc9c46bb004, so
> let's clean it up.
>
> gcc/ChangeLog:
> * system.h: Drop unused INCLUDE_PTHREAD_H.
>
> Signed-off-by: Sam James <sam@gentoo.org>
> ---
> gcc/system.h | 4 ----
> 1 file changed, 4 deletions(-)
>
> diff --git a/gcc/system.h b/gcc/system.h
> index cf45db3f97e..3fdad7abf1e 100644
> --- a/gcc/system.h
> +++ b/gcc/system.h
> @@ -761,10 +761,6 @@ extern int vsnprintf (char *, size_t, const char *, va_list);
> #endif
> #endif
> -#ifdef INCLUDE_PTHREAD_H
> -#include <pthread.h>
> -#endif
> -
> #ifdef INCLUDE_ISL
> #ifdef HAVE_isl
> #include <isl/options.h>
> --
> 2.40.0
>
>
  

On 3/14/23 07:45, Kito Cheng via Gcc-patches wrote:
> It's not the RISC-V part, so this requires a global maintainer there I think?
It does.  I hadn't had a chance to dig into the history of the pthread.h 
include to understand why it was originally included which would be the 
first step in determining if it's actually safe to remove.

Jeff
  

Kito Cheng <kito.cheng@gmail.com> writes:

> It's not the RISC-V part, so this requires a global maintainer there I think?
>

Someone able to look at the system.h bit? It should be trivial, there's
no uses left and it was added purely for a bug like this in the past
(see commit message).

> On Tue, Mar 14, 2023 at 8:28 AM juzhe.zhong@rivai.ai
> <juzhe.zhong@rivai.ai> wrote:
>>
>> Thank you for fixing this. I am not familiar with this.
>> This generator code (genrvv-type-indexer.cc) is written by @kito.
>>
>> Kito ? Can you take a look at this?
>>
>>
>> juzhe.zhong@rivai.ai
>>
>> From: Sam James
>> Date: 2023-03-14 08:23
>> To: gcc-patches
>> CC: Kito Cheng; Palmer Dabbelt; Andrew Waterman; Jim Wilson; Ju-Zhe Zhong; Sam James
>> Subject: [PATCH v4 2/2] gcc: Drop obsolete INCLUDE_PTHREAD_H
>> This is no longer used since 0a62889c7a155f8ed971860d68870dc9c46bb004, so
>> let's clean it up.
>>
>> gcc/ChangeLog:
>> * system.h: Drop unused INCLUDE_PTHREAD_H.
>>
>> Signed-off-by: Sam James <sam@gentoo.org>
>> ---
>> gcc/system.h | 4 ----
>> 1 file changed, 4 deletions(-)
>>
>> diff --git a/gcc/system.h b/gcc/system.h
>> index cf45db3f97e..3fdad7abf1e 100644
>> --- a/gcc/system.h
>> +++ b/gcc/system.h
>> @@ -761,10 +761,6 @@ extern int vsnprintf (char *, size_t, const char *, va_list);
>> #endif
>> #endif
>> -#ifdef INCLUDE_PTHREAD_H
>> -#include <pthread.h>
>> -#endif
>> -
>> #ifdef INCLUDE_ISL
>> #ifdef HAVE_isl
>> #include <isl/options.h>
>> --
>> 2.40.0
>>
>>
  

On 3/31/23 12:44, Sam James wrote:
> 
> Kito Cheng <kito.cheng@gmail.com> writes:
> 
>> It's not the RISC-V part, so this requires a global maintainer there I think?
>>
> 
> Someone able to look at the system.h bit? It should be trivial, there's
> no uses left and it was added purely for a bug like this in the past
> (see commit message).
You assert that pthread.h is no longer used...  But ISTM you really need 
to go back to when the include was added, understand why it was added 
and explain why it is no longer needed.

Additionally, we're in a "regression fixes only" stage in our 
development cycle.  As far as I can tell this does not fix a regression 
and is thus going to be deferred to gcc-14 unless you have a compelling 
reason why it needs to change now.

Jeff
  

On Sun, Apr 2, 2023 at 12:55 PM Jeff Law via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
>
>
> On 3/31/23 12:44, Sam James wrote:
> >
> > Kito Cheng <kito.cheng@gmail.com> writes:
> >
> >> It's not the RISC-V part, so this requires a global maintainer there I think?
> >>
> >
> > Someone able to look at the system.h bit? It should be trivial, there's
> > no uses left and it was added purely for a bug like this in the past
> > (see commit message).
> You assert that pthread.h is no longer used...  But ISTM you really need
> to go back to when the include was added, understand why it was added
> and explain why it is no longer needed.

It was needed for the JIT front-end at the time used pthread_mutex_*
and pthread.h could use a poisoned identifier (I think it was calloc);
the INCLUDE_PTHREAD_H was added with r13-1350-g49d508065bdd36. The JIT
front-end moved to using C++11's mutex in r13-4164-g0a62889c7a155f and
moved away from using pthread.h but didn't remove INCLUDE_PTHREAD_H
support.

I hope that help explains why it is no longer needed and how it became
even unused.

Thanks,
Andrew Pinski

>
> Additionally, we're in a "regression fixes only" stage in our
> development cycle.  As far as I can tell this does not fix a regression
> and is thus going to be deferred to gcc-14 unless you have a compelling
> reason why it needs to change now.
>
> Jeff
>
  

On 4/2/23 14:06, Andrew Pinski wrote:
> On Sun, Apr 2, 2023 at 12:55 PM Jeff Law via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
>>
>>
>>
>> On 3/31/23 12:44, Sam James wrote:
>>>
>>> Kito Cheng <kito.cheng@gmail.com> writes:
>>>
>>>> It's not the RISC-V part, so this requires a global maintainer there I think?
>>>>
>>>
>>> Someone able to look at the system.h bit? It should be trivial, there's
>>> no uses left and it was added purely for a bug like this in the past
>>> (see commit message).
>> You assert that pthread.h is no longer used...  But ISTM you really need
>> to go back to when the include was added, understand why it was added
>> and explain why it is no longer needed.
> 
> It was needed for the JIT front-end at the time used pthread_mutex_*
> and pthread.h could use a poisoned identifier (I think it was calloc);
> the INCLUDE_PTHREAD_H was added with r13-1350-g49d508065bdd36. The JIT
> front-end moved to using C++11's mutex in r13-4164-g0a62889c7a155f and
> moved away from using pthread.h but didn't remove INCLUDE_PTHREAD_H
> support.
> 
> I hope that help explains why it is no longer needed and how it became
> even unused.
SO I'm confused, what does this have to do with RISC-V?

jeff
  

On Sun, Apr 2, 2023 at 1:07 PM Jeff Law <jeffreyalaw@gmail.com> wrote:
>
>
>
> On 4/2/23 14:06, Andrew Pinski wrote:
> > On Sun, Apr 2, 2023 at 12:55 PM Jeff Law via Gcc-patches
> > <gcc-patches@gcc.gnu.org> wrote:
> >>
> >>
> >>
> >> On 3/31/23 12:44, Sam James wrote:
> >>>
> >>> Kito Cheng <kito.cheng@gmail.com> writes:
> >>>
> >>>> It's not the RISC-V part, so this requires a global maintainer there I think?
> >>>>
> >>>
> >>> Someone able to look at the system.h bit? It should be trivial, there's
> >>> no uses left and it was added purely for a bug like this in the past
> >>> (see commit message).
> >> You assert that pthread.h is no longer used...  But ISTM you really need
> >> to go back to when the include was added, understand why it was added
> >> and explain why it is no longer needed.
> >
> > It was needed for the JIT front-end at the time used pthread_mutex_*
> > and pthread.h could use a poisoned identifier (I think it was calloc);
> > the INCLUDE_PTHREAD_H was added with r13-1350-g49d508065bdd36. The JIT
> > front-end moved to using C++11's mutex in r13-4164-g0a62889c7a155f and
> > moved away from using pthread.h but didn't remove INCLUDE_PTHREAD_H
> > support.
> >
> > I hope that help explains why it is no longer needed and how it became
> > even unused.
> SO I'm confused, what does this have to do with RISC-V?

So this was originally submitted with a patch against
gcc/config/riscv/genrvv-type-indexer.cc as it was noticed that during
the development of r13-6662-g0e6f87835ccabf, that INCLUDE_PTHREAD_H
became unused. The original patch against genrvv-type-indexer.cc was
to use INCLUDE_PTHREAD_H which was obviously wrong and then I helped
sam come up with the correct fix and we found that INCLUDE_PTHREAD_H
was not being used any more so Sam submitted a patch to remove it so
it would not be used accidently by anyone again.

Thanks,
Andrew

>
> jeff
  

Jeff Law <jeffreyalaw@gmail.com> writes:

> On 4/2/23 14:06, Andrew Pinski wrote:
>> On Sun, Apr 2, 2023 at 12:55 PM Jeff Law via Gcc-patches
>> <gcc-patches@gcc.gnu.org> wrote:
>>>
>>>
>>>
>>> On 3/31/23 12:44, Sam James wrote:
>>>>
>>>> Kito Cheng <kito.cheng@gmail.com> writes:
>>>>
>>>>> It's not the RISC-V part, so this requires a global maintainer there I think?
>>>>>
>>>>
>>>> Someone able to look at the system.h bit? It should be trivial, there's
>>>> no uses left and it was added purely for a bug like this in the past
>>>> (see commit message).
>>> You assert that pthread.h is no longer used...  But ISTM you really need
>>> to go back to when the include was added, understand why it was added
>>> and explain why it is no longer needed.
>> It was needed for the JIT front-end at the time used pthread_mutex_*
>> and pthread.h could use a poisoned identifier (I think it was calloc);
>> the INCLUDE_PTHREAD_H was added with r13-1350-g49d508065bdd36. The JIT
>> front-end moved to using C++11's mutex in r13-4164-g0a62889c7a155f and
>> moved away from using pthread.h but didn't remove INCLUDE_PTHREAD_H
>> support.
>> I hope that help explains why it is no longer needed and how it
>> became
>> even unused.
> SO I'm confused, what does this have to do with RISC-V?

Thanks Andrew for explaining. I took some of it as obvious given
the context in the series but I should've explained this more in the
commit message.

It's related to RISC-V in that I sent it at the same time while fixing
a musl poisoning issue in RISC-V, then cleaned up something while
working on it, and the thing I cleaned up was added for a similar issue
to the thing I fixed in RISC-V.

I don't mind if we defer it to GCC 14, but it seems overkill given it's
trivial and the include was only added in the course of GCC 13's
development.

But happy to resend with an improved commit message either way.

thanks,
sam