Re: [PATCH] Support g++ 4.8 as a host compiler.

2023-10-07 Thread Sam James


Jeff Law  writes:

> On 10/4/23 16:19, Roger Sayle wrote:
>> The recent patch to remove poly_int_pod triggers a bug in g++
>> 4.8.5's
>> C++ 11 support which mistakenly believes poly_uint16 has a non-trivial
>> constructor.  This in turn prohibits it from being used as a member in
>> a union (rtxunion) that constructed statically, resulting in a (fatal)
>> error during stage 1.  A workaround is to add an explicit constructor
>> to the problematic union, which allows mainline to be bootstrapped with
>> the system compiler on older RedHat 7 systems.
>> This patch has been tested on x86_64-pc-linux-gnu where it allows a
>> bootstrap to complete when using g++ 4.8.5 as the host compiler.
>> Ok for mainline?
>> 2023-10-04  Roger Sayle  
>> gcc/ChangeLog
>>  * rtl.h (rtx_def::u): Add explicit constructor to workaround
>>  issue using g++ 4.8 as a host compiler.
> I think the bigger question is whether or not we're going to step
> forward on the minimum build requirements.
>
> My recollection was we settled on gcc-4.8 for the benefit of RHEL 7
> and Centos 7 which are rapidly approaching EOL (June 2024).
>
> I would certainly support stepping forward to a more modern compiler
> for the build requirements, which might make this patch obsolete.

See also richi and jakub's comments at 
https://inbox.sourceware.org/gcc-patches/mpt5y3ppio0@arm.com/T/#m985295bedaadb47aa0b9ba63b7cb69a660a108bb.

>
> Jeff



Re: [PATCH] Do not prepend target triple to -fuse-ld=lld,mold.

2023-10-15 Thread Sam James


Tatsuyuki Ishi  writes:

> lld and mold are platform-agnostic and not prefixed with target triple.
> Prepending the target triple makes it less likely to find the intended
> linker executable.
>
> A potential breaking change is that we no longer try to search for
> triple-prefixed lld/mold binaries anymore. However, since there doesn't
> seem to be support to build LLVM or mold with triple-prefixed executable
> names, it seems better to just not bother with that case.
>

We do provide those symlinks for LLVM in Gentoo, but we don't for mold.


>   PR driver/111605
>
> gcc/Changelog:
>
>   * collect2.cc (main): Do not prepend target triple to
>   -fuse-ld=lld,mold.
> ---
>  gcc/collect2.cc | 13 -
>  1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/gcc/collect2.cc b/gcc/collect2.cc
> index 63b9a0c233a..c943f9f577c 100644
> --- a/gcc/collect2.cc
> +++ b/gcc/collect2.cc
> @@ -865,12 +865,15 @@ main (int argc, char **argv)
>int i;
>  
>for (i = 0; i < USE_LD_MAX; i++)
> -full_ld_suffixes[i]
>  #ifdef CROSS_DIRECTORY_STRUCTURE
> -  = concat (target_machine, "-", ld_suffixes[i], NULL);
> -#else
> -  = ld_suffixes[i];
> -#endif
> +/* lld and mold are platform-agnostic and not prefixed with target
> +   triple.  */
> +if (!(i == USE_LLD_LD || i == USE_MOLD_LD))
> +  full_ld_suffixes[i] = concat (target_machine, "-", ld_suffixes[i],
> + NULL);
> +else
> +#endif
> +  full_ld_suffixes[i] = ld_suffixes[i];
>  
>p = argv[0] + strlen (argv[0]);
>while (p != argv[0] && !IS_DIR_SEPARATOR (p[-1]))



Re: [PATCH] genemit: Split insn-emit.cc into ten files.

2023-10-16 Thread Sam James


Robin Dapp  writes:

> Hi,
>
> the attached v2 includes Tamar's suggestion of keeping the current
> stdout behavior.  When no output files are passed (via -O) the output
> is written to stdout as before.
>
> Tamar also mentioned off-list that, similar to match.pd, it might make
> sense to balance the partitions in a better way than a fixed number
> of patterns threshold.  That's a good idea but I'd rather do that
> separately as the current approach already helps considerably.
>
> Attached v2 was bootstrapped and regtested on power10, aarch64 and
> x86 are still running.
> Stefan also tested v1 on s390 where the partitioning does not help
> but also doesn't slow anything down.  insn-emit.cc isn't very large
> to begin with on s390.

I tested v1 on x86/arm64, I'll do at least the same for v2. (I also
backported it to 13 for my own purposes locally and everything seemed
fine there.)

I didn't notice any change on my x86 machine but it's already
quite powerful and I didn't have a chance to try on anything weaker,
so I wasn't too surprised.

>
> Regards
>  Robin
>
> From 34d05113a4e3c7e83a4731020307e26c1144af69 Mon Sep 17 00:00:00 2001
> From: Robin Dapp 
> Date: Thu, 12 Oct 2023 11:23:26 +0200
> Subject: [PATCH v2] genemit: Split insn-emit.cc into several partitions.
>
> On riscv insn-emit.cc has grown to over 1.2 mio lines of code and
> compiling it takes considerable time.
> Therefore, this patch adjust genemit to create several partitions
> (insn-emit-1.cc to insn-emit-n.cc).  In order to do so it first counts
> the number of available patterns, calculates the number of patterns per
> file and starts a new file whenever that number is reached.
>
> Similar to match.pd a configure option --with-emitinsn-partitions=num
> is introduced that makes the number of partition configurable.
>
> gcc/ChangeLog:
>
>   PR bootstrap/84402
>   PR target/111600
>
>   * Makefile.in: Handle split insn-emit.cc.
>   * configure: Regenerate.
>   * configure.ac: Add --with-insnemit-partitions.
>   * genemit.cc (output_peephole2_scratches): Print to file instead
>   of stdout.
>   (print_code): Ditto.
>   (gen_rtx_scratch): Ditto.
>   (gen_exp): Ditto.
>   (gen_emit_seq): Ditto.
>   (emit_c_code): Ditto.
>   (gen_insn): Ditto.
>   (gen_expand): Ditto.
>   (gen_split): Ditto.
>   (output_add_clobbers): Ditto.
>   (output_added_clobbers_hard_reg_p): Ditto.
>   (print_overload_arguments): Ditto.
>   (print_overload_test): Ditto.
>   (handle_overloaded_code_for): Ditto.
>   (handle_overloaded_gen): Ditto.
>   (print_header): New function.
>   (handle_arg): New function.
>   (main): Split output into 10 files.
>   * gensupport.cc (count_patterns): New function.
>   * gensupport.h (count_patterns): Define.
>   * read-md.cc (md_reader::print_md_ptr_loc): Add file argument.
>   * read-md.h (class md_reader): Change definition.
> ---
>  gcc/Makefile.in   |  38 +++-
>  gcc/configure |  24 ++-
>  gcc/configure.ac  |  13 ++
>  gcc/genemit.cc| 536 +-
>  gcc/gensupport.cc |  36 
>  gcc/gensupport.h  |   1 +
>  gcc/read-md.cc|   4 +-
>  gcc/read-md.h |   2 +-
>  8 files changed, 399 insertions(+), 255 deletions(-)
>
> diff --git a/gcc/Makefile.in b/gcc/Makefile.in
> index 9cc16268abf..ca0a616f768 100644
> --- a/gcc/Makefile.in
> +++ b/gcc/Makefile.in
> @@ -236,6 +236,13 @@ GIMPLE_MATCH_PD_SEQ_O = $(patsubst %, gimple-match-%.o, 
> $(MATCH_SPLITS_SEQ))
>  GENERIC_MATCH_PD_SEQ_SRC = $(patsubst %, generic-match-%.cc, 
> $(MATCH_SPLITS_SEQ))
>  GENERIC_MATCH_PD_SEQ_O = $(patsubst %, generic-match-%.o, 
> $(MATCH_SPLITS_SEQ))
>  
> +# The number of splits to be made for the insn-emit files.
> +NUM_INSNEMIT_SPLITS = @DEFAULT_INSNEMIT_PARTITIONS@
> +INSNEMIT_SPLITS_SEQ = $(wordlist 1,$(NUM_INSNEMIT_SPLITS),$(one_to_))
> +INSNEMIT_SEQ_SRC = $(patsubst %, insn-emit-%.cc, $(INSNEMIT_SPLITS_SEQ))
> +INSNEMIT_SEQ_TMP = $(patsubst %, tmp-emit-%.cc, $(INSNEMIT_SPLITS_SEQ))
> +INSNEMIT_SEQ_O = $(patsubst %, insn-emit-%.o, $(INSNEMIT_SPLITS_SEQ))
> +
>  # These files are to have specific diagnostics suppressed, or are not to
>  # be subject to -Werror:
>  # flex output may yield harmless "no previous prototype" warnings
> @@ -1354,7 +1361,7 @@ OBJS = \
>   insn-attrtab.o \
>   insn-automata.o \
>   insn-dfatab.o \
> - insn-emit.o \
> + $(INSNEMIT_SEQ_O) \
>   insn-extract.o \
>   insn-latencytab.o \
>   insn-modes.o \
> @@ -1852,7 +1859,8 @@ TREECHECKING = @TREECHECKING@
>  FULL_DRIVER_NAME=$(target_noncanonical)-gcc-$(version)$(exeext)
>  
>  MOSTLYCLEANFILES = insn-flags.h insn-config.h insn-codes.h \
> - insn-output.cc insn-recog.cc insn-emit.cc insn-extract.cc insn-peep.cc \
> + insn-output.cc insn-recog.cc $(INSNEMIT_SEQ_SRC) \
> + insn-extract.cc insn-peep.cc \
>   insn-attr.h insn-attr-common.h insn-attrtab.cc insn-dfatab.cc \
>   insn-latencyt

Re: [PATCH] genemit: Split insn-emit.cc into ten files.

2023-10-16 Thread Sam James


Robin Dapp  writes:

> Hi,
>
> the attached v2 includes Tamar's suggestion of keeping the current
> stdout behavior.  When no output files are passed (via -O) the output
> is written to stdout as before.
>
> Tamar also mentioned off-list that, similar to match.pd, it might make
> sense to balance the partitions in a better way than a fixed number
> of patterns threshold.  That's a good idea but I'd rather do that
> separately as the current approach already helps considerably.
>
> Attached v2 was bootstrapped and regtested on power10, aarch64 and
> x86 are still running.
> Stefan also tested v1 on s390 where the partitioning does not help
> but also doesn't slow anything down.  insn-emit.cc isn't very large
> to begin with on s390.
>
> Regards
>  Robin
>
> From 34d05113a4e3c7e83a4731020307e26c1144af69 Mon Sep 17 00:00:00 2001
> From: Robin Dapp 
> Date: Thu, 12 Oct 2023 11:23:26 +0200
> Subject: [PATCH v2] genemit: Split insn-emit.cc into several partitions.
>
> On riscv insn-emit.cc has grown to over 1.2 mio lines of code and
> compiling it takes considerable time.
> Therefore, this patch adjust genemit to create several partitions
> (insn-emit-1.cc to insn-emit-n.cc).  In order to do so it first counts
> the number of available patterns, calculates the number of patterns per
> file and starts a new file whenever that number is reached.
>
> Similar to match.pd a configure option --with-emitinsn-partitions=num
> is introduced that makes the number of partition configurable.
>

Natively, things seem fine, but for cross, I get failures on a few
targets (hppa2.0-unknown-linux-gnu, hppa64-unknown-linux-gnu).

With ./configure --host=x86_64-pc-linux-gnu
--target=hppa2.0-unknown-linux-gnu --build=x86_64-pc-linux-gnu && make
-j$(nproc), I get a bunch of stuff like:

mv: cannot stat 'tmp-emit-9.cc': No such file or directory
echo timestamp > s-insn-emit-8
mv: cannot stat 'tmp-emit-10.cc': No such file or directory
make[2]: *** [Makefile:2598: s-insn-emit-9] Error 1
make[2]: *** Waiting for unfinished jobs
make[2]: *** [Makefile:2598: s-insn-emit-10] Error 1


Re: [PATCH v2] gcc: Introduce -fhardened

2023-10-19 Thread Sam James


Richard Biener  writes:

> On Wed, Oct 11, 2023 at 10:48 PM Marek Polacek  wrote:
>>
>> On Tue, Sep 19, 2023 at 10:58:19AM -0400, Marek Polacek wrote:
>> > On Mon, Sep 18, 2023 at 08:57:39AM +0200, Richard Biener wrote:
>> > > On Fri, Sep 15, 2023 at 5:09 PM Marek Polacek via Gcc-patches
>> > >  wrote:
>> > > >
>> > > > Bootstrapped/regtested on x86_64-pc-linux-gnu, 
>> > > > powerpc64le-unknown-linux-gnu,
>> > > > and aarch64-unknown-linux-gnu; ok for trunk?
>> > > >
>> > > > -- >8 --
>> > > > In 
>> > > > I proposed -fhardened, a new umbrella option that enables a reasonable 
>> > > > set
>> > > > of hardening flags.  The read of the room seems to be that the option
>> > > > would be useful.  So here's a patch implementing that option.
>> > > >
>> > > > Currently, -fhardened enables:
>> > > >
>> > > >   -D_FORTIFY_SOURCE=3 (or =2 for older glibcs)
>> > > >   -D_GLIBCXX_ASSERTIONS
>> > > >   -ftrivial-auto-var-init=pattern
>
> I think =zero is much better here given the overhead is way
> cheaper and pointers get a more reliable behavior.

Yes please, as I wouldn't want us to use =pattern distro-wide.

>
>> > > >   -fPIE  -pie  -Wl,-z,relro,-z,now
>> > > >   -fstack-protector-strong
>> > > >   -fstack-clash-protection
>> > > >   -fcf-protection=full (x86 GNU/Linux only)
>> > > >
>> > > > -fhardened will not override options that were specified on the 
>> > > > command line
>> > > > (before or after -fhardened).  For example,
>> > > >
>> > > >  -D_FORTIFY_SOURCE=1 -fhardened
>> > > >
>> > > > means that _FORTIFY_SOURCE=1 will be used.  Similarly,
>> > > >
>> > > >   -fhardened -fstack-protector
>> > > >
>> > > > will not enable -fstack-protector-strong.
>> > > >
>> > > > In DW_AT_producer it is reflected only as -fhardened; it doesn't expand
>> > > > to anything.  I think we need a better way to show what it actually
>> > > > enables.
>> > >
>> > > I do think we need to find a solution here to solve asserting compliance.
>> >
>> > Fair enough.
>> >
>> > > Maybe we can have -Whardened that will diagnose any altering of
>> > > -fhardened by other options on the command-line or by missed target
>> > > implementations?  People might for example use -fstack-protector
>> > > but don't really want to make protection lower than requested with 
>> > > -fhardened.
>> > >
>> > > Any such conflict is much less appearant than when you use the
>> > > flags -fhardened composes.
>> >
>> > How about: --help=hardened says which options -fhardened attempts to
>> > enable, and -Whardened warns when it didn't enable an option?  E.g.,
>> >
>> >   -fstack-protector -fhardened -Whardened
>> >
>> > would say that it didn't enable -fstack-protector-strong because
>> > -fstack-protector was specified on the command line?
>> >
>> > If !HAVE_LD_NOW_SUPPORT, --help=hardened probably doesn't even have to
>> > list -z now, likewise for -z relro.
>> >
>> > Unclear if -Whardened should be enabled by default, but probably yes?
>>
>> Here's v2 which adds -Whardened (enabled by default).
>>
>> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
>
> I think it's OK but I'd like to see a second ACK here.  Can you see how our
> primary and secondary targets (+ host OS) behave here?  I think the
> documentation should elaborate a bit on expectations for non-Linux/GNU
> targets, specifically I think the default configuration for a target should
> with -fhardened _not_ have any -Whardened diagnostics.  Maybe we can
> have a testcase for this?
>
> Thanks,
> Richard.
>
>>
>> -- >8 --
>> In 
>> I proposed -fhardened, a new umbrella option that enables a reasonable set
>> of hardening flags.  The read of the room seems to be that the option
>> would be useful.  So here's a patch implementing that option.
>>
>> Currently, -fhardened enables:
>>
>>   -D_FORTIFY_SOURCE=3 (or =2 for older glibcs)
>>   -D_GLIBCXX_ASSERTIONS
>>   -ftrivial-auto-var-init=pattern
>>   -fPIE  -pie  -Wl,-z,relro,-z,now
>>   -fstack-protector-strong
>>   -fstack-clash-protection
>>   -fcf-protection=full (x86 GNU/Linux only)
>>
>> -fhardened will not override options that were specified on the command line
>> (before or after -fhardened).  For example,
>>
>>  -D_FORTIFY_SOURCE=1 -fhardened
>>
>> means that _FORTIFY_SOURCE=1 will be used.  Similarly,
>>
>>   -fhardened -fstack-protector
>>
>> will not enable -fstack-protector-strong.
>>
>> In DW_AT_producer it is reflected only as -fhardened; it doesn't expand
>> to anything.  This patch provides -Whardened, enabled by default, which
>> warns when -fhardened couldn't enable a particular option.  I think most
>> often it will say that _FORTIFY_SOURCE wasn't enabled because optimization
>> were not enabled.
>>
>> gcc/c-family/ChangeLog:
>>
>> * c-opts.cc (c_finish_options): Maybe cpp_define _FORTIFY_SOURCE
>> and _GLIBCXX_ASSERTIONS.
>>
>> gcc/ChangeLog:
>>
>>

[PATCH htdocs] bugs: Mention -D_GLIBCXX_ASSERTIONS and -D_GLIBCXX_DEBUG

2023-10-26 Thread Sam James
These options both enabled more checking within the C++ standard library
and can expose errors in submitted code.

-D_GLIBCXX_DEBUG is mentioned separately because while we want people to try it,
it's not always feasible because it requires the whole program and any used
libraries to also be built with it (as it breaks ABI).

Signed-off-by: Sam James 
---
 htdocs/bugs/index.html | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/htdocs/bugs/index.html b/htdocs/bugs/index.html
index da3d4c0d..a5a38f42 100644
--- a/htdocs/bugs/index.html
+++ b/htdocs/bugs/index.html
@@ -56,6 +56,13 @@ makes a difference, or if compiling with 
-fsanitize=undefined
 produces any run-time errors, then your code is probably not correct.
 
 
+We also ask that for C++ code, users test their programs with
+-D_GLIBCXX_ASSERTIONS. If you're able to rebuild the entire
+program (including any libraries it uses, because it breaks ABI), please do try
+-D_GLIBCXX_DEBUG which enables thorough checking throughout
+the C++ standard library. If either of these fail, this is a strong indicator
+of an error in your code.
+
 Summarized bug reporting instructions
 
 After this summary, you'll find detailed instructions that explain
-- 
2.42.0



Re: [PATCH htdocs] bugs: Mention -D_GLIBCXX_ASSERTIONS and -D_GLIBCXX_DEBUG

2023-10-26 Thread Sam James


Jonathan Wakely  writes:

> On Thursday, 26 October 2023, Sam James  wrote:
>> These options both enabled more checking within the C++ standard library
>> and can expose errors in submitted code.
>>
>> -D_GLIBCXX_DEBUG is mentioned separately because while we want people to try 
>> it,
>> it's not always feasible because it requires the whole program and any used
>> libraries to also be built with it (as it breaks ABI).
>>
>> Signed-off-by: Sam James 
>> ---
>>  htdocs/bugs/index.html | 7 +++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/htdocs/bugs/index.html b/htdocs/bugs/index.html
>> index da3d4c0d..a5a38f42 100644
>> --- a/htdocs/bugs/index.html
>> +++ b/htdocs/bugs/index.html
>> @@ -56,6 +56,13 @@ makes a difference, or if compiling with 
>> -fsanitize=undefined
>>  produces any run-time errors, then your code is probably not correct.
>>  
>>
>> +We also ask that for C++ code, users test their programs with
>> +-D_GLIBCXX_ASSERTIONS. If you're able to rebuild the entire
>> +program (including any libraries it uses, because it breaks ABI), please do 
>> try
>
> s/breaks/changes/ maybe? Breaks sounds like it's doing something bad.

Ah, yeah, a bad habit of mine I think.

>
>> +-D_GLIBCXX_DEBUG which enables thorough checking throughout
>
> It's not really throughout, just in containers and algos. Maybe "which 
> enables more thorough checking in parts of the C++
> standard library".
>

wfm

>> +the C++ standard library. If either of these fail, this is a strong 
>> indicator
>> +of an error in your code.
>> +
>>  Summarized bug reporting instructions
>>
>>  After this summary, you'll find detailed instructions that explain
>> --
>> 2.42.0
>>
>>



[PATCH htdocs v2] bugs: Mention -D_GLIBCXX_ASSERTIONS and -D_GLIBCXX_DEBUG

2023-10-26 Thread Sam James
These options both enabled more checking within the C++ standard library
and can expose errors in submitted code.

-D_GLIBCXX_DEBUG is mentioned separately because while we want people to try it,
it's not always feasible because it requires the whole program and any used
libraries to also be built with it (as it breaks ABI).

Signed-off-by: Sam James 
---
v2: Improve phrasing for the types of checks and be less scornful about ABI 
changes.

 htdocs/bugs/index.html | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/htdocs/bugs/index.html b/htdocs/bugs/index.html
index da3d4c0d..162d846a 100644
--- a/htdocs/bugs/index.html
+++ b/htdocs/bugs/index.html
@@ -56,6 +56,13 @@ makes a difference, or if compiling with 
-fsanitize=undefined
 produces any run-time errors, then your code is probably not correct.
 
 
+We also ask that for C++ code, users test their programs with
+-D_GLIBCXX_ASSERTIONS. If you're able to rebuild the entire
+program (including any libraries it uses, because it changes ABI), please do 
try
+-D_GLIBCXX_DEBUG which enables more thorough checking in parts of
+the C++ standard library. If either of these fail, this is a strong indicator
+of an error in your code.
+
 Summarized bug reporting instructions
 
 After this summary, you'll find detailed instructions that explain
-- 
2.42.0



[PATCH htdocs v3] bugs: Mention -D_GLIBCXX_ASSERTIONS and -D_GLIBCXX_DEBUG

2023-10-26 Thread Sam James
These options both enabled more checking within the C++ standard library
and can expose errors in submitted code.

-D_GLIBCXX_DEBUG is mentioned separately because while we want people to try it,
it's not always feasible because it requires the whole program and any used
libraries to also be built with it (as it breaks ABI).

Signed-off-by: Sam James 
---
v3: Link to debug mode docs.
v2: Improve phrasing for the types of checks and be less scornful about ABI 
changes.

 htdocs/bugs/index.html | 8 
 1 file changed, 8 insertions(+)

diff --git a/htdocs/bugs/index.html b/htdocs/bugs/index.html
index da3d4c0d..40355911 100644
--- a/htdocs/bugs/index.html
+++ b/htdocs/bugs/index.html
@@ -56,6 +56,14 @@ makes a difference, or if compiling with 
-fsanitize=undefined
 produces any run-time errors, then your code is probably not correct.
 
 
+We also ask that for C++ code, users test their programs with
+-D_GLIBCXX_ASSERTIONS. If you're able to rebuild the entire
+program (including any libraries it uses, because it changes ABI), please do 
try
+libstdc++'s https://gcc.gnu.org/onlinedocs/libstdc++/manual/debug_mode.html";>debug 
mode
+(-D_GLIBCXX_DEBUG) which enables more thorough checking in parts 
of
+the C++ standard library. If either of these fail, this is a strong indicator
+of an error in your code.
+
 Summarized bug reporting instructions
 
 After this summary, you'll find detailed instructions that explain
-- 
2.42.0



Re: [PING][PATCH] Include safe-ctype.h after C++ standard headers, to avoid over-poisoning

2023-10-29 Thread Sam James


Dimitry Andric  writes:

> Ping. It would be nice to get this QoL fix in.
>

Yes please - we've been using this in Gentoo since around when it was
first posted. No complaints.

I cannot approve but it looks good to me.

> -Dimitry
>
>> On 28 Sep 2023, at 18:37, Dimitry Andric  wrote:
>> 
>> Ref: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111632
>> 
>> When building gcc's C++ sources against recent libc++, the poisoning of
>> the ctype macros due to including safe-ctype.h before including C++
>> standard headers such as , , etc, causes many compilation
>> errors, similar to:
>> 
>> In file included from /home/dim/src/gcc/master/gcc/gensupport.cc:23:
>> In file included from /home/dim/src/gcc/master/gcc/system.h:233:
>> In file included from /usr/include/c++/v1/vector:321:
>> In file included from
>> /usr/include/c++/v1/__format/formatter_bool.h:20:
>> In file included from
>> /usr/include/c++/v1/__format/formatter_integral.h:32:
>> In file included from /usr/include/c++/v1/locale:202:
>> /usr/include/c++/v1/__locale:546:5: error: '__abi_tag__' attribute
>> only applies to structs, variables, functions, and namespaces
>>   546 | _LIBCPP_INLINE_VISIBILITY
>>   | ^
>> /usr/include/c++/v1/__config:813:37: note: expanded from macro
>> '_LIBCPP_INLINE_VISIBILITY'
>>   813 | #  define _LIBCPP_INLINE_VISIBILITY _LIBCPP_HIDE_FROM_ABI
>>   | ^
>> /usr/include/c++/v1/__config:792:26: note: expanded from macro
>> '_LIBCPP_HIDE_FROM_ABI'
>>   792 |
>>   __attribute__((__abi_tag__(_LIBCPP_TOSTRING(
>> _LIBCPP_VERSIONED_IDENTIFIER
>>   |  ^
>> In file included from /home/dim/src/gcc/master/gcc/gensupport.cc:23:
>> In file included from /home/dim/src/gcc/master/gcc/system.h:233:
>> In file included from /usr/include/c++/v1/vector:321:
>> In file included from
>> /usr/include/c++/v1/__format/formatter_bool.h:20:
>> In file included from
>> /usr/include/c++/v1/__format/formatter_integral.h:32:
>> In file included from /usr/include/c++/v1/locale:202:
>> /usr/include/c++/v1/__locale:547:37: error: expected ';' at end of
>> declaration list
>>   547 | char_type toupper(char_type __c) const
>>   | ^
>> /usr/include/c++/v1/__locale:553:48: error: too many arguments
>> provided to function-like macro invocation
>>   553 | const char_type* toupper(char_type* __low, const
>>   char_type* __high) const
>>   |^
>> /home/dim/src/gcc/master/gcc/../include/safe-ctype.h:146:9: note:
>> macro 'toupper' defined here
>>   146 | #define toupper(c) do_not_use_toupper_with_safe_ctype
>>   | ^
>> 
>> This is because libc++ uses different transitive includes than
>> libstdc++, and some of those transitive includes pull in various ctype
>> declarations (typically via ).
>> 
>> There was already a special case for including  before
>> safe-ctype.h, so move the rest of the C++ standard header includes to
>> the same location, to fix the problem.
>> 
>> Signed-off-by: Dimitry Andric 
>> ---
>> gcc/system.h | 39 ++-
>> 1 file changed, 18 insertions(+), 21 deletions(-)
>> 
>> diff --git a/gcc/system.h b/gcc/system.h
>> index e924152ad4c..7a516b11438 100644
>> --- a/gcc/system.h
>> +++ b/gcc/system.h
>> @@ -194,27 +194,8 @@ extern int fprintf_unlocked (FILE *, const char *, ...);
>> #undef fread_unlocked
>> #undef fwrite_unlocked
>> 
>> -/* Include  before "safe-ctype.h" to avoid GCC poisoning
>> -   the ctype macros through safe-ctype.h */
>> -
>> -#ifdef __cplusplus
>> -#ifdef INCLUDE_STRING
>> -# include 
>> -#endif
>> -#endif
>> -
>> -/* There are an extraordinary number of issues with .
>> -   The last straw is that it varies with the locale.  Use libiberty's
>> -   replacement instead.  */
>> -#include "safe-ctype.h"
>> -
>> -#include 
>> -
>> -#include 
>> -
>> -#if !defined (errno) && defined (HAVE_DECL_ERRNO) && !HAVE_DECL_ERRNO
>> -extern int errno;
>> -#endif
>> +/* Include C++ standard headers before "safe-ctype.h" to avoid GCC
>> +   poisoning the ctype macros through safe-ctype.h */
>> 
>> #ifdef __cplusplus
>> #if defined (INCLUDE_ALGORITHM) || !defined (HAVE_SWAP_IN_UTILITY)
>> @@ -229,6 +210,9 @@ extern int errno;
>> #ifdef INCLUDE_SET
>> # include 
>> #endif
>> +#ifdef INCLUDE_STRING
>> +# include 
>> +#endif
>> #ifdef INCLUDE_VECTOR
>> # include 
>> #endif
>> @@ -245,6 +229,19 @@ extern int errno;
>> # include 
>> #endif
>> 
>> +/* There are an extraordinary number of issues with .
>> +   The last straw is that it varies with the locale.  Use libiberty's
>> +   replacement instead.  */
>> +#include "safe-ctype.h"
>> +
>> +#include 
>> +
>> +#include 
>> +
>> +#if !defined (errno) && defined (HAVE_DECL_ERRNO) && !HAVE_DECL_ERRNO
>> +extern int errno;
>> +#endif
>> +
>> /* Some of glibc's string inlines cause warnings.  Plus we'd rather
>>   rely on (and therefore test) GCC's string builtins.  */
>> #defi

[PATCH 2/4] maintainer-scripts/gcc_release: create index between snapshots <-> commits

2023-11-02 Thread Sam James
Create and maintain a known_snapshots.txt index with space-separated format
BRANCH-DATE COMMIT.

For example:
8-20210107 5114ee0676e432493ada968e34071f02fb08114f
8-20210114 f9267925c648f2ccd9e4680b699e581003125bcf
...

This is helpful for bisects and quickly looking up the information from bug
reports.

maintainer-scripts/
* gcc_release: Create known_snapshots.txt as an index between snapshots
and commits.

Signed-off-by: Sam James 
---
Note that there's a few different approaches we can take here. I've gone
for the simpler one of having it still fetch from the remote site and parse
because it's obviously hard for me to test a part which runs on the remote
machine.

We can skip this patch for now if desired. I have mixed feelings about 
complicating
the contrib/generate_snapshot_index.py script to take an URL / path as I'd 
ideally
like it to still be easily usable locally.

We could have it be generated locally and then uploaded as well, as another 
option.

 maintainer-scripts/gcc_release | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/maintainer-scripts/gcc_release b/maintainer-scripts/gcc_release
index 962b8efe99a7..4cd1fa799660 100755
--- a/maintainer-scripts/gcc_release
+++ b/maintainer-scripts/gcc_release
@@ -448,6 +448,9 @@ announce_snapshot() {
   SNAPSHOT_INDEX=${RELEASE}/index.html
 
   changedir "${SNAPSHOTS_DIR}"
+  # Create an index if it doesn't already exist and populate it before we add
+  # the new snapshot.
+  ${PYTHON} ${SOURCE_DIRECTORY}/contrib/generate_snapshot_index.py || error 
"Failed to generate snapshot index"
   echo \
 "Snapshot gcc-"${RELEASE}" is now available on
   https://gcc.gnu.org/pub/gcc/snapshots/"${RELEASE}"/
@@ -514,6 +517,9 @@ Last modified "${TEXT_DATE}"
   rm -f LATEST-${BRANCH}
   ln -s ${RELEASE} LATEST-${BRANCH}
 
+  # Add the snapshot we just made to the index
+  printf "${RELEASE} ${GITREV}\n" >> known_snapshots.txt
+
   inform "Sending mail"
 
   export QMAILHOST=gcc.gnu.org
@@ -617,6 +623,7 @@ GZIP="${GZIP:-gzip --best}"
 SCP="${SCP:-scp -p}"
 SSH="${SSH:-ssh}"
 TAR="${TAR:-tar}"
+PYTHON="${PYTHON:-python3}"
 
 
 # Command Line Processing
-- 
2.42.0



[PATCH 1/4] contrib: add generate_snapshot_index.py

2023-11-02 Thread Sam James
Script to create a map between weekly snapshots and the commit they're based on
with space-separated format BRANCH-DATE COMMIT.

For example:
8-20210107 5114ee0676e432493ada968e34071f02fb08114f
8-20210114 f9267925c648f2ccd9e4680b699e581003125bcf
...

This is helpful for bisects and quickly looking up the information from bug
reports.

contrib/:
* generate_snapshot_index.py: New file.

Signed-off-by: Sam James 
---
 contrib/generate_snapshot_index.py | 79 ++
 1 file changed, 79 insertions(+)
 create mode 100755 contrib/generate_snapshot_index.py

diff --git a/contrib/generate_snapshot_index.py 
b/contrib/generate_snapshot_index.py
new file mode 100755
index ..80fc14b2cf1e
--- /dev/null
+++ b/contrib/generate_snapshot_index.py
@@ -0,0 +1,79 @@
+#!/usr/bin/env python3
+#
+# Copyright (C) 2023 Free Software Foundation, Inc.
+# Contributed by Sam James.
+#
+# This script is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3, or (at your option)
+# any later version.
+#
+# Script to create a map between weekly snapshots and the commit they're based 
on.
+# Creates known_snapshots.txt with space-separated format: BRANCH-DATE COMMIT
+# For example:
+# 8-20210107 5114ee0676e432493ada968e34071f02fb08114f
+# 8-20210114 f9267925c648f2ccd9e4680b699e581003125bcf
+
+import os
+import re
+import urllib.request
+
+MIRROR = "https://mirrorservice.org/sites/sourceware.org/pub/gcc/snapshots/";
+
+
+def get_remote_snapshot_list() -> list[str]:
+# Parse the HTML index for links to snapshots
+with urllib.request.urlopen(MIRROR) as index_response:
+html = index_response.read().decode("utf-8")
+snapshots = re.findall(r'href="([0-9]+-.*)"', html)
+
+return snapshots
+
+
+def load_cached_entries() -> dict[str, str]:
+local_snapshots = {}
+
+with open("known_snapshots.txt", encoding="utf-8") as local_entries:
+for entry in local_entries.readlines():
+if not entry:
+continue
+
+date, commit = entry.strip().split(" ")
+local_snapshots[date] = commit
+
+return local_snapshots
+
+
+remote_snapshots = get_remote_snapshot_list()
+try:
+known_snapshots = load_cached_entries()
+except FileNotFoundError:
+# No cache available
+known_snapshots = {}
+
+# This would give us chronological order (as in by creation)
+# snapshots.sort(reverse=False, key=lambda x: x.split('-')[1])
+# snapshots.sort(reverse=True, key=lambda x: x.split('-')[0])
+
+for snapshot in remote_snapshots:
+# 8-20210107/ -> 8-20210107
+snapshot = snapshot.strip("/")
+
+# Don't fetch entries we already have stored.
+if snapshot in known_snapshots:
+continue
+
+# The READMEs are plain text with several lines, one of which is:
+# "with the following options: git://gcc.gnu.org/git/gcc.git branch 
releases/gcc-8 revision e4e5ad2304db534957c4af612aa288cb6ef51f25""
+# We match after 'revision ' to grab the commit used.
+with urllib.request.urlopen(f"{MIRROR}/{snapshot}/README") as 
readme_response:
+data = readme_response.read().decode("utf-8")
+parsed_commit = re.findall(r"revision (.*)", data)[0]
+known_snapshots[snapshot] = parsed_commit
+
+# Dump it all back out to disk.
+with open("known_snapshots.txt.tmp", "w", encoding="utf-8") as known_entries:
+for name, stored_commit in known_snapshots.items():
+known_entries.write(f"{name} {stored_commit}\n")
+
+os.rename("known_snapshots.txt.tmp", "known_snapshots.txt")
-- 
2.42.0



[PATCH 3/4] maintainer-scripts/gcc_release: use HTTPS for links

2023-11-02 Thread Sam James
maintainer-scripts/
* gcc_release: Use HTTPS for links.

Signed-off-by: Sam James 
---
 maintainer-scripts/gcc_release | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/maintainer-scripts/gcc_release b/maintainer-scripts/gcc_release
index 4cd1fa799660..cf6a5731c609 100755
--- a/maintainer-scripts/gcc_release
+++ b/maintainer-scripts/gcc_release
@@ -25,7 +25,7 @@
 #
 # You should have received a copy of the GNU General Public License
 # along with GCC; see the file COPYING3.  If not see
-# <http://www.gnu.org/licenses/>.
+# <https://www.gnu.org/licenses/>.
 #
 
 
@@ -454,7 +454,7 @@ announce_snapshot() {
   echo \
 "Snapshot gcc-"${RELEASE}" is now available on
   https://gcc.gnu.org/pub/gcc/snapshots/"${RELEASE}"/
-and on various mirrors, see http://gcc.gnu.org/mirrors.html for details.
+and on various mirrors, see https://gcc.gnu.org/mirrors.html for details.
 
 This snapshot has been generated from the GCC "${BRANCH}" git branch
 with the following options: "git://gcc.gnu.org/git/gcc.git branch ${GITBRANCH} 
revision ${GITREV}"
@@ -472,7 +472,7 @@ You'll find:
 
 GCC "${RELEASE}" Snapshot
 
-The http://gcc.gnu.org/\";>GCC Project makes
+The https://gcc.gnu.org/\";>GCC Project makes
 periodic snapshots of the GCC source tree available to the public
 for testing purposes.

-- 
2.42.0



[PATCH 4/4] maintainer-scripts/gcc_release: cleanup whitespace

2023-11-02 Thread Sam James
maintainer-scripts/
* gcc_release: Cleanup whitespace.

Signed-off-by: Sam James 
---
 maintainer-scripts/gcc_release | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/maintainer-scripts/gcc_release b/maintainer-scripts/gcc_release
index cf6a5731c609..965163b65b74 100755
--- a/maintainer-scripts/gcc_release
+++ b/maintainer-scripts/gcc_release
@@ -153,7 +153,7 @@ build_sources() {
   # Update this ChangeLog file only if it does not yet contain the
   # entry we are going to add.  (This is a safety net for repeated
   # runs of this script for the same release.)
-  if ! grep "GCC ${RELEASE} released." ${SOURCE_DIRECTORY}/${x} > 
/dev/null ; then   
+  if ! grep "GCC ${RELEASE} released." ${SOURCE_DIRECTORY}/${x} > 
/dev/null ; then
cat - ${SOURCE_DIRECTORY}/${x} > ${SOURCE_DIRECTORY}/${x}.new <The https://gcc.gnu.org/\";>GCC Project makes
 periodic snapshots of the GCC source tree available to the public
 for testing purposes.
-   
+
 If you are planning to download and use one of our snapshots, then
 we highly recommend you join the GCC developers list.  Details for
 how to sign up can be found on the GCC project home page.
@@ -484,7 +484,7 @@ how to sign up can be found on the GCC project home 
page.
 with the following options: "git://gcc.gnu.org/git/gcc.git branch 
${GITBRANCH} revision ${GITREV}"
 
 " > ${SNAPSHOT_INDEX}
-   
+
   snapshot_print gcc-${RELEASE}.tar.xz "Complete GCC"
 
   echo \
@@ -554,7 +554,7 @@ FTP_PATH=/var/ftp/pub/gcc
 # The directory in which snapshots will be placed.
 SNAPSHOTS_DIR=${FTP_PATH}/snapshots
 
-# The major number for the release.  For release `3.0.2' this would be 
+# The major number for the release.  For release `3.0.2' this would be
 # `3'
 RELEASE_MAJOR=""
 # The minor number for the release.  For release `3.0.2' this would be
@@ -566,7 +566,7 @@ RELEASE_REVISION=""
 # The complete name of the release.
 RELEASE=""
 
-# The name of the branch from which the release should be made, in a 
+# The name of the branch from which the release should be made, in a
 # user-friendly form.
 BRANCH=""
 
-- 
2.42.0



[PATCH] doc: explicitly say 'lifetime' for DCE

2023-11-02 Thread Sam James
Say 'memory lifetime' rather than 'memory life' as lifetime is the more
standard term nowadays (indeed we have e.g. -fno-lifetime-dse).

It's also easier to grep for if someone is looking for the documentation on
where we do that.

gcc/ChangeLog:
* doc/passes.texi (Dead code elimination): Explicitly say 'lifetime'
as this has become the standard term for what we're doing here.

Signed-off-by: Sam James 
---
 gcc/doc/passes.texi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/doc/passes.texi b/gcc/doc/passes.texi
index eb2bb6062834..470ac498a132 100644
--- a/gcc/doc/passes.texi
+++ b/gcc/doc/passes.texi
@@ -543,7 +543,7 @@ and is defined by @code{pass_early_warn_uninitialized} and
 @item Dead code elimination
 
 This pass scans the function for statements without side effects whose
-result is unused.  It does not do memory life analysis, so any value
+result is unused.  It does not do memory lifetime analysis, so any value
 that is stored in memory is considered used.  The pass is run multiple
 times throughout the optimization process.  It is located in
 @file{tree-ssa-dce.cc} and is described by @code{pass_dce}.
-- 
2.42.0



Re: [PATCH] doc: explicitly say 'lifetime' for DCE

2023-11-02 Thread Sam James


Richard Biener  writes:

> On Thu, Nov 2, 2023 at 10:03 AM Sam James  wrote:
>>
>> Say 'memory lifetime' rather than 'memory life' as lifetime is the more
>> standard term nowadays (indeed we have e.g. -fno-lifetime-dse).
>>
>> It's also easier to grep for if someone is looking for the documentation on
>> where we do that.
>
> OK

Could you push for me please? I have a sw account but no gcc access
(yet).

cheers

>
>> gcc/ChangeLog:
>> * doc/passes.texi (Dead code elimination): Explicitly say 'lifetime'
>> as this has become the standard term for what we're doing here.
>>
>> Signed-off-by: Sam James 
>> ---
>>  gcc/doc/passes.texi | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/gcc/doc/passes.texi b/gcc/doc/passes.texi
>> index eb2bb6062834..470ac498a132 100644
>> --- a/gcc/doc/passes.texi
>> +++ b/gcc/doc/passes.texi
>> @@ -543,7 +543,7 @@ and is defined by @code{pass_early_warn_uninitialized} 
>> and
>>  @item Dead code elimination
>>
>>  This pass scans the function for statements without side effects whose
>> -result is unused.  It does not do memory life analysis, so any value
>> +result is unused.  It does not do memory lifetime analysis, so any value
>>  that is stored in memory is considered used.  The pass is run multiple
>>  times throughout the optimization process.  It is located in
>>  @file{tree-ssa-dce.cc} and is described by @code{pass_dce}.
>> --
>> 2.42.0
>>



Re: [PATCH] doc: explicitly say 'lifetime' for DCE

2023-11-02 Thread Sam James


Richard Biener  writes:

> On Thu, Nov 2, 2023 at 11:25 AM Sam James  wrote:
>>
>>
>> Richard Biener  writes:
>>
>> > On Thu, Nov 2, 2023 at 10:03 AM Sam James  wrote:
>> >>
>> >> Say 'memory lifetime' rather than 'memory life' as lifetime is the more
>> >> standard term nowadays (indeed we have e.g. -fno-lifetime-dse).
>> >>
>> >> It's also easier to grep for if someone is looking for the documentation 
>> >> on
>> >> where we do that.
>> >
>> > OK
>>
>> Could you push for me please? I have a sw account but no gcc access
>> (yet).
>
> Done after fixing ChangeLog format.
>
> Richard.

Thanks!

>
>> cheers
>>
>> >
>> >> gcc/ChangeLog:
>> >> * doc/passes.texi (Dead code elimination): Explicitly say 'lifetime'
>> >> as this has become the standard term for what we're doing here.
>> >>
>> >> Signed-off-by: Sam James 
>> >> ---
>> >>  gcc/doc/passes.texi | 2 +-
>> >>  1 file changed, 1 insertion(+), 1 deletion(-)
>> >>
>> >> diff --git a/gcc/doc/passes.texi b/gcc/doc/passes.texi
>> >> index eb2bb6062834..470ac498a132 100644
>> >> --- a/gcc/doc/passes.texi
>> >> +++ b/gcc/doc/passes.texi
>> >> @@ -543,7 +543,7 @@ and is defined by 
>> >> @code{pass_early_warn_uninitialized} and
>> >>  @item Dead code elimination
>> >>
>> >>  This pass scans the function for statements without side effects whose
>> >> -result is unused.  It does not do memory life analysis, so any value
>> >> +result is unused.  It does not do memory lifetime analysis, so any value
>> >>  that is stored in memory is considered used.  The pass is run multiple
>> >>  times throughout the optimization process.  It is located in
>> >>  @file{tree-ssa-dce.cc} and is described by @code{pass_dce}.
>> >> --
>> >> 2.42.0
>> >>
>>



Re: [PATCH] libstdc++: avoid uninitialized read in basic_string constructor

2023-11-03 Thread Sam James


Jonathan Wakely  writes:

> On Thu, 2 Nov 2023 at 19:58, Ben Sherman  
> wrote:
>>
>> Tested on x86_64-pc-linux-gnu, please let me know if there's anything
>> else needed. I haven't contributed before and don't have write access, so
>> apologies if I've missed anything.
>
> This was https://gcc.gnu.org/PR109703 (and several duplicates) and
> should already be fixed in all affected branches. Where are you seeing
> this?

It would help to know where they got their GCC from too. Ben?


Re: [PATCH][_Hashtable] Add missing destructor call

2023-11-06 Thread Sam James


François Dumont  writes:

> Noticed looking for other occasion to replace __try/__catch with RAII
> helper.
>
>     libstdc++: [_Hashtable] Add missing node destructor call
>
>     libstdc++-v3/ChangeLog:
>
>     * include/bits/hashtable_policy.h
>     (_Hashtable_alloc<>::_M_allocate_node): Add missing call
> to node destructor
>     on construct exception.
>
> Tested under Linux x64, ok to commit ?
>
> I hope gmail appli will appreciate .diff instead of .patch. .txt are
> not in .gitignore so annoying to use for patches.

Are you using 'git send-email'? It should handle all of that for you.

>
> François
>
> [2. text/x-patch; hashtable_policy.h.diff]...



Re: [PATCH] Include safe-ctype.h after C++ standard headers, to avoid over-poisoning

2024-03-06 Thread Sam James
FX Coudert  writes:

> I would like to patch this patch from September 2023:
> https://gcc.gnu.org/pipermail/gcc-patches/2023-September/631611.html
>
> This bug is now hitting macOS in the latest version of Xcode (it was 
> originally seen on freebsd).
> I confirm that the patch is restoring bootstrap on x86_64-apple-darwin23

Iain hit an issue with it and I never got a chance to look into how to
fix it.

>
> OK to push?
> FX
>
>
>
>
>
>> Ref: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111632
>> 
>> When building gcc's C++ sources against recent libc++, the poisoning of
>> the ctype macros due to including safe-ctype.h before including C++
>> standard headers such as , , etc, causes many compilation
>> errors, similar to:
>> 
>> In file included from /home/dim/src/gcc/master/gcc/gensupport.cc:23:
>> In file included from /home/dim/src/gcc/master/gcc/system.h:233:
>> In file included from /usr/include/c++/v1/vector:321:
>> In file included from
>> /usr/include/c++/v1/__format/formatter_bool.h:20:
>> In file included from
>> /usr/include/c++/v1/__format/formatter_integral.h:32:
>> In file included from /usr/include/c++/v1/locale:202:
>> /usr/include/c++/v1/__locale:546:5: error: '__abi_tag__' attribute
>> only applies to structs, variables, functions, and namespaces
>> 546 | _LIBCPP_INLINE_VISIBILITY
>> | ^
>> /usr/include/c++/v1/__config:813:37: note: expanded from macro
>> '_LIBCPP_INLINE_VISIBILITY'
>> 813 | # define _LIBCPP_INLINE_VISIBILITY _LIBCPP_HIDE_FROM_ABI
>> | ^
>> /usr/include/c++/v1/__config:792:26: note: expanded from macro
>> '_LIBCPP_HIDE_FROM_ABI'
>> 792 |
>> __attribute__((__abi_tag__(_LIBCPP_TOSTRING(
>> _LIBCPP_VERSIONED_IDENTIFIER
>> | ^
>> In file included from /home/dim/src/gcc/master/gcc/gensupport.cc:23:
>> In file included from /home/dim/src/gcc/master/gcc/system.h:233:
>> In file included from /usr/include/c++/v1/vector:321:
>> In file included from
>> /usr/include/c++/v1/__format/formatter_bool.h:20:
>> In file included from
>> /usr/include/c++/v1/__format/formatter_integral.h:32:
>> In file included from /usr/include/c++/v1/locale:202:
>> /usr/include/c++/v1/__locale:547:37: error: expected ';' at end of
>> declaration list
>> 547 | char_type toupper(char_type __c) const
>> | ^
>> /usr/include/c++/v1/__locale:553:48: error: too many arguments
>> provided to function-like macro invocation
>> 553 | const char_type* toupper(char_type* __low, const
>> char_type* __high) const
>> | ^
>> /home/dim/src/gcc/master/gcc/../include/safe-ctype.h:146:9: note:
>> macro 'toupper' defined here
>> 146 | #define toupper(c) do_not_use_toupper_with_safe_ctype
>> | ^
>> 
>> This is because libc++ uses different transitive includes than
>> libstdc++, and some of those transitive includes pull in various ctype
>> declarations (typically via ).
>> 
>> There was already a special case for including  before
>> safe-ctype.h, so move the rest of the C++ standard header includes to
>> the same location, to fix the problem.
>> 


signature.asc
Description: PGP signature


[PATCH] contrib: Improve dg-extract-results.sh's Python detection

2024-03-07 Thread Sam James
'python' on some systems (e.g. SLES 15) might be Python 2. Prefer ${EPYTHON}
if defined (used by Gentoo's python-exec wrapping), then python3, then python.

contrib/ChangeLog:

* dg-extract-results.sh: Check for python3 before python.
---
 contrib/dg-extract-results.sh | 17 ++---
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/contrib/dg-extract-results.sh b/contrib/dg-extract-results.sh
index 00ef80046f74..2d1cd76fe255 100755
--- a/contrib/dg-extract-results.sh
+++ b/contrib/dg-extract-results.sh
@@ -28,14 +28,17 @@
 
 PROGNAME=dg-extract-results.sh
 
-# Try to use the python version if possible, since it tends to be faster.
+# Try to use the python version if possible, since it tends to be faster and
+# produces more stable results.
 PYTHON_VER=`echo "$0" | sed 's/sh$/py/'`
-if test "$PYTHON_VER" != "$0" &&
-   test -f "$PYTHON_VER" &&
-   python -c 'import sys, getopt, re, io, datetime, operator; sys.exit (0 if 
sys.version_info >= (2, 6) else 1)' \
- > /dev/null 2> /dev/null; then
-  exec python $PYTHON_VER "$@"
-fi
+for python in ${EPYTHON:-python3} python ; do
+   if test "$PYTHON_VER" != "$0" &&
+  test -f "$PYTHON_VER" &&
+  ${python} -c 'import sys, getopt, re, io, datetime, operator; 
sys.exit (0 if sys.version_info >= (2, 6) else 1)' \
+> /dev/null 2> /dev/null; then
+ exec ${python} $PYTHON_VER "$@"
+   fi
+done
 
 usage() {
   cat <&2
-- 
2.44.0



Re: [PATCH] contrib: Improve dg-extract-results.sh's Python detection

2024-03-07 Thread Sam James
Jakub Jelinek  writes:

> On Thu, Mar 07, 2024 at 02:16:37PM +0000, Sam James wrote:
>> 'python' on some systems (e.g. SLES 15) might be Python 2. Prefer ${EPYTHON}
>> if defined (used by Gentoo's python-exec wrapping), then python3, then 
>> python.
>
> I'd say EPYTHON is too distro specific, just use for python in python3 python 
> ?
> Other scripts just have
> #!/usr/bin/env python3
> as the first line and go with that.

Sure. Should I add python2 too as well (last), given the script nominally tries 
to
work with it still?

>
>> contrib/ChangeLog:
>> 
>> * dg-extract-results.sh: Check for python3 before python.
>> ---
>>  contrib/dg-extract-results.sh | 17 ++---
>>  1 file changed, 10 insertions(+), 7 deletions(-)
>> 
>> diff --git a/contrib/dg-extract-results.sh b/contrib/dg-extract-results.sh
>> index 00ef80046f74..2d1cd76fe255 100755
>> --- a/contrib/dg-extract-results.sh
>> +++ b/contrib/dg-extract-results.sh
>> @@ -28,14 +28,17 @@
>>  
>>  PROGNAME=dg-extract-results.sh
>>  
>> -# Try to use the python version if possible, since it tends to be faster.
>> +# Try to use the python version if possible, since it tends to be faster and
>> +# produces more stable results.
>>  PYTHON_VER=`echo "$0" | sed 's/sh$/py/'`
>> -if test "$PYTHON_VER" != "$0" &&
>> -   test -f "$PYTHON_VER" &&
>> -   python -c 'import sys, getopt, re, io, datetime, operator; sys.exit (0 
>> if sys.version_info >= (2, 6) else 1)' \
>> - > /dev/null 2> /dev/null; then
>> -  exec python $PYTHON_VER "$@"
>> -fi
>> +for python in ${EPYTHON:-python3} python ; do
>> +if test "$PYTHON_VER" != "$0" &&
>> +   test -f "$PYTHON_VER" &&
>> +   ${python} -c 'import sys, getopt, re, io, datetime, operator; 
>> sys.exit (0 if sys.version_info >= (2, 6) else 1)' \
>> + > /dev/null 2> /dev/null; then
>> +  exec ${python} $PYTHON_VER "$@"
>> +fi
>> +done
>>  
>>  usage() {
>>cat <&2
>> -- 
>> 2.44.0
>
>   Jakub


[PATCH v2] contrib: Improve dg-extract-results.sh's Python detection

2024-03-07 Thread Sam James
'python' on some systems (e.g. SLES 15) might be Python 2. Prefer python3,
then python, then python2 (as the script still tries to work there).

contrib/ChangeLog:

* dg-extract-results.sh: Check for python3 before python. Check for python2 
last.
---
v2: Add python2 and drop EPYTHON.

 contrib/dg-extract-results.sh | 17 ++---
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/contrib/dg-extract-results.sh b/contrib/dg-extract-results.sh
index 00ef80046f74..9398de786125 100755
--- a/contrib/dg-extract-results.sh
+++ b/contrib/dg-extract-results.sh
@@ -28,14 +28,17 @@
 
 PROGNAME=dg-extract-results.sh
 
-# Try to use the python version if possible, since it tends to be faster.
+# Try to use the python version if possible, since it tends to be faster and
+# produces more stable results.
 PYTHON_VER=`echo "$0" | sed 's/sh$/py/'`
-if test "$PYTHON_VER" != "$0" &&
-   test -f "$PYTHON_VER" &&
-   python -c 'import sys, getopt, re, io, datetime, operator; sys.exit (0 if 
sys.version_info >= (2, 6) else 1)' \
- > /dev/null 2> /dev/null; then
-  exec python $PYTHON_VER "$@"
-fi
+for python in python3 python python2 ; do
+   if test "$PYTHON_VER" != "$0" &&
+  test -f "$PYTHON_VER" &&
+  ${python} -c 'import sys, getopt, re, io, datetime, operator; 
sys.exit (0 if sys.version_info >= (2, 6) else 1)' \
+> /dev/null 2> /dev/null; then
+ exec ${python} $PYTHON_VER "$@"
+   fi
+done
 
 usage() {
   cat <&2
-- 
2.44.0



Re: [PATCH] testsuite: Fix vfprintf-chk-1.c with -fhardened

2024-03-12 Thread Sam James
Sam James  writes:

> With _FORTIFY_SOURCE >= 2 (enabled by -fhardened), vfprintf-chk-1.c's
> __vfprintf_chk ends up calling __vprintf_chk rather than vprintf.
>
> ```
> --- a/fortify.s
> +++ b/no-fortify.s
> @@ -8,27 +8,28 @@
> [...]
>  __vfprintf_chk:
> [...]
> movl$1, should_optimize(%rip)
> -   jmp __vfprintf_chk
> +   jmp     vfprintf@PLT
> ```

Ping.

>
> 2024-02-15Sam James 
>
> gcc/testsuite/ChangeLog:
>   * gcc.c-torture/execute/vfprintf-chk-1.c (__vfprintf_chk): Undefine 
> _FORTIFY_SOURCE
>   to call the real vfprintf.
> ---
> The test, AIUI, is trying to test GCC's own basic _chk bits rather than
> any of e.g. glibc's _FORTIFY_SOURCE handling.
>
> I'm curious as to why only vfprintf triggers this right now. If this patch is 
> right,
> perhaps we should do printf-chk-1.c, fprintf-chk-1.c, and vprintf-chk-1.
>
> Please push if OK as I don't have access.
>
>  gcc/testsuite/gcc.c-torture/execute/vfprintf-chk-1.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/gcc/testsuite/gcc.c-torture/execute/vfprintf-chk-1.c 
> b/gcc/testsuite/gcc.c-torture/execute/vfprintf-chk-1.c
> index 401eaf4304a4..a8e5689e3fe6 100644
> --- a/gcc/testsuite/gcc.c-torture/execute/vfprintf-chk-1.c
> +++ b/gcc/testsuite/gcc.c-torture/execute/vfprintf-chk-1.c
> @@ -1,6 +1,7 @@
>  /* { dg-skip-if "requires io" { freestanding } }  */
>  
>  #ifndef test
> +#undef _FORTIFY_SOURCE
>  #include 
>  #include 
>  #include 


Re: [PATCH] Predefine __STRICT_ALIGN__ if STRICT_ALIGNMENT

2024-03-16 Thread Sam James
YunQiang Su  writes:

> Arm32 predefines __ARM_FEATURE_UNALIGNED if -mno-unaligned-access,
> and RISC-V predefines __riscv_misaligned_avoid, while other ports
> that support -mstrict-align/-mno-unaligned-access don't have such
> macro, and these backend macros are only avaiable for c-family.
> Note: Arm64 always predefine __ARM_FEATURE_UNALIGNED: See #111555.

I would say tag the bug even if you're not fixing it, as it was related
enough for you to cite it.

>
> Let's add a generic one.
>
> __STRICT_ALIGN__ is used instead of __STRICT_ALIGNMENT__, due to that
> the later is used by some softwares, such as lzo2, syslinux etc.
>
> gcc
>   * cppbuiltin.cc: Predefine __STRICT_ALIGNMENT__ if
>   STRICT_ALIGNMENT.
> ---
>  gcc/cppbuiltin.cc | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/gcc/cppbuiltin.cc b/gcc/cppbuiltin.cc
> index c4bfc2917dc..d32efdf9a07 100644
> --- a/gcc/cppbuiltin.cc
> +++ b/gcc/cppbuiltin.cc
> @@ -123,6 +123,9 @@ define_builtin_macros_for_compilation_flags (cpp_reader 
> *pfile)
>  
>cpp_define_formatted (pfile, "__FINITE_MATH_ONLY__=%d",
>   flag_finite_math_only);
> +
> +  if (STRICT_ALIGNMENT)
> +cpp_define (pfile, "__STRICT_ALIGNMENT__");
>  }


Re: [PATCH 0/2] Condition coverage fixes

2024-04-07 Thread Sam James
Jørgen Kvalsvik  writes:

> Hi,
>
> I propose these fixes for the current issues with the condition
> coverage.
>
> Rainer, I propose to simply delete the test with __sigsetjmp. I don't
> think it actually detects anything reasonable any more, I kept it around
> to prevent a regression. Since then I have built a lot of programs (with
> optimization enabled) and not really seen this problem.
>
> H.J., the problem you found with -O2 was really a problem of
> tree-inlining, which was actually caught earlier by Jan [1]. It probably
> warrants some more testing, but I could reproduce by tuning your test
> case to use always_inline and not -O2 and trigger the error.
>
> [1] https://gcc.gnu.org/pipermail/gcc-patches/2024-April/648785.html

I couldn't find your BZ account, but FWIW:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114627.

Thanks.

>
> Thanks,
> Jørgen
>
> Jørgen Kvalsvik (2):
>   Remove unecessary and broken MC/DC compile test
>   Copy condition->expr map when inlining [PR114599]
>
>  gcc/testsuite/gcc.misc-tests/gcov-19.c   | 11 -
>  gcc/testsuite/gcc.misc-tests/gcov-pr114599.c | 25 
>  gcc/tree-inline.cc   | 20 +++-
>  3 files changed, 44 insertions(+), 12 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.misc-tests/gcov-pr114599.c


Re: [PATCH v3 06/11] c: Turn -Wimplicit-function-declaration into a permerror

2024-04-09 Thread Sam James
Sebastian Huber  writes:

> On 20.11.23 10:56, Florian Weimer wrote:
>> In the future, it may make sense to avoid cascading errors from
>> the implicit declaration, especially its assumed int return type.
>> This change here only changes the kind of the diagnostic, not
>> its wording or consequences.
> Maybe this change should be added to the GCC 14 release notes.

Can you be more specific? Florian wrote about it in detail at
https://gcc.gnu.org/gcc-14/porting_to.html#c.

If you're referring specifically to the cascade-affecting-diagnostics,
that change hasn't been made yet.

What am I missing?


Re: [PATCH v3 06/11] c: Turn -Wimplicit-function-declaration into a permerror

2024-04-09 Thread Sam James
Sebastian Huber  writes:

> On 09.04.24 14:10, Sam James wrote:
>> Sebastian Huber  writes:
>> 
>>> On 20.11.23 10:56, Florian Weimer wrote:
>>>> In the future, it may make sense to avoid cascading errors from
>>>> the implicit declaration, especially its assumed int return type.
>>>> This change here only changes the kind of the diagnostic, not
>>>> its wording or consequences.
>>> Maybe this change should be added to the GCC 14 release notes.
>> Can you be more specific? Florian wrote about it in detail at
>> https://gcc.gnu.org/gcc-14/porting_to.html#c.
>> If you're referring specifically to the
>> cascade-affecting-diagnostics,
>> that change hasn't been made yet.
>> What am I missing?
>
> I searched for "implicit-function-declaration" at
>
> https://gcc.gnu.org/gcc-14/changes.html
>
> and found nothing. All right, the
>
> https://gcc.gnu.org/gcc-14/porting_to.html
>
> has a description, but this is one step away from the release
> notes. Maybe something like this could be added to the release notes:
>

I sympathise with the request. I note that we *did* mention -fno-common
at https://gcc.gnu.org/gcc-10/changes.html and just gave more detail at
https://gcc.gnu.org/gcc-10/porting_to.html. (I was pretty sure we hadn't
until I checked.)

Gerald?

> diff --git a/htdocs/gcc-14/changes.html b/htdocs/gcc-14/changes.html
> index ff69e859..90a14f37 100644
> --- a/htdocs/gcc-14/changes.html
> +++ b/htdocs/gcc-14/changes.html
> @@ -231,6 +231,17 @@ a work-in-progress.
>previous options -std=c2x, -std=gnu2x
>and -Wc11-c2x-compat, which are deprecated but remain
>supported.
> +  The following warnings are now errors (see also
> +Porting to GCC 14):
> +
> +  -Werror=declaration-missing-parameter-type
> +  -Werror=implicit-function-declaration
> +  -Werror=implicit-int
> +  -Werror=incompatible-pointer-types
> +  -Werror=int-conversion
> +  -Werror=return-mismatch
> +
> +  
>  
>
>  C++

thanks,
sam


Re: [PATCH 0/2] mmap: Avoid the sanitizer configure check failure

2024-04-09 Thread Sam James
"H.J. Lu"  writes:

> When -fsanitize=address,undefined is used to build, the mmap configure
> check failed with

I think Paul fixed this in autoconf commit
09b6e78d1592ce10fdc975025d699ee41444aa3f, so we should add a comment
about that so we can clean this up in future.

>
> =
> ==231796==ERROR: LeakSanitizer: detected memory leaks
>
> Direct leak of 4096 byte(s) in 1 object(s) allocated from:
> #0 0x7cdd3d0defdf in __interceptor_malloc 
> ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:69
> #1 0x5750c7f6d72b in main /home/alan/build/gas-san/all/bfd/conftest.c:239
>
> Direct leak of 4096 byte(s) in 1 object(s) allocated from:
> #0 0x7cdd3d0defdf in __interceptor_malloc 
> ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:69
> #1 0x5750c7f6d2e1 in main /home/alan/build/gas-san/all/bfd/conftest.c:190
>
> SUMMARY: AddressSanitizer: 8192 byte(s) leaked in 2 allocation(s).
>
> Define GCC_AC_FUNC_MMAP with export ASAN_OPTIONS=detect_leaks=0 to avoid
> the sanitizer configure check failure.
>
> H.J. Lu (2):
>   mmap: Avoid the sanitizer configure check failure
>   mmap: Avoid the sanitizer configure check failure
>
>  bfd/Makefile.in  |  2 +-
>  bfd/aclocal.m4   |  1 +
>  bfd/configure|  5 +
>  bfd/configure.ac |  2 +-
>  binutils/Makefile.in |  2 +-
>  binutils/aclocal.m4  |  1 +
>  binutils/configure   |  5 +
>  binutils/configure.ac|  2 +-
>  config/mmap.m4   | 12 
>  config/no-executables.m4 |  4 ++--
>  ld/Makefile.in   |  2 +-
>  ld/aclocal.m4|  1 +
>  ld/configure |  5 +
>  ld/configure.ac  |  2 +-
>  libctf/Makefile.in   |  2 +-
>  libctf/aclocal.m4|  1 +
>  libctf/configure |  5 +
>  libctf/configure.ac  |  2 +-
>  libiberty/Makefile.in|  1 +
>  libiberty/acinclude.m4   |  2 +-
>  libiberty/aclocal.m4 |  1 +
>  libiberty/configure  |  5 +
>  libsframe/Makefile.in|  1 +
>  libsframe/aclocal.m4 |  1 +
>  libsframe/configure  |  5 +
>  libsframe/configure.ac   |  2 +-
>  zlib/Makefile.in |  2 +-
>  zlib/acinclude.m4|  1 +
>  zlib/configure   |  7 ---
>  29 files changed, 64 insertions(+), 20 deletions(-)


signature.asc
Description: PGP signature


Re: [PATCH]middle-end: check memory accesses in the destination block [PR113588].

2024-02-03 Thread Sam James


Toon Moene  writes:

> On 2/1/24 22:33, Tamar Christina wrote:
>
>> Bootstrapped Regtested on aarch64-none-linux-gnu and x86_64-pc-linux-gnu no 
>> issues.
>> Also checked both with --enable-lto --with-build-config='bootstrap-O3 
>> bootstrap-lto' --enable-multilib
>> and --enable-lto --with-build-config=bootstrap-O3 
>> --enable-checking=release,yes,rtl,extra;
>> and checked the libcrypt testsuite as reported on PR113467.
>
> Note that I still run into problems if bootstrapping
> --with-build-config=bootstrap-O3
> (https://gcc.gnu.org/pipermail/gcc-testresults/2024-February/806840.html),
> but it is not visible.
>
> That is because it happens in the test suite of gmp, which I build
> locally as part of the build.
>
> It *is* visible in the full log of the bootstrap:

Can you file a bug please? The GMP test suite passes for me. It sounds
like it _might_ be PR113576?

> [...]

thanks,
sam


Re: [PATCH] Notes on the warnings-as-errors change in GCC 14

2024-02-07 Thread Sam James

Florian Weimer  writes:

> ---
>  htdocs/gcc-14/porting_to.html | 465 
> ++
>  1 file changed, 465 insertions(+)
>

Can't approve but LGTM. Thank you for being so thorough - it'll be
helpful when showing upstreams.

> diff --git a/htdocs/gcc-14/porting_to.html b/htdocs/gcc-14/porting_to.html
> index 3e4cedc3..4c8f9c8f 100644
> --- a/htdocs/gcc-14/porting_to.html
> +++ b/htdocs/gcc-14/porting_to.html
> @@ -24,6 +24,471 @@ porting to GCC 14. This document is an effort to identify 
> common issues
>  and provide solutions. Let us know if you have suggestions for improvements!
>  
>  
> +C language issues
> +
> +Certain warnings are now errors
> +
> + cite="https://archive.org/details/PC-Mag-1988-09-13/page/n115/mode/2up";>
> +  Function prototyping was added, first to help enforce type checking
> +  on both the types of the arguments passed to the function, and also
> +  to check the assignment compatibility of the function return type.
> +  
> +Standard C: The ANSI Draft Grows Up.
> +  PC Magazine, September 13, 1988, page 117.
> +  
> +
> +
> +The initial ISO C standard and its 1999 revision removed support for
> +many C language features that were widely known as sources of
> +application bugs due to accidental misuse.  For backwards
> +compatibility, GCC 13 and earlier compiler versions diagnosed use of
> +these features as warnings only.  Although these warnings have been
> +enabled by default for many releases, experience shows that these
> +warnings are easily ignored, resulting in difficult-to-diagnose bugs.
> +In GCC 14, these issues are now reported as errors, and no output file
> +is created, providing clearer feedback to programmers that something
> +is wrong.
> +
> +
> +Most components of the GNU project have already been fixed for
> +compatibility, but not all of them have had releases since fixes were
> +applied.  Programs that bundle parts
> +of https://www.gnu.org/software/gnulib/";>gnulib or
> +https://www.gnu.org/software/autoconf-archive/";>autoconf-archive
> +may need to update these sources files from their upstream versions.
> +
> +
> +Several GNU/Linux distributions have shared patches from their porting
> +efforts on relevant upstream mailing lists and bug trackers, but of
> +course there is a strong overlap between programs that have these
> +historic C compatibility issues and are dormant today.
> +
> +Implicit int types 
> (-Werror=implicit-int)
> +
> +In most cases, simply adding the missing int keyword
> +addresses the error.  For example, a flag like
> +
> +
> +  static initialized;
> +
> +
> +might turn into:
> +
> +
> +  static int initialized;
> +
> +
> +If the return type of a function is omitted, the correct type to
> +add could be int or void, depending on
> +whether the function returns a value or not.
> +
> +In some cases, the previously implied int type
> +was not correct.  This mostly happens in function definitions
> +that are not prototypes, when argument types are not
> +declared outside the parameter list.  Using the correct
> +type maybe required to avoid int-conversion errors (see below).

maybe -> may be

> +In the example below, the type of s should be
> +const char *, not int:
> +
> +
> +  write_string (fd, s)
> +  {
> +write (1, s, strlen (s));
> +  }
> +
> +
> +The corrected standard C source code might look like this (still
> +disregarding error handling and short writes):
> +
> +
> +  void
> +  write_string (int fd, const char *s)
> +  {
> +write (1, s, strlen (s));
> +  }
> +
> +
> +Implicit function declarations 
> (-Werror=implicit-function-declaration)
> +
> +It is no longer possible to call a function that has not been
> +declared.  In general, the solution is to include a header file with
> +an appropriate function prototype.  Note that GCC will perform further
> +type checks based on the function prototype, which can reveal further
> +type errors that require additional changes.
> +
> +
> +For well-known functions declared in standard headers, GCC provides
> +fix-it hints with the appropriate #include directives:
> +
> +
> +error: implicit declaration of function ‘strlen’ 
> [-Wimplicit-function-declaration]
> +5 |   return strlen (s);
> +  |  ^~
> +note: include ‘’ or provide a declaration of ‘strlen’
> +  +++ |+#include 
> +1 |
> +
> +
> +
> +On GNU systems, headers described in standards (such as the C
> +standard, or POSIX) may require the definition of certain
> +macros at the start of the compilation before all required
> +function declarations are made available.
> +See  href="https://sourceware.org/glibc/manual/html_node/Feature-Test-Macros.html";>Feature
>  Test Macros
> +in the GNU C Library manual for details.
> +
> +
> +Some programs are built with -std=c11 or
> +similar -std options that do not contain the
> +string gnu, but these programs still use POSIX or other
> +extensions in standard C headers such as .
> +The GNU C librar

Re: [PATCH V1] Common infrastructure for load-store fusion for aarch64 and rs6000 target

2024-02-14 Thread Sam James


Ajit Agarwal  writes:

> Hello Richard:
>
>
> On 14/02/24 4:03 pm, Richard Sandiford wrote:
>> Hi,
>> 
>> Thanks for working on this.
>> 
>> You posted a version of this patch on Sunday too.  If you need to repost
>> to fix bugs or make other improvements, could you describe the changes
>> that you've made since the previous version?  It makes things easier
>> to follow.
>
> Sure. Sorry for that I forgot to add that.
>
>> 
>> Also, sorry for starting with a meta discussion about reviews, but
>> there are multiple types of review comment, including:
>> 
>> (1) Suggestions for changes that are worded as suggestions.
>> 
>> (2) Suggestions for changes that are worded as questions ("Wouldn't it be
>> better to do X?", etc).
>> 
>> (3) Questions asking for an explanation or for more information.
>> 
>> Just sending a new patch makes sense when the previous review comments
>> were all like (1), and arguably also (1)+(2).  But Alex's previous review
>> included (3) as well.  Could you go back and respond to his questions there?
>> It would help understand some of the design choices.
>>
>
> I have responded to Alex comments for the previous patches.
> I have incorporated all of his comments in this patch.
>
>  
>> A natural starting point when reviewing a patch like this is to diff
>> the current aarch64-ldp-fusion.cc with the new pair-fusion.cc.  This shows
>> many of the kind of changes that I'd expect.  But it also seems to include
>> some code reordering, such as putting fuse_pair after try_fuse_pair.
>> If some reordering is necessary, could you try to organise the patch as
>> a series in which the reordering is a separate step?  It's a bit hard
>> to review at the moment.  (Reordering for cosmetic reasons is also OK,
>> but again please separate it out for ease of review.)
>> 
>> Maybe one way of making the review easier would be to split the aarch64
>> pass into the "target-dependent" and "target-independent" pieces
>> in-place, i.e. keeping everything within aarch64-ldp-fusion.cc, and then
>> (as separate patches) move the target-independent pieces outside
>> config/aarch64.
>> 
> Sure I will do that.
>
>> The patch includes:
>> 
>>> * emit-rtl.cc: Modify ge with gt on PolyINT data structure.
>>> * dce.cc: Add changes not to delete the load store pair.
>>> * rtl-ssa/changes.cc: Modified assert code.
>>> * var-tracking.cc: Modified assert code.
>>> * df-problems.cc: Not to generate REG_UNUSED for multi
>>> word registers that is requied for rs6000 target.
>> 
>> Please submit these separately, as independent preparatory patches,
>> with an explanation for why they're needed & correct.  But:
>> 
> Sure I will do that.
>
>>> diff --git a/gcc/df-problems.cc b/gcc/df-problems.cc
>>> index 88ee0dd67fc..a8d0ee7c4db 100644
>>> --- a/gcc/df-problems.cc
>>> +++ b/gcc/df-problems.cc
>>> @@ -3360,7 +3360,7 @@ df_set_unused_notes_for_mw (rtx_insn *insn, struct 
>>> df_mw_hardreg *mws,
>>>if (df_whole_mw_reg_unused_p (mws, live, artificial_uses))
>>>  {
>>>unsigned int regno = mws->start_regno;
>>> -  df_set_note (REG_UNUSED, insn, mws->mw_reg);
>>> +  //df_set_note (REG_UNUSED, insn, mws->mw_reg);
>>>dead_debug_insert_temp (debug, regno, insn, 
>>> DEBUG_TEMP_AFTER_WITH_REG);
>>>  
>>>if (REG_DEAD_DEBUGGING)
>>> @@ -3375,7 +3375,7 @@ df_set_unused_notes_for_mw (rtx_insn *insn, struct 
>>> df_mw_hardreg *mws,
>>> if (!bitmap_bit_p (live, r)
>>> && !bitmap_bit_p (artificial_uses, r))
>>>   {
>>> -   df_set_note (REG_UNUSED, insn, regno_reg_rtx[r]);
>>> +  // df_set_note (REG_UNUSED, insn, regno_reg_rtx[r]);

I just want to emphasise here:
a) adding out commented code is very unusual (I know a reviewer picked
up on that already);

b) if you are going to comment something out as a hack / you need help,
please *clearly flag that* (apologies if I missed it), and possibly add
a comment above it saying "// TODO: Need to figure out " or similar,
otherwise it just looks like it was forgotten about.

In this case, your question about how to handle REG_UNUSED should've
been made clear in a summary at the top where you mention the
outstanding items. Again, sorry if I missed it.

>>> dead_debug_insert_temp (debug, r, insn, DEBUG_TEMP_AFTER_WITH_REG);
>>> if (REG_DEAD_DEBUGGING)
>>>   df_print_note ("adding 2: ", insn, REG_NOTES (insn));
>>> @@ -3493,9 +3493,9 @@ df_create_unused_note (rtx_insn *insn, df_ref def,
>>> || bitmap_bit_p (artificial_uses, dregno)
>>> || df_ignore_stack_reg (dregno)))
>>>  {
>>> -  rtx reg = (DF_REF_LOC (def))
>>> -? *DF_REF_REAL_LOC (def): DF_REF_REG (def);
>>> -  df_set_note (REG_UNUSED, insn, reg);
>>> +  //rtx reg = (DF_REF_LOC (def))
>>> +  //  ? *DF_REF_REAL_LOC (def): DF_REF_REG (def);
>>> +  //df_set_note (REG_UNUSED, insn, reg);
>>>dead_debug_insert_temp (debug, dregno, insn, 
>>> DEBUG_TEMP_AFTER_WITH_REG);
>>>   

Re: [PATCH] Notes on the warnings-as-errors change in GCC 14

2024-02-15 Thread Sam James

Florian Weimer  writes:

> * Sam James:
>
>> It's fine if you leave this out, but consider mentioning the common
>> pitfall of autoconf projects not including config.h consistently before
>> all inclues. We could also mention AC_USE_SYSTEM_EXTENSIONS.
>
> I added: 
>
> “
> Alternatively, projects using using Autoconf
> could enable AC_USE_SYSTEM_EXTENSIONS.
> ”
>
>  inclusion is a larger issue, I think, best addressed by
> future diagnostics.

OK, works for me. We should discuss some options for the latter at some
point though (I think we could do it for libc cases where it matters for
LFS at least) but that's for another time.

>
>>> +
>>> +When building library code on GNU systems, it was possible to call
>>> +undefined (not just undeclared) functions and still run other code in
>>> +the library, particularly if ELF lazy binding was used.  Only
>>> +executing the undefined function call would result in a lazy binding
>>> +error and program crash.
>>
>> Maybe explicitly refer to the bfd linker's relaxed behaviour so it
>> sounds less mysterious.
>
> Like this?
>
> “
> 
> When building library code on GNU systems,
>  href="https://sourceware.org/binutils/docs-2.42/ld/Options.html#index-_002d_002dallow_002dshlib_002dundefined";>it
>  was possible to call
>   undefined (not just undeclared) functions
> and still run other code in the library, particularly if ELF lazy binding
> was used.  Only executing the undefined function call would result in a
> lazy binding error and program crash.
> ”

Sounds good, thanks.

>
> Thanks,
> Florian

best,
sam


signature.asc
Description: PGP signature


Re: [PATCH] Turn on LRA on all targets

2024-02-15 Thread Sam James

Sam James  writes:

> [[PGP Signed Part:Undecided]]
>
> "Maciej W. Rozycki"  writes:
>
>> On Sun, 23 Apr 2023, Segher Boessenkool wrote:
>>
>>> >  There are extra ICEs in regression testing and code quality is poor; cf. 
>>> > <https://gcc.gnu.org/pipermail/gcc-patches/2022-January/588296.html>.  
>>> 
>>> Do you have something you can show for this?  Maybe in a PR?
>>
>>  I have filed no PRs as I didn't assess the collateral damage at the time 
>> I looked at it.  I only ran regression-testing with `-mlra' shortly after 
>> I completed MODE_CC conversion and added the option, to see what lies 
>> beyond.  And I only added `-mlra' and made minimal changes to make the 
>> compiler build again just to make it easier to proceed towards LRA.
>
> I think before moving forward with the plan in general, a PR is ideally
> needed for each target anyway. Not all machine maintainers actively watch the
> MLs.

I have now started doing this in PR113932.

>
> [[End of PGP Signed Part]]



signature.asc
Description: PGP signature


[PATCH] testsuite: Fix vfprintf-chk-1.c with -fhardened

2024-02-15 Thread Sam James
With _FORTIFY_SOURCE >= 2 (enabled by -fhardened), vfprintf-chk-1.c's
__vfprintf_chk ends up calling __vprintf_chk rather than vprintf.

```
--- a/fortify.s
+++ b/no-fortify.s
@@ -8,27 +8,28 @@
[...]
 __vfprintf_chk:
[...]
movl$1, should_optimize(%rip)
-   jmp __vfprintf_chk
+   jmp vfprintf@PLT
```

2024-02-15  Sam James 

gcc/testsuite/ChangeLog:
* gcc.c-torture/execute/vfprintf-chk-1.c (__vfprintf_chk): Undefine 
_FORTIFY_SOURCE
to call the real vfprintf.
---
The test, AIUI, is trying to test GCC's own basic _chk bits rather than
any of e.g. glibc's _FORTIFY_SOURCE handling.

I'm curious as to why only vfprintf triggers this right now. If this patch is 
right,
perhaps we should do printf-chk-1.c, fprintf-chk-1.c, and vprintf-chk-1.

Please push if OK as I don't have access.

 gcc/testsuite/gcc.c-torture/execute/vfprintf-chk-1.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/gcc/testsuite/gcc.c-torture/execute/vfprintf-chk-1.c 
b/gcc/testsuite/gcc.c-torture/execute/vfprintf-chk-1.c
index 401eaf4304a4..a8e5689e3fe6 100644
--- a/gcc/testsuite/gcc.c-torture/execute/vfprintf-chk-1.c
+++ b/gcc/testsuite/gcc.c-torture/execute/vfprintf-chk-1.c
@@ -1,6 +1,7 @@
 /* { dg-skip-if "requires io" { freestanding } }  */
 
 #ifndef test
+#undef _FORTIFY_SOURCE
 #include 
 #include 
 #include 
-- 
2.43.1



Re: [PATCH wwwdocs] gcc-14: Some very common historic Autoconf probes that no longer work

2024-02-17 Thread Sam James

Florian Weimer  writes:

> ---
>  htdocs/gcc-14/porting_to.html | 43 
> +++
>  1 file changed, 43 insertions(+)
>
> diff --git a/htdocs/gcc-14/porting_to.html b/htdocs/gcc-14/porting_to.html
> index 123b5e9f..ab65c5e7 100644
> --- a/htdocs/gcc-14/porting_to.html
> +++ b/htdocs/gcc-14/porting_to.html
> @@ -437,6 +437,49 @@ issues addressed in more recent versions.)  Versions 
> before 2.69 may
>  have generic probes (for example for standard C support) that rely on
>  C features that were removed in C99 and thus fail with GCC 14.
>  
> +
> +Older Autoconf versions (for example, Autoconf 2.13) generate core
> +probes that are incompatible with C99.  These include the basic
> +compiler functionality check:
> +
> +
> +#include "confdefs.h"
> +main(){return(0);}
> +
> +
> +And a check for standard C compatibility:
> +
> +
> +#define XOR(e, f) (((e) && !(f)) || (!(e) && (f)))
> +int main () { int i; for (i = 0; i < 256; i++)
> +if (XOR (islower (i), ISLOWER (i)) || toupper (i) != TOUPPER (i)) exit(2);
> +exit (0); }
> +
> +

Consider adding the check name so it shows up on search engine results?

("Checking for $X ... no")

> +(Several variants with different line breaks and whitespace were in
> +use.)  If it is not possible to port the configure script to current
> +Autoconf, these issues can be patched directly:
> +
> +
> +#include "confdefs.h"
> +int main(){return(0);}
> +
> +
> +And for the second probe:
> +
> +
> +#define XOR(e, f) (((e) && !(f)) || (!(e) && (f)))
> +int main () { int i; for (i = 0; i < 256; i++)
> +if (XOR (islower (i), ISLOWER (i)) || toupper (i) != TOUPPER (i)) 
> return 2;
> +return 0; }
> +
> +
> +There is a long tail of less frequent issues, involving keyword
> +checking (for inline), and checks for dlopen
> +and mmap.  A setvbuf probe was previously
> +expected to fail at run time because it triggered undefined behavior,
> +now it fails because of a compilation error.
> +
>  Turning errors back into warnings
>  
>  

LGTM otherwise.

>
> base-commit: 1fcd61437d6a3d7bf24b993b09d525486dc9a2e5



signature.asc
Description: PGP signature


Re: [PATCH wwwdocs] gcc-14: Add code examples for -Wreturn-mismatch

2024-02-17 Thread Sam James

Florian Weimer  writes:

> ---
>  htdocs/gcc-14/porting_to.html | 46 
> ---
>  1 file changed, 43 insertions(+), 3 deletions(-)
>
> diff --git a/htdocs/gcc-14/porting_to.html b/htdocs/gcc-14/porting_to.html
> index bbbaa25a..123b5e9f 100644
> --- a/htdocs/gcc-14/porting_to.html
> +++ b/htdocs/gcc-14/porting_to.html
> @@ -213,19 +213,59 @@ in functions which are declared to return 
> void, or
>  return statements without expressions for functions
>  returning a non-void type.
>  
> +
> +Both function definitions below contain -Wreturn-mismatch
> +errors:
> +
> +
> +void
> +do_something (int flag)
> +{
> +  if (!flag)
> +return -1;
> +  do_something_else ();
> +}
> +
> +int
> +unimplemented_function (void)
> +{
> +  puts ("unimplemented function foo called");
> +}
> +
> +
> +
>  
>  To address this, remove the incorrect expression (or turn it into a
>  statement expression immediately prior to the return
>  statements if the expression has side effects), or add a dummy return
> -value, as appropriate.  If there is no suitable dummy return value,
> -further changes may be needed to implement appropriate error handling.
> +value, as appropriate.
> +
> +
> +void
> +do_something (int flag)
> +{
> +  if (!flag)
> +return -1;
> +  do_something_else ();
> +}
> +
> +int
> +unimplemented_function (void)
> +{
> +  puts ("unimplemented function foo called");
> +  return 0;
> +}
> +
> +
> +If there is no suitable dummy return value, further changes may be
> +needed to implement appropriate error handling.

LGTM.

>  
>  
>  Previously, these mismatches were diagnosed as
>  a -Wreturn-type warning.  This warning still exists, and
>  is not treated as an error by default.  It now covers remaining
>  potential correctness issues, such as reaching the closing
> -brace } of function that does not
> +brace } of a function that does not
>  return void.
>  
>  
>
> base-commit: 5ef0adf3098478600f0c108e07e568d864b4c731



signature.asc
Description: PGP signature


Re: [PATCH] Add ia64*-*-* to the list of obsolete targets

2024-02-23 Thread Sam James


Richard Biener  writes:

> The following deprecates ia64*-*-* for GCC 14.  Since we plan to
> force LRA for GCC 15 and the target only has slim chances of getting
> updated this notifies people in advance.  Given both Linux and
> glibc have axed the target further development is also made difficult.
>
> "Tested" for ia64-elf and x86_64-unknown-linux-gnu.
>
> OK?  There's no listed ia64 maintainer to CC.

Maybe tag PR90785. Anyway, no objection from us. It's not in a great
state anyway.

>
> Thanks,
> Richard.
>
> gcc/
>   * config.cc: Add ia64*-*-* to the list of obsoleted targets.
>
> contrib/
>   * config-list.mk (LIST): --enable-obsolete for ia64*-*-*.
> ---
>  contrib/config-list.mk | 5 +++--
>  gcc/config.gcc | 1 +
>  2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/contrib/config-list.mk b/contrib/config-list.mk
> index 0694cc128fa..16df66f0fc6 100644
> --- a/contrib/config-list.mk
> +++ b/contrib/config-list.mk
> @@ -60,8 +60,9 @@ LIST = \
>i686-pc-linux-gnu i686-pc-msdosdjgpp i686-lynxos i686-nto-qnx \
>i686-rtems i686-solaris2.11 i686-wrs-vxworks \
>i686-wrs-vxworksae \
> -  i686-cygwinOPT-enable-threads=yes i686-mingw32crt ia64-elf \
> -  ia64-linux ia64-hpux ia64-hp-vms iq2000-elf lm32-elf \
> +  i686-cygwinOPT-enable-threads=yes i686-mingw32crt 
> ia64-elfOPT-enable-obsolete \
> +  ia64-linuxOPT-enable-obsolete ia64-hpuxOPT-enable-obsolete \
> +  ia64-hp-vmsOPT-enable-obsolete iq2000-elf lm32-elf \
>lm32-rtems lm32-uclinux \
>loongarch64-linux-gnuf64 loongarch64-linux-gnuf32 loongarch64-linux-gnusf \
>m32c-elf m32r-elf m32rle-elf \
> diff --git a/gcc/config.gcc b/gcc/config.gcc
> index a0f9c672308..2e35a112040 100644
> --- a/gcc/config.gcc
> +++ b/gcc/config.gcc
> @@ -273,6 +273,7 @@ esac
>  # Obsolete configurations.
>  case ${target}${target_min} in
>  *-*-solaris2.11.[0-3]*   \
> +   | ia64*-*-*   \
>   )
>  if test "x$enable_obsolete" != xyes; then
>echo "*** Configuration ${target}${target_min} is obsolete." >&2



Re: [PATCH v2] Do not emulate vectors containing floats.

2024-02-23 Thread Sam James


Juergen Christ  writes:

> The emulation via word mode tries to perform integer arithmetic on floating
> point values instead of floating point arithmetic.  This leads to
> mis-compilations.

Is the bug ref + test missing?

>
> Failure occured on s390x on these existing test cases:
> gcc.dg/vect/tsvc/vect-tsvc-s112.c
> gcc.dg/vect/tsvc/vect-tsvc-s113.c
> gcc.dg/vect/tsvc/vect-tsvc-s119.c
> gcc.dg/vect/tsvc/vect-tsvc-s121.c
> gcc.dg/vect/tsvc/vect-tsvc-s131.c
> gcc.dg/vect/tsvc/vect-tsvc-s132.c
> gcc.dg/vect/tsvc/vect-tsvc-s2233.c
> gcc.dg/vect/tsvc/vect-tsvc-s421.c
> gcc.dg/vect/vect-alias-check-14.c
> gcc.target/s390/vector/partial/s390-vec-length-epil-run-1.c
> gcc.target/s390/vector/partial/s390-vec-length-epil-run-3.c
> gcc.target/s390/vector/partial/s390-vec-length-full-run-3.c
>
> gcc/ChangeLog:
>
>   * tree-vect-stmts.cc (vectorizable_operation): Don't emulate floating
>   point vectors
>
> Signed-off-by: Juergen Christ 
> ---
>  gcc/tree-vect-stmts.cc | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/gcc/tree-vect-stmts.cc b/gcc/tree-vect-stmts.cc
> index 09749ae38174..f95ff2c2aa34 100644
> --- a/gcc/tree-vect-stmts.cc
> +++ b/gcc/tree-vect-stmts.cc
> @@ -6756,7 +6756,8 @@ vectorizable_operation (vec_info *vinfo,
>those through even when the mode isn't word_mode.  For
>ops we have to lower the lowering code assumes we are
>dealing with word_mode.  */
> -  if code == PLUS_EXPR || code == MINUS_EXPR || code == NEGATE_EXPR)
> +  if (!INTEGRAL_TYPE_P (TREE_TYPE (vectype))
> +   || (((code == PLUS_EXPR || code == MINUS_EXPR || code == NEGATE_EXPR)
>   || !target_support_p)
>  && maybe_ne (GET_MODE_SIZE (vec_mode), UNITS_PER_WORD))
> /* Check only during analysis.  */



Re: [PATCH] libstdc++: hashtable: No need to update before begin node in _M_remove_bucket_begin

2024-01-16 Thread Sam James


Huanghui Nie  writes:

> Hi.

Please CC the libstdc++ LM for libstdc++ patches, per
https://gcc.gnu.org/onlinedocs/libstdc++/manual/appendix_contributing.html#list.patches.

> [...]



Re: [PING][PATCH] Include safe-ctype.h after C++ standard headers, to avoid over-poisoning

2024-01-20 Thread Sam James


Sam James  writes:

> Dimitry Andric  writes:
>
>> Ping. It would be nice to get this QoL fix in.
>>
>
> Yes please - we've been using this in Gentoo since around when it was
> first posted. No complaints.
>
> I cannot approve but it looks good to me.

Ping.

>
>> -Dimitry
>>
>>> On 28 Sep 2023, at 18:37, Dimitry Andric  wrote:
>>> 
>>> Ref: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111632
>>> 
>>> When building gcc's C++ sources against recent libc++, the poisoning of
>>> the ctype macros due to including safe-ctype.h before including C++
>>> standard headers such as , , etc, causes many compilation
>>> errors, similar to:
>>> 
>>> In file included from /home/dim/src/gcc/master/gcc/gensupport.cc:23:
>>> In file included from /home/dim/src/gcc/master/gcc/system.h:233:
>>> In file included from /usr/include/c++/v1/vector:321:
>>> In file included from
>>> /usr/include/c++/v1/__format/formatter_bool.h:20:
>>> In file included from
>>> /usr/include/c++/v1/__format/formatter_integral.h:32:
>>> In file included from /usr/include/c++/v1/locale:202:
>>> /usr/include/c++/v1/__locale:546:5: error: '__abi_tag__' attribute
>>> only applies to structs, variables, functions, and namespaces
>>>   546 | _LIBCPP_INLINE_VISIBILITY
>>>   | ^
>>> /usr/include/c++/v1/__config:813:37: note: expanded from macro
>>> '_LIBCPP_INLINE_VISIBILITY'
>>>   813 | #  define _LIBCPP_INLINE_VISIBILITY _LIBCPP_HIDE_FROM_ABI
>>>   | ^
>>> /usr/include/c++/v1/__config:792:26: note: expanded from macro
>>> '_LIBCPP_HIDE_FROM_ABI'
>>>   792 |
>>>   __attribute__((__abi_tag__(_LIBCPP_TOSTRING(
>>> _LIBCPP_VERSIONED_IDENTIFIER
>>>   |  ^
>>> In file included from /home/dim/src/gcc/master/gcc/gensupport.cc:23:
>>> In file included from /home/dim/src/gcc/master/gcc/system.h:233:
>>> In file included from /usr/include/c++/v1/vector:321:
>>> In file included from
>>> /usr/include/c++/v1/__format/formatter_bool.h:20:
>>> In file included from
>>> /usr/include/c++/v1/__format/formatter_integral.h:32:
>>> In file included from /usr/include/c++/v1/locale:202:
>>> /usr/include/c++/v1/__locale:547:37: error: expected ';' at end of
>>> declaration list
>>>   547 | char_type toupper(char_type __c) const
>>>   | ^
>>> /usr/include/c++/v1/__locale:553:48: error: too many arguments
>>> provided to function-like macro invocation
>>>   553 | const char_type* toupper(char_type* __low, const
>>>   char_type* __high) const
>>>   |^
>>> /home/dim/src/gcc/master/gcc/../include/safe-ctype.h:146:9: note:
>>> macro 'toupper' defined here
>>>   146 | #define toupper(c) do_not_use_toupper_with_safe_ctype
>>>   | ^
>>> 
>>> This is because libc++ uses different transitive includes than
>>> libstdc++, and some of those transitive includes pull in various ctype
>>> declarations (typically via ).
>>> 
>>> There was already a special case for including  before
>>> safe-ctype.h, so move the rest of the C++ standard header includes to
>>> the same location, to fix the problem.
>>> 
>>> Signed-off-by: Dimitry Andric 
>>> ---
>>> gcc/system.h | 39 ++-
>>> 1 file changed, 18 insertions(+), 21 deletions(-)
>>> 
>>> diff --git a/gcc/system.h b/gcc/system.h
>>> index e924152ad4c..7a516b11438 100644
>>> --- a/gcc/system.h
>>> +++ b/gcc/system.h
>>> @@ -194,27 +194,8 @@ extern int fprintf_unlocked (FILE *, const char *, 
>>> ...);
>>> #undef fread_unlocked
>>> #undef fwrite_unlocked
>>> 
>>> -/* Include  before "safe-ctype.h" to avoid GCC poisoning
>>> -   the ctype macros through safe-ctype.h */
>>> -
>>> -#ifdef __cplusplus
>>> -#ifdef INCLUDE_STRING
>>> -# include 
>>> -#endif
>>> -#endif
>>> -
>>> -/* There are an extraordinary number of issues with .
>>> -   The last straw is that it varies with the locale.  Use libiberty's
>>> -   replacement instead.  */
>>> -#include "safe-ctype.h"
>>> -
>>> -#include 
>>> -

Re: [PATCH v1 2/4] C++: Support clang compatible [[musttail]]

2024-01-24 Thread Sam James


Andi Kleen  writes:

> This patch implements a clang compatible [[musttail]] attribute for
> returns.

This is PR83324. See also PR52067 and PR110899.

>
> musttail is useful as an alternative to computed goto for interpreters.
> With computed goto the interpreter function usually ends up very big
> which causes problems with register allocation and other per function
> optimizations not scaling. With musttail the interpreter can be instead
> written as a sequence of smaller functions that call each other. To
> avoid unbounded stack growth this requires forcing a sibling call, which
> this attribute does. It guarantees an error if the call cannot be tail
> called which allows the programmer to fix it instead of risking a stack
> overflow. Unlike computed goto it is also type-safe.
>

Yeah, CPython is going to require this for its new JIT.

> The attribute is only supported for C++, since the C-parser
> has no support for statement attributes for non empty statements.
> It could be added there with __attribute__ too but would need
> some minor grammar adjustments.

... although it'll need C there.



Re: [PING][PATCH] Include safe-ctype.h after C++ standard headers, to avoid over-poisoning

2024-01-30 Thread Sam James

Sam James  writes:

> Sam James  writes:
>
>> Dimitry Andric  writes:
>>
>>> Ping. It would be nice to get this QoL fix in.
>>>
>>
>> Yes please - we've been using this in Gentoo since around when it was
>> first posted. No complaints.
>>
>> I cannot approve but it looks good to me.
>
> Ping.

Ping. This is straightforward, I just can't merge it myself.

>
>>
>>> -Dimitry
>>>
>>>> On 28 Sep 2023, at 18:37, Dimitry Andric  wrote:
>>>> 
>>>> Ref: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111632
>>>> 
>>>> When building gcc's C++ sources against recent libc++, the poisoning of
>>>> the ctype macros due to including safe-ctype.h before including C++
>>>> standard headers such as , , etc, causes many compilation
>>>> errors, similar to:
>>>> 
>>>> In file included from /home/dim/src/gcc/master/gcc/gensupport.cc:23:
>>>> In file included from /home/dim/src/gcc/master/gcc/system.h:233:
>>>> In file included from /usr/include/c++/v1/vector:321:
>>>> In file included from
>>>> /usr/include/c++/v1/__format/formatter_bool.h:20:
>>>> In file included from
>>>> /usr/include/c++/v1/__format/formatter_integral.h:32:
>>>> In file included from /usr/include/c++/v1/locale:202:
>>>> /usr/include/c++/v1/__locale:546:5: error: '__abi_tag__' attribute
>>>> only applies to structs, variables, functions, and namespaces
>>>>   546 | _LIBCPP_INLINE_VISIBILITY
>>>>   | ^
>>>> /usr/include/c++/v1/__config:813:37: note: expanded from macro
>>>> '_LIBCPP_INLINE_VISIBILITY'
>>>>   813 | #  define _LIBCPP_INLINE_VISIBILITY _LIBCPP_HIDE_FROM_ABI
>>>>   | ^
>>>> /usr/include/c++/v1/__config:792:26: note: expanded from macro
>>>> '_LIBCPP_HIDE_FROM_ABI'
>>>>   792 |
>>>>   __attribute__((__abi_tag__(_LIBCPP_TOSTRING(
>>>> _LIBCPP_VERSIONED_IDENTIFIER
>>>>   |  ^
>>>> In file included from /home/dim/src/gcc/master/gcc/gensupport.cc:23:
>>>> In file included from /home/dim/src/gcc/master/gcc/system.h:233:
>>>> In file included from /usr/include/c++/v1/vector:321:
>>>> In file included from
>>>> /usr/include/c++/v1/__format/formatter_bool.h:20:
>>>> In file included from
>>>> /usr/include/c++/v1/__format/formatter_integral.h:32:
>>>> In file included from /usr/include/c++/v1/locale:202:
>>>> /usr/include/c++/v1/__locale:547:37: error: expected ';' at end of
>>>> declaration list
>>>>   547 | char_type toupper(char_type __c) const
>>>>   | ^
>>>> /usr/include/c++/v1/__locale:553:48: error: too many arguments
>>>> provided to function-like macro invocation
>>>>   553 | const char_type* toupper(char_type* __low, const
>>>>   char_type* __high) const
>>>>   |^
>>>> /home/dim/src/gcc/master/gcc/../include/safe-ctype.h:146:9: note:
>>>> macro 'toupper' defined here
>>>>   146 | #define toupper(c) do_not_use_toupper_with_safe_ctype
>>>>   | ^
>>>> 
>>>> This is because libc++ uses different transitive includes than
>>>> libstdc++, and some of those transitive includes pull in various ctype
>>>> declarations (typically via ).
>>>> 
>>>> There was already a special case for including  before
>>>> safe-ctype.h, so move the rest of the C++ standard header includes to
>>>> the same location, to fix the problem.
>>>> 
>>>> Signed-off-by: Dimitry Andric 
>>>> ---
>>>> gcc/system.h | 39 ++-
>>>> 1 file changed, 18 insertions(+), 21 deletions(-)
>>>> 
>>>> diff --git a/gcc/system.h b/gcc/system.h
>>>> index e924152ad4c..7a516b11438 100644
>>>> --- a/gcc/system.h
>>>> +++ b/gcc/system.h
>>>> @@ -194,27 +194,8 @@ extern int fprintf_unlocked (FILE *, const char *, 
>>>> ...);
>>>> #undef fread_unlocked
>>>> #undef fwrite_unlocked
>>>> 
>>>> -/* Include  before "safe-ctype.h" to avoid GCC poisoning
>>>> -   the ctype macros through safe-ctype.h */
>&

Re: [PING][PATCH] Include safe-ctype.h after C++ standard headers, to avoid over-poisoning

2024-01-30 Thread Sam James


Sam James  writes:

> [[PGP Signed Part:Undecided]]
>
> Sam James  writes:
>
>> Sam James  writes:
>>
>>> Dimitry Andric  writes:
>>>
>>>> Ping. It would be nice to get this QoL fix in.
>>>>
>>>
>>> Yes please - we've been using this in Gentoo since around when it was
>>> first posted. No complaints.
>>>
>>> I cannot approve but it looks good to me.
>>
>> Ping.
>
> Ping. This is straightforward, I just can't merge it myself.
>
>>

We discussed it on IRC and Iain is going to test it on Darwin, I'm going
to test it on the cfarm on platforms the patch was not already exposed
to, after Jeff raised some concerns.

I also have an additional hunk or two I'm testing for JIT.

>>>
>>>> -Dimitry
>>>>
>>>>> On 28 Sep 2023, at 18:37, Dimitry Andric  wrote:
>>>>> 
>>>>> Ref: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111632
>>>>> 
>>>>> When building gcc's C++ sources against recent libc++, the poisoning of
>>>>> the ctype macros due to including safe-ctype.h before including C++
>>>>> standard headers such as , , etc, causes many compilation
>>>>> errors, similar to:
>>>>> 
>>>>> In file included from /home/dim/src/gcc/master/gcc/gensupport.cc:23:
>>>>> In file included from /home/dim/src/gcc/master/gcc/system.h:233:
>>>>> In file included from /usr/include/c++/v1/vector:321:
>>>>> In file included from
>>>>> /usr/include/c++/v1/__format/formatter_bool.h:20:
>>>>> In file included from
>>>>> /usr/include/c++/v1/__format/formatter_integral.h:32:
>>>>> In file included from /usr/include/c++/v1/locale:202:
>>>>> /usr/include/c++/v1/__locale:546:5: error: '__abi_tag__' attribute
>>>>> only applies to structs, variables, functions, and namespaces
>>>>>   546 | _LIBCPP_INLINE_VISIBILITY
>>>>>   | ^
>>>>> /usr/include/c++/v1/__config:813:37: note: expanded from macro
>>>>> '_LIBCPP_INLINE_VISIBILITY'
>>>>>   813 | #  define _LIBCPP_INLINE_VISIBILITY _LIBCPP_HIDE_FROM_ABI
>>>>>   | ^
>>>>> /usr/include/c++/v1/__config:792:26: note: expanded from macro
>>>>> '_LIBCPP_HIDE_FROM_ABI'
>>>>>   792 |
>>>>>   __attribute__((__abi_tag__(_LIBCPP_TOSTRING(
>>>>> _LIBCPP_VERSIONED_IDENTIFIER
>>>>>   |  ^
>>>>> In file included from /home/dim/src/gcc/master/gcc/gensupport.cc:23:
>>>>> In file included from /home/dim/src/gcc/master/gcc/system.h:233:
>>>>> In file included from /usr/include/c++/v1/vector:321:
>>>>> In file included from
>>>>> /usr/include/c++/v1/__format/formatter_bool.h:20:
>>>>> In file included from
>>>>> /usr/include/c++/v1/__format/formatter_integral.h:32:
>>>>> In file included from /usr/include/c++/v1/locale:202:
>>>>> /usr/include/c++/v1/__locale:547:37: error: expected ';' at end of
>>>>> declaration list
>>>>>   547 | char_type toupper(char_type __c) const
>>>>>   | ^
>>>>> /usr/include/c++/v1/__locale:553:48: error: too many arguments
>>>>> provided to function-like macro invocation
>>>>>   553 | const char_type* toupper(char_type* __low, const
>>>>>   char_type* __high) const
>>>>>   |^
>>>>> /home/dim/src/gcc/master/gcc/../include/safe-ctype.h:146:9: note:
>>>>> macro 'toupper' defined here
>>>>>   146 | #define toupper(c) do_not_use_toupper_with_safe_ctype
>>>>>   | ^
>>>>> 
>>>>> This is because libc++ uses different transitive includes than
>>>>> libstdc++, and some of those transitive includes pull in various ctype
>>>>> declarations (typically via ).
>>>>> 
>>>>> There was already a special case for including  before
>>>>> safe-ctype.h, so move the rest of the C++ standard header includes to
>>>>> the same location, to fix the problem.
>>>>> 
>>>>> Signed-off-by: Dimitry Andric 
>>>>> ---
>>>>> gcc/system.h | 39 ++

Re: About 31109 - gprofng not built and installed in a combined binutils+gcc build

2024-01-31 Thread Sam James


Richard Biener  writes:

> On Wed, Jan 31, 2024 at 4:46 AM Vladimir Mezentsev
>  wrote:
>>
>> Hi,
>>
>> I asked in https://sourceware.org/bugzilla/show_bug.cgi?id=31109
>>  > I prepared a patch for the releases/gcc-13 branch.
>>  > Richard Biener  rejected my patch for
>> this branch.
>>  > Which branch should I use? master, trunk or something else?
>
> toplevel changes are synced between binutils/gcc master branches only
>
>> Do you really need gprofng in the gcc repo ?
>> if yes:
>>the fix is trivial.
>>I did for the releases/gcc-13 branch:
>>   git cherry-pick 24552056fd5fc677c0d032f54a5cad1c4303d312
>>Can anyone do the same for the correct branch.
>>I have no write permissions for gcc.gnu.org/git/gcc.git
>>
>>I maintain binutils-gdb/gprofng. Who will maintain gcc/gprofng ?
>
> It's maintained in the binutils-gdb repository.  Shared files are synced
> as said above.
>
> I've never seen us care for release branches in the GCC repository,
> combined builds are not really "supported" (or even tested regularly).
>
>> If no:
>>   may I close 31109 ?
>
> So yes, I'd say that's an INVALID bug since it doesn't use master
> branches on both sides.
>

wfm - we don't use them at all and I've also never expected syncs
to happen for non-master.

> Richard.
>
>> Thank you,
>> -Vladimir
>>
>> .
>>

best,
sam



Re: [PATCH 2/6] c: Turn int-conversion warnings into permerrors

2023-12-01 Thread Sam James


钟居哲  writes:

> Hi, This patch cause error on building newlib/glibc/musl on RISC-V port:
>
> /work/home/jzzhong/work/toolchain/riscv/build/dev-rv64gcv_zvfh_zfh-lp64d-medany-newlib-spike-debug/../../newlib/libgloss/riscv/sys_access.c:8:40:
> error: passing argument 3 of 'syscall_errno' makes integer from pointer 
> without a cast [-Wint-conversion]
> 8 |   return syscall_errno (SYS_access, 2, file, mode, 0, 0, 0, 0);
>   |^~~~
>   ||
>   |const char *

This looks like an issue in newlib. We expect broken code to be broken
by the recent changes. Can you investigate it on the newlib side?

Thanks.



[11 PATCH] libiberty, Darwin: Fix a build warning. [PR112823]

2023-12-01 Thread Sam James
From: Iain Sandoe 

r12-3005-g220c410162ebece4f missed a cast for the set_32 call.
Fixed thus.

Signed-off-by: Iain Sandoe 
Signed-off-by: Sam James 

libiberty/ChangeLog:
PR other/112823
* simple-object-mach-o.c (simple_object_mach_o_write_segment):
Cast the first argument to set_32 as needed.

(cherry picked from commit 38757aa88735ab2e511bc428e2407a5a5e9fa0be)
---
 libiberty/simple-object-mach-o.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libiberty/simple-object-mach-o.c b/libiberty/simple-object-mach-o.c
index 72b69d19c216..a8869e7c6395 100644
--- a/libiberty/simple-object-mach-o.c
+++ b/libiberty/simple-object-mach-o.c
@@ -1228,7 +1228,7 @@ simple_object_mach_o_write_segment (simple_object_write 
*sobj, int descriptor,
   /* Swap the indices, if required.  */
 
   for (i = 0; i < (nsects_in * 4); ++i)
-   set_32 (&index[i], index[i]);
+   set_32 ((unsigned char *) &index[i], index[i]);
 
   sechdr_offset += sechdrsize;
 
-- 
2.43.0



Re: [PATCH 2/6] c: Turn int-conversion warnings into permerrors

2023-12-01 Thread Sam James


Jeff Law  writes:

> On 12/1/23 18:13, Sam James wrote:
>> 钟居哲  writes:
>> 
>>> Hi, This patch cause error on building newlib/glibc/musl on RISC-V port:
>>>
>>> /work/home/jzzhong/work/toolchain/riscv/build/dev-rv64gcv_zvfh_zfh-lp64d-medany-newlib-spike-debug/../../newlib/libgloss/riscv/sys_access.c:8:40:
>>> error: passing argument 3 of 'syscall_errno' makes integer from pointer 
>>> without a cast [-Wint-conversion]
>>>  8 |   return syscall_errno (SYS_access, 2, file, mode, 0, 0, 0, 0);
>>>|^~~~
>>>||
>>>|const char *
>> This looks like an issue in newlib. We expect broken code to be
>> broken
>> by the recent changes. Can you investigate it on the newlib side?
> A ton of stuff in newlib/libgloss is broken due to the compiler
> changes.   But that's not a big surprise -- much of the
> newlib/libgloss code is c89 and clearly wrong for c99 and newer.

Yeah, it's probably a reasonable candidate for -fpermissive to start
with until it's cleaned up.

(Also, sorry, I didn't mean my comment to appear glib. I just meant to
say "yes, this looks expected".)

>
> Jeff

thanks,
sam


Re: [11 PATCH] libiberty, Darwin: Fix a build warning. [PR112823]

2023-12-01 Thread Sam James


Iain Sandoe  writes:

> HI Sam,

Hi Iain,

>
> I think this qualifies as obvious (it’s on my list, but I did not get to it 
> yet,
> so go ahead).

Thanks. I can't push it myself - could you do that for me?

thanks again,
sam

>
> Iain
>
>> On 2 Dec 2023, at 05:30, Sam James  wrote:
>> 
>> From: Iain Sandoe 
>> 
>> r12-3005-g220c410162ebece4f missed a cast for the set_32 call.
>> Fixed thus.
>> 
>> Signed-off-by: Iain Sandoe 
>> Signed-off-by: Sam James 
>> 
>> libiberty/ChangeLog:
>>  PR other/112823
>>  * simple-object-mach-o.c (simple_object_mach_o_write_segment):
>>  Cast the first argument to set_32 as needed.
>> 
>> (cherry picked from commit 38757aa88735ab2e511bc428e2407a5a5e9fa0be)
>> ---
>> libiberty/simple-object-mach-o.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/libiberty/simple-object-mach-o.c 
>> b/libiberty/simple-object-mach-o.c
>> index 72b69d19c216..a8869e7c6395 100644
>> --- a/libiberty/simple-object-mach-o.c
>> +++ b/libiberty/simple-object-mach-o.c
>> @@ -1228,7 +1228,7 @@ simple_object_mach_o_write_segment 
>> (simple_object_write *sobj, int descriptor,
>>   /* Swap the indices, if required.  */
>> 
>>   for (i = 0; i < (nsects_in * 4); ++i)
>> -set_32 (&index[i], index[i]);
>> +set_32 ((unsigned char *) &index[i], index[i]);
>> 
>>   sechdr_offset += sechdrsize;
>> 
>> -- 
>> 2.43.0
>> 



Re: [PATCH] testsuite: Adjust for the new permerror -Wincompatible-pointer-types

2023-12-06 Thread Sam James


Yang Yujie  writes:

> On Wed, Dec 06, 2023 at 10:45:22AM -0700, Jeff Law wrote:
>> 
>> 
>> On 12/6/23 05:12, Florian Weimer wrote:
>> > * Yang Yujie:
>> > 
>> > > From: Yang Yujie 
>> > > Subject: [PATCH] testsuite: Adjust for the new permerror
>> > >   -Wincompatible-pointer-types
>> > > To: gcc-patches@gcc.gnu.org
>> > > Cc: r...@cebitec.uni-bielefeld.de, mikest...@comcast.net, 
>> > > fwei...@redhat.com,
>> > >   Yang Yujie 
>> > > Date: Wed,  6 Dec 2023 10:29:31 +0800 (9 hours, 42 minutes, 7 seconds 
>> > > ago)
>> > > Message-ID: <20231206022931.33437-1-yangyu...@loongson.cn>
>> > > 
>> > > r14-6037 turned -Wincompatible-pointer-types into a permerror,
>> > > which causes the following tests to fail.
>> > > 
>> > > gcc/testsuite/ChangeLog:
>> > > 
>> > >  * gcc.dg/fixed-point/composite-type.c: replace dg-warning with dg-error.
>> > > ---
>> > >   .../gcc.dg/fixed-point/composite-type.c   | 64 +--
>> > >   1 file changed, 32 insertions(+), 32 deletions(-)
>> > 
>> > Looks reasonable to me, but I can't approve it.
>> We might want to fix that from a policy standpoint :-)
>> 
>> Regardless, this is OK for the trunk.  Thanks Yang for taking care of it.  I
>> don't see you in the maintainers file, so I'll go ahead and push it
>> momentarily.
>> 
>> jeff
>
> Thanks for the review!
>
> With this patch, I also noticed a few errors in building unpatched older
> software like expect-5.45.4, perl-5.28.3 and bash-5.0.  Will this also be
> the case when GCC 14 gets released?
>

Old versions of software will need to be patched or built with
-fpermissive. We are working on fixing supported versions of software
and sending patches upstream - please do join in if you're able, as
the more help the better.

It is normal for software to need porting to newer compilers. For
example, https://gcc.gnu.org/gcc-10/porting_to.html (-fcommon).

> Thanks,
> Yujie

thanks,
sam


Re: Introduce -finline-stringops (was: Re: [RFC] Introduce -finline-memset-loops)

2023-12-11 Thread Sam James


Alexandre Oliva via Gcc-patches  writes:

> On Jun  2, 2023, Alexandre Oliva  wrote:
>
>> Introduce -finline-stringops
>
> Ping?  https://gcc.gnu.org/pipermail/gcc-patches/2023-June/620472.html

Should the docs for the x86-specific -minline-all-stringops refer to
the new -finline-stringops?

thanks,
sam


Re: [PATCH 4/4] maintainer-scripts/gcc_release: cleanup whitespace

2023-11-10 Thread Sam James


Joseph Myers  writes:

> On Thu, 2 Nov 2023, Sam James wrote:
>
>> maintainer-scripts/
>>  * gcc_release: Cleanup whitespace.
>
> OK.

Thanks. Would you mind pushing the two you approved?


[PATCH] MAINTAINERS: Fix formatting

2023-11-10 Thread Sam James
ChangeLog:
* MAINTAINERS (Write After Approval): Fix indentation and missing email 
bracket.

Signed-off-by: Sam James 
---
 MAINTAINERS | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index c43167d9a752..9ad68687f769 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -325,7 +325,7 @@ Mark G. Adams   

 Ajit Kumar Agarwal 
 Pedro Alves
 Paul-Antoine Arras 
-Arsen Arsenović
+Arsen Arsenović
 Raksit Ashok   
 Matt Austern   
 David Ayers
@@ -411,7 +411,7 @@ Chris Fairles   

 Alessandro Fanfarillo  
 Changpeng Fang 
 Sam Feifer 
-Eric Feng  
+Eric Feng  
 Li Feng
 Thomas Fitzsimmons 
 Alexander Fomin

@@ -534,16 +534,16 @@ Renlin Li 

 Xinliang David Li  
 Chen Liqin 
 Martin Liska   
-Hao Liu
+Hao Liu

 Jiangning Liu  
 Sa Liu 
 Ralph Loader   
-Sheldon Lobo   
 Gabor Loki 
 Manuel López-Ibáñez
 Carl Love  
 Martin v. Löwis

-Edwin Lu   
+Edwin Lu   
 H.J. Lu
 Xiong Hu Luo   
 Bin Bin Lv 
@@ -622,7 +622,7 @@ Hafiz Abid Qadeer   

 Yao Qi 
 Jerry Quinn
 Navid Rahimi   
-Rishi Raj  

+Rishi Raj  
 Easwaran Raman 
 Joe Ramsay 
 Rolf Rasmussen 
@@ -762,7 +762,7 @@ Immad Mir   

 Gaius Mulley   
 Siddhesh Poyarekar 
 Navid Rahimi   
-Rishi Raj  

+Rishi Raj  
 Trevor Saunders

 Bill Schmidt   
 Nathan Sidwell 
-- 
2.42.1



Re: [PATCH] C99 testsuite readiness: -fpermissive tests

2023-11-11 Thread Sam James


Florian Weimer  writes:

> * Eric Gallager:
>
>>> diff --git a/gcc/testsuite/gcc.c-torture/compile/20080910-1.c 
>>> b/gcc/testsuite/gcc.c-torture/compile/20080910-1.c
>>> index bf32775d401..911fb562790 100644
>>> --- a/gcc/testsuite/gcc.c-torture/compile/20080910-1.c
>>> +++ b/gcc/testsuite/gcc.c-torture/compile/20080910-1.c
>>> @@ -1,5 +1,6 @@
>>>  /* This used to crash IRA with -O3 -fPIC.
>>>See PR 37333.  */
>>> +/* { dg-additional-options "-fpermissive" } */
>>>  struct yy_buffer_state  {
>>>int yy_is_interactive;
>>>  };
>>
>> The fact that this one appears to be based on a scanner generated by
>> flex gives me pause. Is code from flex/yacc/bison going to need
>> -fpermissive in general, or is this just because it was generated by
>> an old version? If it's only old versions that generate code requiring
>> -fpermissive, when exactly did they stop doing that?
>
> It can be a bit tricky to get bison, flex, and automake to work
> together.  The inter-dependencies can be challenging, and apparently
> it's difficult to teach automake that generating a lexer or parser
> produces both the implementation C file and a header file.  There's a
> fairly generic way to deal with this: write a custom header that
> contains the necessary function declarations and include it everywhere.
> It's not as elegant as using the generated headers, but that can be
> difficult depending on the build system.

Right. The issues with anything Bison (and friends) were usually missing
declarations because of the header dependencies. Nothing intrinsic.

In fact, I can't even remember anything where Bison et. al generated
bad code purely by virtue of the version (even though sometimes comments
would suggest that, I never found any evidence for it, and it was
always the fault of the project itself which we'd then fix).

So, not worried.

>
> However, the problems with this test case aren't related to that.  For
> example, yy_fatal_error is undefined, but is a static function in the
> code generated by flex.  The test case has been preprocessed (that's how
> _IO_getc got there), and the declarations from system headers have been
> removed.
>
> Thanks,
> Florian



Re: [RFC PATCH] Detecting lifetime-dse issues via Valgrind [PR66487]

2023-11-11 Thread Sam James


exactl...@ispras.ru writes:

> From: Daniil Frolov 
>
> PR 66487 is asking to provide sanitizer-like detection for C++ object lifetime
> violations that are worked around with -fno-lifetime-dse in Firefox, LLVM,
> OpenJade.
>
> The discussion in the PR was centered around extending MSan, but MSan was not
> ported to GCC (and requires rebuilding everything with instrumentation).
>
> Instead, allow Valgrind to see lifetime boundaries by emitting client requests
> along *this = { CLOBBER }.  The client request marks the "clobbered" memory as
> undefined for Valgrind; clobbering assignments mark the beginning of ctor and
> end of dtor execution for C++ objects.  Hence, attempts to read object storage
> after the destructor, or "pre-initialize" its fields prior to the constructor
> will be caught.
>
> Valgrind client requests are offered as macros that emit inline asm.  For use
> in code generation, we need to wrap it in a built-in.  We know that 
> implementing
> such a built-in in libgcc is undesirable, [...].

Perhaps less objectionable than you think, at least given the new CFR
stuff from oliva from the other week that landed.

> ideally contents of libgcc should not
> depend on availability of external headers.  Suggestion for cleaner solutions
> would be welcome.

This is a really neat idea (it also makes me wonder if there's any other
opportunities for Valgrind integration like this?). It provides a good
alternative to some of the problems from sanitizers too. Sanitizers are
great but they're really not perfect, at least as they stand.

We've "wasted" a lot of time debugging lifetime issues when building
with GCC, especially with LTO. As noted, sanitizers don't help for this
apart from MSan and MSan has its own substantial problems (even if you're on a
source-based distro where you're willing to try rebuild everything).

LLVM was the most recent example but it wasn't the first, and this has
come up with LLVM in a few other places too (same root cause, wasn't
obvious at all).

I can't formally review but the implementation appears straightforward,
just a few comments below.

>
> gcc/ChangeLog:
>
>   * Makefile.in: Add gimple-valgrind.o.
>   * builtins.def (BUILT_IN_VALGRIND_MEM_UNDEF): Add new built-in.
>   * common.opt: Add new option.
>   * passes.def: Add new pass.
>   * tree-pass.h (make_pass_emit_valgrind): New function.
>   * gimple-valgrind.cc: New file.
>
> libgcc/ChangeLog:
>
>   * Makefile.in: Add valgrind.o.
>   * config.in: Regenerate.
>   * configure: Regenerate.
>   * configure.ac: Add option --enable-valgrind-annotations into libgcc
>   config.
>   * libgcc2.h (__valgrind_make_mem_undefined): New function.
>   * valgrind.c: New file.
> ---
>  gcc/Makefile.in|   1 +
>  gcc/builtins.def   |   1 +
>  gcc/common.opt |   4 ++
>  gcc/gimple-valgrind.cc | 124 +
>  gcc/passes.def |   1 +
>  gcc/tree-pass.h|   1 +
>  libgcc/Makefile.in |   2 +-
>  libgcc/config.in   |   9 +++
>  libgcc/configure   |  79 ++
>  libgcc/configure.ac|  48 
>  libgcc/libgcc2.h   |   1 +
>  libgcc/valgrind.c  |  50 +
>  12 files changed, 320 insertions(+), 1 deletion(-)
>  create mode 100644 gcc/gimple-valgrind.cc
>  create mode 100644 libgcc/valgrind.c
>
> diff --git a/gcc/Makefile.in b/gcc/Makefile.in
> index 9cc16268abf..ded6bdf1673 100644
> --- a/gcc/Makefile.in
> +++ b/gcc/Makefile.in
> @@ -1487,6 +1487,7 @@ OBJS = \
>   gimple-ssa-warn-access.o \
>   gimple-ssa-warn-alloca.o \
>   gimple-ssa-warn-restrict.o \
> + gimple-valgrind.o \
>   gimple-streamer-in.o \
>   gimple-streamer-out.o \
>   gimple-walk.o \
> diff --git a/gcc/builtins.def b/gcc/builtins.def
> index 5953266acba..42d34189f1e 100644
> --- a/gcc/builtins.def
> +++ b/gcc/builtins.def
> @@ -1064,6 +1064,7 @@ DEF_GCC_BUILTIN(BUILT_IN_VA_END, "va_end", 
> BT_FN_VOID_VALIST_REF, ATTR_N
>  DEF_GCC_BUILTIN(BUILT_IN_VA_START, "va_start", 
> BT_FN_VOID_VALIST_REF_VAR, ATTR_NOTHROW_LEAF_LIST)
>  DEF_GCC_BUILTIN(BUILT_IN_VA_ARG_PACK, "va_arg_pack", BT_FN_INT, 
> ATTR_PURE_NOTHROW_LEAF_LIST)
>  DEF_GCC_BUILTIN(BUILT_IN_VA_ARG_PACK_LEN, "va_arg_pack_len", 
> BT_FN_INT, ATTR_PURE_NOTHROW_LEAF_LIST)
> +DEF_EXT_LIB_BUILTIN(BUILT_IN_VALGRIND_MEM_UNDEF, 
> "__valgrind_make_mem_undefined", BT_FN_VOID_PTR_SIZE, ATTR_NOTHROW_LEAF_LIST)
>  DEF_EXT_LIB_BUILTIN(BUILT_IN__EXIT, "_exit", BT_FN_VOID_INT, 
> ATTR_NORETURN_NOTHROW_LEAF_LIST)
>  DEF_C99_BUILTIN(BUILT_IN__EXIT2, "_Exit", BT_FN_VOID_INT, 
> ATTR_NORETURN_NOTHROW_LEAF_LIST)
>  
> diff --git a/gcc/common.opt b/gcc/common.opt
> index f137a1f81ac..c9040386956 100644
> --- a/gcc/common.opt
> +++ b/gcc/common.opt
> @@ -2515,6 +2515,10 @@ starts and when the destructor finishes.
>  flifetime-dse=
>  Common Joined RejectNegative UInteger Var(flag_

Re: [RFC PATCH] Detecting lifetime-dse issues via Valgrind [PR66487]

2023-11-12 Thread Sam James


Alexander Monakov  writes:

> On Sat, 11 Nov 2023, Sam James wrote:
>
>> > Valgrind client requests are offered as macros that emit inline asm.  For 
>> > use
>> > in code generation, we need to wrap it in a built-in.  We know that 
>> > implementing
>> > such a built-in in libgcc is undesirable, [...].
>> 
>> Perhaps less objectionable than you think, at least given the new CFR
>> stuff from oliva from the other week that landed.
>
> Yeah; we haven't found any better solution anyway.
>
>> This is a really neat idea (it also makes me wonder if there's any other
>> opportunities for Valgrind integration like this?).
>
> To (attempt to) answer the parenthetical question, note that the patch is not
> limited to instrumenting C++ cdtors, it annotates all lifetime CLOBBER marks,
> so Valgrind should see lifetime boundaries of various on-stack arrays too.

Oh, right!

>
> (I hope positioning the new pass after build_ssa is sufficient to avoid
> annotating too much, like non-address-taken local scalars)
>
>> LLVM was the most recent example but it wasn't the first, and this has
>> come up with LLVM in a few other places too (same root cause, wasn't
>> obvious at all).
>
> I'm very curious what you mean by "this has come up with LLVM [] too": ttbomk,
> LLVM doesn't do such lifetime-based optimization yet, which is why compiling
> LLVM with LLVM doesn't break it. Can you share some examples? Or do you mean
> instances when libLLVM-miscompiled-with-GCC was linked elsewhere, and that
> program crashed mysteriously as a result?
>
> Indeed this work is inspired by the LLVM incident in PR 106943.

For that part, I meant that we had a _lot_ of different variations of
the LLVM miscompilation over the years where people would report issues
when building LLVM with GCC but trying to investigate it didn't get very
far.

i.e. It was very hard to debug and something like this would've made
things substantially easier (it takes us from the realm of "uhh maybe
compiler bug" to provable UB, which we're very used to handling).

It doesn't help that the fact that ubsan and friends can't find this
means it's often not-determined and may be worked around with either
disabling LTO or disabling various passes as a heavy hammer.

I didn't think to try toggling it when I hit issues until PR 106943.

> we don't see many other instances with -flifetime-dse workarounds in public.
> Grepping Gentoo Portage reveals only openjade. Arch applies the workaround to
> a jvm package too, and we know that Firefox and LLVM apply it on their own.
>

I had some vague memories in the back of my head so I went digging
because I enjoy this:

* Qt has hit this in the past, and I actually wonder if it's the cause
of a few other nebulous LTO issues (which we've never had a solid report
for) as well:
https://codereview.qt-project.org/c/qt/qtdeclarative/+/176272
https://bugs.gentoo.org/584818
https://bugs.gentoo.org/626070

We've given up for Qt 5 and I plan on revisiting any possible issues
with LTO with Qt 6 instead.

* Firefox as you noted: https://bugzilla.mozilla.org/show_bug.cgi?id=1232696.
* TBB (no real details available, unfortunately): 
https://github.com/oneapi-src/oneTBB/commit/51c0b2f742920535178560f31c6e91065bf87b41
* crypto++ has had a series of mysterious miscompilations and I suspect
  it may be a victim here. See 
https://github.com/weidai11/cryptopp/issues/1141#issuecomment-1208169530
  onwards but it's not the first bug like this crypto++ has had.
* codeblocks: https://bugs.gentoo.org/625696
* coin: https://bugs.gentoo.org/619378

(I would not be surprised if other wxwidgets applications are victims,
e.g. older kicad or audacity.)

Some of the results for GCC 6 at
https://bugs.gentoo.org/showdependencytree.cgi?id=582084&hide_resolved=0
were from other optimisation changes, but some are DSE (and some might
have been DSE but masked by another flag as I mentioned earlier).

If someone is extremely bored, they may want to look through our LTO tracker /
various -fno-lto/filter-lto grep results in gentoo.git to see if any
of them seem fishy. In the past, people would filter LTO without
examining the real problem. I am trying to fix this but you can't do that 
overnight.

Another good heuristic, I bet, is anything passing -O1, filtering -O3,
or if you want a general sense of crustiness, where the ebuild has to pass
-std=c++98 or so. But I'm just trying to give you fodder if you want
more examples with that last suggestion.

With less detail, I have mentions for the following in some git checkouts
from other distributions. I could not find any bug references:
* injeqt (Void Linux)
* opencollada (Void Linux)

> This patch finds

Re: [RFC PATCH] Detecting lifetime-dse issues via Valgrind [PR66487]

2023-11-12 Thread Sam James


Sam James  writes:

> Alexander Monakov  writes:
> [...]
>>
>> I'm very curious what you mean by "this has come up with LLVM [] too": 
>> ttbomk,
>> LLVM doesn't do such lifetime-based optimization yet, which is why compiling
>> LLVM with LLVM doesn't break it. Can you share some examples? Or do you mean
>> instances when libLLVM-miscompiled-with-GCC was linked elsewhere, and that
>> program crashed mysteriously as a result?
>>
>> Indeed this work is inspired by the LLVM incident in PR 106943.
>
> [...]
> I had some vague memories in the back of my head so I went digging
> because I enjoy this:
> [...]

I ended up stumbling on two more:

* charm (https://github.com/UIUC-PPL/charm/issues/1045)
* firebird (https://github.com/FirebirdSQL/firebird/issues/5384, starring richi)

Now I'm really done :)

> [...]
>>
>> Alexander
>
> thanks,
> sam



Re: [PATCH 3/6] c: Turn -Wimplicit-function-declaration into a pedpermerror [PR91092]

2023-11-13 Thread Sam James


Florian Weimer  writes:

> In the future, it may make sense to avoid cascading errors from
> the implicit declaration, especially its assumed int return type.
> This change here only changes the kind of the diagnostic, not
> its wording or consequences.
>
> gcc/c/
>
>   * doc/invoke.texi (Warning Options): Document changes.
>
> gcc/c/
>
>   * c-decl.cc (implicit_decl_pedpermerror): Rename from
>   implicit_decl_warning.  Call pedpermerror instead of
>   pedwarn and warning_at.
>   (implicitly_declare): Adjust callers.
>
> gcc/testsuite/
>
>   * c-c++-common/spellcheck-reserved.c (test, test_2): Expect
>   error instead of warning.
>   (f): Expect error instead of warning.
>   * gcc.dg/Wimplicit-function-declaration-c99.c: Compile with
>   -fpermissive due to expected warning.
>   * gcc.dg/Wimplicit-function-declaration-c99-2.c: New test.
>   Copied from gcc.dg/Wimplicit-function-declaration-c99.c.
>   Expect error.
>   * gcc.dg/missing-header-fixit-1.c: Compile with -fpermissive
>   due to expect error.
>   * gcc.dg/missing-header-fixit-1a.c: New test.  Copied from
>   gcc.dg/missing-header-fixit-1.c, but expect error.
>   * gcc.dg/missing-header-fixit-2.c: Compile with -fpermissive
>   due to expect error.
>   * gcc.dg/missing-header-fixit-2a.c: New test.  Copied from
>   gcc.dg/missing-header-fixit-2.c, but expect error.
>   * gcc.dg/missing-header-fixit-4.c: Compile with -fpermissive
>   due to expect error.
>   * gcc.dg/missing-header-fixit-4a.c: New test.  Copied from
>   gcc.dg/missing-header-fixit-4.c, but expect error.
>   * gcc.dg/missing-header-fixit-5.c: Compile with -fpermissive
>   due to expect error.
>   * gcc.dg/missing-header-fixit-5a.c: New test.  Copied from
>   gcc.dg/missing-header-fixit-5.c, but expect error.
>   * gcc.dg/pr61852.c: Expect implicit-function-declaration
>   error instead of warning.
>   * gcc.dg/spellcheck-identifiers-2.c: Compile with
>   -fpermissive due to expected warnings.
>   * gcc.dg/spellcheck-identifiers-2a.c: New test.  Copied
>   from gcc.dg/spellcheck-identifiers-2a.c.  Expect errors.
>   * gcc.dg/spellcheck-identifiers-3.c: Compile with
>   -fpermissive due to expected warnings.
>   * gcc.dg/spellcheck-identifiers-3a.c: New test.  Copied
>   from gcc.dg/spellcheck-identifiers-2a.c.  Expect errors.
>   * gcc.dg/spellcheck-identifiers-4.c: Compile with
>   -fpermissive due to expected warnings.
>   * gcc.dg/spellcheck-identifiers-4a.c: New test.  Copied
>   from gcc.dg/spellcheck-identifiers-2a.c.  Expect error.
>   * gcc.dg/spellcheck-identifiers.c: Compile with
>   -fpermissive due to expected warnings.
>   * gcc.dg/spellcheck-identifiers-1a.c: New test.  Copied
>   from gcc.dg/spellcheck-identifiers.c.  Expect errors.
>   * gcc.target/aarch64/sve/acle/general-c/ld1sh_gather_1.c (f1):
>   Expect error.
>   * gcc.target/aarch64/sve/acle/general-c/load_ext_gather_index_1.c:
>   (f1): Likewise.
>   * 
> gcc.target/aarch64/sve/acle/general-c/load_ext_gather_index_restricted_1.c:
>   (f1): Likewise.
>   * gcc.target/aarch64/sve/acle/general-c/load_ext_gather_offset_1.c:
>   (f1): Likewise.
>   * gcc.target/aarch64/sve/acle/general-c/load_ext_gather_offset_2.c:
>   (f1): Likewise.
>   * gcc.target/aarch64/sve/acle/general-c/load_ext_gather_offset_3.c:
>   (f1): Likewise.
>   * gcc.target/aarch64/sve/acle/general-c/load_ext_gather_offset_4.c:
>   (f1): Likewise.
>   * gcc.target/aarch64/sve/acle/general-c/load_ext_gather_offset_5.c:
>   (f1): Likewise.
>   * 
> gcc.target/aarch64/sve/acle/general-c/load_ext_gather_offset_restricted_1.c:
>   (f1): Likewise.
>   * 
> gcc.target/aarch64/sve/acle/general-c/load_ext_gather_offset_restricted_2.c:
>   (f1): Likewise.
>   * 
> gcc.target/aarch64/sve/acle/general-c/load_ext_gather_offset_restricted_3.c:
>   (f1): Likewise.
>   * 
> gcc.target/aarch64/sve/acle/general-c/load_ext_gather_offset_restricted_4.c:
>   (f1): Likewise.

This is PR91092. In due course, we'll also have to update the porting to
GCC 14 page.


Re: [PATCH 0/6] Turn some C warnings into errors by default

2023-11-13 Thread Sam James


Florian Weimer  writes:

> This patch series converts the following warnings into errors by
> default:
>
>   -Wint-conversion
>   -Wimplicit-function-declaration
>   -Wimplicit-int
>   -Wreturn-mismatch
>   -Wincompatible-pointer-types
>
> As explained in the first commit, I decided not to use permerror_opt
> because it does not exhibit the existing behavior for -pedantic-errors.
>
> The impact on existing sources of the last commit is not really known to
> me at this point.  I plan to start a Fedora build later this week with
> an instrumented compiler, to see how much of a compatible impact it will
> be.  The first conversion pass through Fedora only covered
> -Wimplicit-function-declaration, -Wimplicit-int.  I started looking at
> -Wint-conversion, and it did not seem to be too bad, so I think
> including it should be fine.  I'm more worried about
> -Wincompatible-pointer-types.
>
> I have not yet added a new overview test for -fpermissive.  Such a test
> should trigger all the dozen or so places where I introduced
> pedpermerror, and see what happens under multiple dialects, each with
> -fpermissive and without, and maybe also with and withoyt for
> -pedantic-errors in -std=gnu89 and default modes.  I plan to do this
> once I get some initial feedback on the direction of these series
> because this test would likely be obsolete fairly quickly if changes to
> the diagnostics are required.  I did copy some existing tests to test
> both the error and warning (-fpermissive) diagnostics, and adjusted
> others to expect errors, so there is already quite a bit coverage
> without that overview test.
>
> Right now, this series breaks the build on aarch64-linux-gnu due to an
> incompatible pointer assignment in libgcc:
>
>   [PATCH] aarch64: Avoid -Wincompatible-pointer-types warning in Linux 
> unwinder
>   
> 
>
> Other targets had the same issue previously, but I've already fixed most
> of them (I hope).  There could of course be similar issues lurking in
> target-specific code, or even in system headers.
>
> With the recent testsuite fixes, the testsuite should be fairly clean
> despite these changes.  I verified that on i686-linux-gnu,
> powerpc64-linux-gnu, and x86_64-linux-gnu.  There is one
> aarch64-linux-gnu testsuite change I'd like the AArch64 maintainers to
> review:

I'll incrementally work on the other targets once stuff lands.

>
>   [PATCH] aarch64: Call named function in gcc.target/aarch64/aapcs64/ice_1.c
>   
> 
>
> Recently, I also found a problem in the gm2 testsuite:
>
>   [PATCH] gm2: Add missing declaration of m2pim_M2RTS_Terminate to test
>   
> 
>

Also, please tag PR96284 for all of these.

> Thanks,
> Florian
>
>
> Florian Weimer (6):
>   c-family: Introduce pedpermerror
>   c: Turn int-conversion warnings into permerrors
>   c: Turn -Wimplicit-function-declaration into a pedpermerror
>   c: Turn -Wimplicit-int into a pedpermerror
>   c: Turn -Wreturn-mismatch into a pedpermerror
>   c: Turn -Wincompatible-pointer-types into a pedpermerror
>
>  gcc/c-family/c-common.h   |   4 +
>  gcc/c-family/c-warn.cc|  34 
>  gcc/c/c-decl.cc   |  40 ++--
>  gcc/c/c-typeck.cc | 164 +--
>  gcc/diagnostic-core.h |   3 +
>  gcc/diagnostic.cc |   7 +
>  gcc/doc/invoke.texi   |  33 +++-
>  gcc/testsuite/c-c++-common/pr77624-1.c|   4 +-
>  .../c-c++-common/spellcheck-reserved.c|   4 +-
>  gcc/testsuite/gcc.dg/20030906-1.c |   2 +-
>  gcc/testsuite/gcc.dg/20030906-1a.c|  21 ++
>  gcc/testsuite/gcc.dg/20030906-2.c |   2 +-
>  gcc/testsuite/gcc.dg/20030906-2a.c|  21 ++
>  .../Wimplicit-function-declaration-c99-2.c|   7 +
>  .../Wimplicit-function-declaration-c99.c  |   2 +-
>  gcc/testsuite/gcc.dg/Wimplicit-int-1.c|   2 +-
>  gcc/testsuite/gcc.dg/Wimplicit-int-1a.c   |  11 ++
>  gcc/testsuite/gcc.dg/Wimplicit-int-4.c|   2 +-
>  gcc/testsuite/gcc.dg/Wimplicit-int-4a.c   |  11 ++
>  .../gcc.dg/Wincompatible-pointer-types-2.c|   2 +-
>  .../gcc.dg/Wincompatible-pointer-types-4.c|   2 +-
>  .../gcc.dg/Wincompatible-pointer-types-5.c|  10 +
>  .../gcc.dg/Wincompatible-pointer-types-6.c|  10 +
>  gcc/testsuite/gcc.dg/Wint-conversion-2.c  |   2 +-
>  gcc/testsuite/gcc.dg/Wint-conversion-3.c  |   2 +-
>  gcc/testsuite/gcc.dg/Wint-conversion-4.c  |  14 ++
>  gcc/testsuite/gcc.dg/Wreturn-mismatch-1.c |   2 +-
>  gcc/testsuite/gcc.dg/Wreturn-mismatch-1a.c|  40 
>  gcc/testsuite/gcc.dg/Wreturn-mismatch-2.c |   2 +-
>  gcc/testsuite/gcc.dg/Wreturn-m

Re: [PATCH 2/6] c: Turn int-conversion warnings into permerrors [PR106416]

2023-11-13 Thread Sam James


Florian Weimer  writes:

> gcc/
>
>   * doc/invoke.texi (Warning Options): Document changes.
>
> gcc/c/
>
>   * c-typeck.cc (build_conditional_expr): Use pedpermerror for
>   pointer/integer type mismatches, based on -Wint-conversion.
>   (pedwarn_pedpermerror_init, permerror_init): New function.
>   (pedwarn_init): Call pedwarn_pedpermerror_init.
>   (convert_for_assignment): Use pedpermerror and
>   pedpermerror_init for -Wint-conversion  warnings.
>
> gcc/testsuite/
>
>   * c-c++-common/pr77624-1.c (foo, bar): Expect
>   error instead of warning.
>   * gcc.dg/Wint-conversion-2.c: Compile with -fpermissive due
>   to expected int-conversion warning.
>   * gcc.dg/Wint-conversion-3.c: Likewise.
>   * gcc.dg/Wint-conversion-4.c: New test.  Based on
>   gcc.dg/Wint-conversion-3.c.  Expect int-conversion errors.
>   * gcc.dg/assign-warn-1.c: Compile with -fpermissive.
>   * gcc.dg/assign-warn-4.c: New file.  Extracted from
>   assign-warn1.c.  Expect int-cnversion errors.
>   * gcc.dg/diagnostic-types-1.c: compile with -fpermissive.
>   * gcc.dg/diagnostic-types-2.c: New file.  Extracted from
>   gcc.dg/diagnostic-types-1.c.  Expect some errors instead of
>   warnings.
>   * gcc.dg/gomp/pr35738.c: Compile with -fpermissive due to
>   expected int-conversion error.
>   * gcc.dg/gomp/pr35738-2.c: New test.  Based on
>   gcc.dg/gomp/pr35738.c.  Expect int-converison errors.
>   * gcc.dg/init-excess-3.c: Expect int-converison errors.
>   * gcc.dg/overflow-warn-1.c: Likewise.
>   * gcc.dg/overflow-warn-3.c: Likewise.
>   * gcc.dg/param-type-mismatch.c: Compile with -fpermissive.
>   * gcc.dg/param-type-mismatch-2.c: New test.  Copied from
>   gcc.dg/param-type-mismatch.c.  Expect errors.
>   * gcc.dg/pr61162-2.c: Compile with -fpermissive.
>   * gcc.dg/pr61162-3.c: New test. Extracted from
>   gcc.dg/pr61162-2.c.  Expect int-conversion errors.
>   * gcc.dg/spec-barrier-3.c: Use -fpermissive due to expected
>   int-conversion error.
>   * gcc.dg/spec-barrier-3a.c: New test.  Based on
>   gcc.dg/spec-barrier-3.c.  Expect int-conversion errors.
>   * gcc.target/aarch64/acle/memtag_2.c: Use -fpermissive due to expected
>   int-conversion error.
>   * gcc.target/aarch64/acle/memtag_2a.c: New test.  Copied from
>   gcc.target/aarch64/acle/memtag_2.c.  Expect error.
>   * gcc.target/aarch64/sve/acle/general-c/load_3.c (f1): Expect
>   error.
>   * gcc.target/aarch64/sve/acle/general-c/store_2.c (f1): Likewise.
>   * gcc.target/aarch64/sve/acle/general-c/store_scatter_index_1.c
>   (f1): Likewise.
>   * 
> gcc.target/aarch64/sve/acle/general-c/store_scatter_index_restricted_1.c
>   (f1): Likewise.
>   * gcc.target/aarch64/sve/acle/general-c/store_scatter_offset_2.c
>   (f1): Likewise.
>   * 
> gcc.target/aarch64/sve/acle/general-c/store_scatter_offset_restricted_1.c
>   (f1): Likewise.

This is PR106416.

> ---


Re: [PATCH 4/6] c: Turn -Wimplicit-int into a pedpermerror [PR91093]

2023-11-13 Thread Sam James


Florian Weimer  writes:

> There is a missed opportunity here to issue spelling diagnostics
> in prototype declarations (e.g., for “extern int foo (int32t);”).
>
> gcc/
>
>   * doc/invoke.texi (Warning Options): Document changes.
>
> gcc/c/
>
>   * c-decl.cc (warn_defaults_to): Call emit_diagnostic_valist
>   instead of reimplementing it. Issue a pedpermerror for C99
>   and later.
>   (store_parm_decls_oldstyle): Call pedpermerror for
>   OPT_Wimplicit_int.
>
> gcc/testsuite/
>
>   * gcc.dg/Wimplicit-int-1.c: Compile with -fpermissive due to
>   expected warning.
>   * gcc.dg/Wimplicit-int-4.c: Likewise.
>   * gcc.dg/Wimplicit-int-1a.c: New test.  Copied from
>   gcc.dg/Wimplicit-int-1.c, but expect errors.
>   * gcc.dg/Wimplicit-int-4a.c: New test.  Copied from
>   gcc.dg/Wimplicit-int-4.c, but expect errors.
>   * gcc.dg/gnu23-attr-syntax-2.c: Compile with -fpermissive
>   due to expected implicit-int error.
>   * gcc.dg/gnu23-attr-syntax-3.c: New test.  Copied from
>   gcc.dg/gnu23-attr-syntax-2.c, but expect an error.
>   * gcc.dg/pr105635.c: Build with -fpermissive due to implicit
>   int.
>   * gcc.dg/pr105635-2.c: New test.  Copied from
>   gcc.dg/pr105635.c.  Expect implicit int error.
>   * gcc.dg/noncompile/pr79758.c: Build with -fpermissive due to
>   implicitint.
>   * gcc.dg/noncompile/pr79758-2.c: New test.  Copied from
>   gcc.dg/noncompile/pr79758.c.  Expect implicit int error.

This is PR91093.


Re: [PATCH v2 5/8] c: Do not ignore some forms of -Wimplicit-int in system headers

2023-11-14 Thread Sam James


Florian Weimer  writes:

> Most -Wimplicit-int warnings were unconditionally disabled for system
> headers.  Only missing types for parameters in old-style function
> definitions resulted in warnings.  This is inconsistent with the
> treatment of other permerrors, which are active in system headers.

The situation with system headers is kind of a mess still. I went
looking for a bug for the -Wimplicit-int behaviour but I only found
PR78000 for -Wimplicit-function-declaration. But in the bug, Joseph
makes the same observation.

I don't suppose he'll want to block on that at this late point though.

Do you know offhand what Clang's behaviour is wrt warnings in system headers?

>
> gcc/c/
>
>   * c-decl.cc (grokdeclarator): Do not skip -Wimplicit-int
>   warnings or errors in system headers.
>
> gcc/testsuite/
>
>   * gcc.dg/permerror-system.c: Expect all -Wimplicit-int
>   permerrors.
> ---
>  gcc/c/c-decl.cc | 2 +-
>  gcc/testsuite/gcc.dg/permerror-system.c | 5 +
>  2 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc
> index 893e24f7dc6..d16958113a8 100644
> --- a/gcc/c/c-decl.cc
> +++ b/gcc/c/c-decl.cc
> @@ -6845,7 +6845,7 @@ grokdeclarator (const struct c_declarator *declarator,
>  
>/* Diagnose defaulting to "int".  */
>  
> -  if (declspecs->default_int_p && !in_system_header_at (input_location))
> +  if (declspecs->default_int_p)
>  {
>/* Issue a warning if this is an ISO C 99 program or if
>-Wreturn-type and this is a function, or if -Wimplicit;
> diff --git a/gcc/testsuite/gcc.dg/permerror-system.c 
> b/gcc/testsuite/gcc.dg/permerror-system.c
> index 60c65ffc1d4..cad48c93f90 100644
> --- a/gcc/testsuite/gcc.dg/permerror-system.c
> +++ b/gcc/testsuite/gcc.dg/permerror-system.c
> @@ -10,7 +10,12 @@
>  
>  /* { dg-error "'f1' \\\[-Wimplicit-function-declaration\\\]" "" { target 
> *-*-* } 10 } */
>  
> +/* { dg-error "'implicit_int_1' \\\[-Wimplicit-int\\\]" "" { target *-*-* } 
> 13 } */
> +/* { dg-error "'implicit_int_2' \\\[-Wimplicit-int\\\]" "" { target *-*-* } 
> 14 } */
> +/* { dg-error "'implicit_int_3' \\\[-Wimplicit-int\\]" "" { target *-*-* } 
> 15 } */
> +/* { dg-error "return type defaults to 'int' \\\[-Wimplicit-int\\\]" "" { 
> target *-*-* } 16 } */
>  /* { dg-error "type of 'i' defaults to 'int' \\\[-Wimplicit-int\\\]" "" { 
> target *-*-*} 16 } */
> +/* { dg-error "type defaults to 'int' in type name \\\[-Wimplicit-int\\\]" 
> "" { target *-*-* } 19 } */
>  
>  /* { dg-error "pointer/integer type mismatch in conditional expression 
> \\\[-Wint-conversion\\\]" "" { target *-*-* } 29 } */
>  /* { dg-error "pointer/integer type mismatch in conditional expression 
> \\\[-Wint-conversion\\\]" "" { target *-*-* } 30 } */



Re: [PATCH v2 5/8] c: Do not ignore some forms of -Wimplicit-int in system headers

2023-11-14 Thread Sam James


Florian Weimer  writes:

> * Sam James:
>
>> Florian Weimer  writes:
>>
>>> Most -Wimplicit-int warnings were unconditionally disabled for system
>>> headers.  Only missing types for parameters in old-style function
>>> definitions resulted in warnings.  This is inconsistent with the
>>> treatment of other permerrors, which are active in system headers.
>>
>> The situation with system headers is kind of a mess still. I went
>> looking for a bug for the -Wimplicit-int behaviour but I only found
>> PR78000 for -Wimplicit-function-declaration. But in the bug, Joseph
>> makes the same observation.
>>
>> I don't suppose he'll want to block on that at this late point though.
>>
>> Do you know offhand what Clang's behaviour is wrt warnings in system
>> headers?
>
> Clang ignores these new errors in system headers by default.  I don't
> know if that's deliberate or a bug.  Our permerrors are deliberately
> active in system headers.  As the test shows, -Wimplicit-int really was
> the outlier here because of that check outside the permerror machinery.

Thanks - my assumption was that it was more widespread because of a few
lingering bugs I've seen a few projects complain about, but maybe they
were using old versions or similar.

>
> I expect system headers are quite clean actually because they have to be
> for C++ compatibility.  Some things have improved in the last 25 years.
>

Oh, duh. Thank you!




Re: [PATCH v2 0/8] Turn some C warnings into errors by default

2023-11-14 Thread Sam James


Florian Weimer  writes:

> This new series covers:
>
>   -Wint-conversion
>   -Wimplicit-function-declaration
>   -Wimplicit-int
>   -Wreturn-mismatch
>   -Wincompatible-pointer-types
>   -Wdeclaration-missing-parameter-type (new)
>
> There are now gcc.dg/permerror-*.c tests which track the graduation of
> warnings to errors.
>
> It turns out that pedpermerror was indeed unnecessary, and I can use
> permerror_opt directly (or DK_PERMERROR in case I call one of the
> low-level functions).
>
> While working on this I found out that -Wimplicit-int is mostly
> ineffective in system headers, so I fixed that for consistency.  I also
> added a new -Wdeclaration-missing-parameter-type warning because missing
> types in parameter lists in function declarations (as opposed to
> old-style function definitions) are not covered by -Wimplicit-int, and
> probably shouldn't be because the omitted types are not necessarily int.
>
> This still depends on a couple of unreviewed patches.  In particular, I
> do not want to break the AArch64 bootstrap, so the first patch looks
> like a blocker.
>
>   [PATCH] aarch64: Avoid -Wincompatible-pointer-types warning in Linux 
> unwinder
>   
> 
>
>   [PATCH] aarch64: Call named function in gcc.target/aarch64/aapcs64/ice_1.c
>   
> 
>
>   [PATCH] gm2: Add missing declaration of m2pim_M2RTS_Terminate to test
>   
> 

When posting v3, I guess consider CCing the aarch64 maintainers (or just
pinging them separately)?

I'm not really sure how approval of (an earlier version of) this series
interacts with stage 3 ending on Friday.

>
> Thanks,
> Florian
>
>
> Florian Weimer (8):
>   Add tests for validating future C permerrors
>   c: Turn int-conversion warnings into permerrors
>   c: Turn -Wimplicit-function-declaration into a permerror
>   c: Turn -Wimplicit-int into a permerror
>   c: Do not ignore some forms of -Wimplicit-int in system headers
>   c: Turn -Wreturn-mismatch into a permerror
>   c: Turn -Wincompatible-pointer-types into a permerror
>   c: Add new -Wdeclaration-missing-parameter-type permerror
>
>  gcc/c-family/c.opt|   4 +
>  gcc/c/c-decl.cc   |  71 +++
>  gcc/c/c-typeck.cc | 164 ---
>  gcc/doc/invoke.texi   |  50 -
>  gcc/testsuite/c-c++-common/pr77624-1.c|   4 +-
>  .../c-c++-common/spellcheck-reserved.c|   4 +-
>  gcc/testsuite/gcc.dg/20030906-1.c |   2 +-
>  gcc/testsuite/gcc.dg/20030906-1a.c|  21 ++
>  gcc/testsuite/gcc.dg/20030906-2.c |   2 +-
>  gcc/testsuite/gcc.dg/20030906-2a.c|  21 ++
>  .../Wimplicit-function-declaration-c99-2.c|   7 +
>  .../Wimplicit-function-declaration-c99.c  |   2 +-
>  gcc/testsuite/gcc.dg/Wimplicit-int-1.c|   2 +-
>  gcc/testsuite/gcc.dg/Wimplicit-int-1a.c   |  11 ++
>  gcc/testsuite/gcc.dg/Wimplicit-int-4.c|   2 +-
>  gcc/testsuite/gcc.dg/Wimplicit-int-4a.c   |  11 ++
>  .../gcc.dg/Wincompatible-pointer-types-2.c|   2 +-
>  .../gcc.dg/Wincompatible-pointer-types-5.c|  10 +
>  gcc/testsuite/gcc.dg/Wint-conversion-2.c  |   2 +-
>  gcc/testsuite/gcc.dg/Wint-conversion-3.c  |   2 +-
>  gcc/testsuite/gcc.dg/Wint-conversion-4.c  |  14 ++
>  gcc/testsuite/gcc.dg/Wreturn-mismatch-1.c |   2 +-
>  gcc/testsuite/gcc.dg/Wreturn-mismatch-1a.c|  40 
>  gcc/testsuite/gcc.dg/Wreturn-mismatch-2.c |   2 +-
>  gcc/testsuite/gcc.dg/Wreturn-mismatch-2a.c|  41 
>  gcc/testsuite/gcc.dg/anon-struct-11.c |   5 +-
>  gcc/testsuite/gcc.dg/anon-struct-11a.c| 111 +++
>  gcc/testsuite/gcc.dg/anon-struct-13.c |   2 +-
>  gcc/testsuite/gcc.dg/anon-struct-13a.c|  76 +++
>  gcc/testsuite/gcc.dg/assign-warn-1.c  |   2 +-
>  gcc/testsuite/gcc.dg/assign-warn-4.c  |  21 ++
>  .../gcc.dg/builtin-arith-overflow-4.c |   2 +-
>  .../gcc.dg/builtin-arith-overflow-4a.c|  43 
>  gcc/testsuite/gcc.dg/c23-qual-4.c |   6 +-
>  gcc/testsuite/gcc.dg/dfp/composite-type-2.c   |  58 ++
>  gcc/testsuite/gcc.dg/dfp/composite-type.c |   2 +-
>  gcc/testsuite/gcc.dg/diag-aka-1.c |   2 +-
>  gcc/testsuite/gcc.dg/diag-aka-1a.c|  29 +++
>  .../gcc.dg/diagnostic-range-bad-return-2.c|  52 +
>  .../gcc.dg/diagnostic-range-bad-return.c  |   2 +-
>  gcc/testsuite/gcc.dg/diagnostic-types-1.c |   2 +-
>  gcc/testsuite/gcc.dg/diagnostic-types-2.c |  24 +++
>  gcc/testsuite/gcc.dg/enum-compat-1.c  |   2 +-
>  gcc/testsuite/gcc.dg/enum-compat-2.c  |  32 +++
>  gcc/testsuite/gcc.dg/func-ptr-conv-1.c|   2 +-
>  gcc/testsuite/gcc.dg/fu

Re: [PATCH 1/4] libsanitizer: merge from upstream (c425db2eb558c263)

2023-11-15 Thread Sam James


Jakub Jelinek  writes:

> Hi!
>
> The following patch is result of libsanitizer/merge.sh
> from c425db2eb558c263 (yesterday evening).
>
> Bootstrapped/regtested on x86_64-linux and i686-linux (together with
> the follow-up 3 patches I'm about to post).
>
> Iain, could you please check Darwin?
>
> And anyone else their favourite platform?
>
> BTW, seems upstream has added riscv64 support for I think lsan/tsan,
> so if anyone is willing to try it there, it would be a matter of
> copying e.g. the s390*-*-linux* libsanitizer/configure.tgt entry
> to riscv64-*-linux* with the obvious s/s390x/riscv64/ change in it.

Thank you for doing it.

This will fix PR109882.

>
> Thanks.
>
>   Jakub
>
> [2. application/gzip; gcc14-libsanitizer-merge.patch.gz]...



Re: [committed] hppa: Revise REG+D address support to allow long displacements before reload

2023-11-16 Thread Sam James


John David Anglin  writes:

> On 2023-11-16 4:52 p.m., Jeff Law wrote:
>>
>>
>> On 11/16/23 10:54, John David Anglin wrote:
>>> Tested on hppa-unknown-linux-gnu and hppa64-hp-hpux11.11.  Committed
>>> to trunk.
>>>
>>> This patch works around problem compiling python3.11 by improving
>>> REG+D address handling.  The change results in smaller code and
>>> reduced register pressure.
>>>
>>> Dave
>>> ---
>>>
>>> hppa: Revise REG+D address support to allow long displacements before reload
>>>
>>> In analyzing PR rtl-optimization/112415, I realized that restricting
>>> REG+D offsets to 5-bits before reload results in very poor code and
>>> complexities in optimizing these instructions after reload.  The
>>> general problem is long displacements are not allowed for floating
>>> point accesses when generating PA 1.1 code.  Even with PA 2.0, there
>>> is a ELF linker bug that prevents using long displacements for
>>> floating point loads and stores.
>>>
>>> In the past, enabling long displacements before reload caused issues
>>> in reload.  However, there have been fixes in the handling of reloads
>>> for floating-point accesses.  This change allows long displacements
>>> before reload and corrects a couple of issues in the constraint
>>> handling for integer and floating-point accesses.
>>>
>>> 2023-11-16  John David Anglin  
>>>
>>> gcc/ChangeLog:
>>>
>>> PR rtl-optimization/112415
>>> * config/pa/pa.cc (pa_legitimate_address_p): Allow 14-bit
>>> displacements before reload.  Simplify logic flow.  Revise
>>> comments.
>>> * config/pa/pa.h (TARGET_ELF64): New define.
>>> (INT14_OK_STRICT): Update define and comment.
>>> * config/pa/pa64-linux.h (TARGET_ELF64): Define.
>>> * config/pa/predicates.md (base14_operand): Don't check
>>> alignment of short displacements.
>>> (integer_store_memory_operand): Don't return true when
>>> reload_in_progress is true.  Remove INT_5_BITS check.
>>> (floating_point_store_memory_operand): Don't return true when
>>> reload_in_progress is true.  Use INT14_OK_STRICT to check
>>> whether long displacements are always okay.
>> I strongly suspect this is going to cause problems in the end.
>>
>> I've already done what you're trying to do.  It'll likely look fine
>> for an extended period of time, but it will almost certainly break
>> one day.

Jeff, I don't suppose you could dig out the old bugs/commits just out of
interest?

> I could happen.  If it happens and can't be fixed, it's easy enough to return 
> false in
> pa_legitimate_address_p before reload.  Maybe we could add an optimization 
> option for this.
>
> As it stands, the code improvement for python is significant.  I don't think 
> f-m-o can fix things after reload.
>a
> Hopefully, Sam will test the change with various package builds on gentoo.  
> Debian is still on gcc-13.

Yeah, happy to do that. We haven't got GCC 14 deployed in the wild, but
we have it available for people who want to test and opt-in to it.

Fingers crossed it's calm. I'll let you know if it isn't ;)

> I'm not seeing any obvious problems in the gcc testsuite.  It needs testing 
> on packages that do extensive
> floating point calculations.

OK, I'll focus on those.

>
> Dave



Re: [committed] hppa: Revise REG+D address support to allow long displacements before reload

2023-11-16 Thread Sam James


Sam James  writes:

> John David Anglin  writes:
>
>> On 2023-11-16 4:52 p.m., Jeff Law wrote:
>>>
>>>
>>> On 11/16/23 10:54, John David Anglin wrote:
>>>> Tested on hppa-unknown-linux-gnu and hppa64-hp-hpux11.11.  Committed
>>>> to trunk.
>>>>
>>>> This patch works around problem compiling python3.11 by improving
>>>> REG+D address handling.  The change results in smaller code and
>>>> reduced register pressure.
>>>>
>>>> Dave
>>>> ---
>>>>
>>>> hppa: Revise REG+D address support to allow long displacements before 
>>>> reload
>>>>
>>>> In analyzing PR rtl-optimization/112415, I realized that restricting
>>>> REG+D offsets to 5-bits before reload results in very poor code and
>>>> complexities in optimizing these instructions after reload.  The
>>>> general problem is long displacements are not allowed for floating
>>>> point accesses when generating PA 1.1 code.  Even with PA 2.0, there
>>>> is a ELF linker bug that prevents using long displacements for
>>>> floating point loads and stores.
>>>>
>>>> In the past, enabling long displacements before reload caused issues
>>>> in reload.  However, there have been fixes in the handling of reloads
>>>> for floating-point accesses.  This change allows long displacements
>>>> before reload and corrects a couple of issues in the constraint
>>>> handling for integer and floating-point accesses.
>>>>
>>>> 2023-11-16  John David Anglin  
>>>>
>>>> gcc/ChangeLog:
>>>>
>>>> PR rtl-optimization/112415
>>>> * config/pa/pa.cc (pa_legitimate_address_p): Allow 14-bit
>>>> displacements before reload.  Simplify logic flow.  Revise
>>>> comments.
>>>> * config/pa/pa.h (TARGET_ELF64): New define.
>>>> (INT14_OK_STRICT): Update define and comment.
>>>> * config/pa/pa64-linux.h (TARGET_ELF64): Define.
>>>> * config/pa/predicates.md (base14_operand): Don't check
>>>> alignment of short displacements.
>>>> (integer_store_memory_operand): Don't return true when
>>>> reload_in_progress is true.  Remove INT_5_BITS check.
>>>> (floating_point_store_memory_operand): Don't return true when
>>>> reload_in_progress is true.  Use INT14_OK_STRICT to check
>>>> whether long displacements are always okay.
>>> I strongly suspect this is going to cause problems in the end.
>>>
>>> I've already done what you're trying to do.  It'll likely look fine
>>> for an extended period of time, but it will almost certainly break
>>> one day.
>
> Jeff, I don't suppose you could dig out the old bugs/commits just out of
> interest?
>
>> I could happen.  If it happens and can't be fixed, it's easy enough to 
>> return false in
>> pa_legitimate_address_p before reload.  Maybe we could add an optimization 
>> option for this.

I might hack in an option for local testing so I can quickly check
with/without...

>>
>> As it stands, the code improvement for python is significant.  I don't think 
>> f-m-o can fix things after reload.
>>a
>> Hopefully, Sam will test the change with various package builds on gentoo.  
>> Debian is still on gcc-13.
>
> Yeah, happy to do that. We haven't got GCC 14 deployed in the wild, but
> we have it available for people who want to test and opt-in to it.
>
> Fingers crossed it's calm. I'll let you know if it isn't ;)
>
>> I'm not seeing any obvious problems in the gcc testsuite.  It needs testing 
>> on packages that do extensive
>> floating point calculations.
>
> OK, I'll focus on those.
>
>>
>> Dave



Re: [PATCH V3 4/7] ira: Support subreg copy

2023-11-18 Thread Sam James


Lehua Ding  writes:

> Hi Vladimir,
>
> On 2023/11/17 22:05, Vladimir Makarov wrote:
>> On 11/16/23 21:06, Lehua Ding wrote:
>>> Hi Vladimir,
>>>
>>> Thank you so much for your review. Based on your comments, I feel
>>> like there are a lot of issues, especially the long compile time
>>> issue. So I'm going to reorganize and refactor the patches so that
>>> as many of them as possible can be reviewed separately. this way
>>> there will be fewer patches to support subreg in the end. I plan to
>>> split it into four separate patches like bellow. What do you think?
>>>
>> I can wait for the new version patches.  The only issue is stage1 deadline.
>> In my opinion, I'd recommend to work on the patches more and start
>> their submission right before GCC-14 release (somewhere in April).
>
> Quite agree, I'll rewrite the patches a bit better before resend new
> version patchs, stage 1 is definitely too late. When you say before
> GCC-14 release do you mean at GCC 14 stage 3? Is it possible to commit
> such changes at stage 3? I was thinking that if I miss GCC 14 stage 1
> I should have to wait until GCC 15 stage 1.

I took it to mean "submit it during GCC 14 stage 3 for merging during
GCC 15 stage 1", as the idea would be that if you're basing it on the
state of the tree & doing further/final testing on GCC 14 stage 3,
the tree should be in a stable state by then with only regression fixes
going in, rather than other changes which might disrupt your testing.

This means you are not constantly rebasing and getting new test failures
possibly due to changes other than yours. It also means lots of time to
review and fix any problems with less pressure.

>
>> You need a lot of testing for the patches: major targets (x86-64,
>> aarhc64, ppc64), some big endian targets, a 32-bit targets. Knowing
>> how even small changes in RA can affect many targets, e.g. GCC
>> testsuite results (there are a lot of different target tests which
>> expect a particular output),  it is better to do this on stabilized
>> GCC and stage3 is the best time for this.  In any case I'll approve
>> patches only if you have successful bootstraps and no GCC testsuite
>> regression on x86-64, ppc64le/be, aarhc64, i686.
>> Also you have a lot of compile time performance issues which you
>> need to address.  So I guess you will be overwhelmed by new
>> different target PRs after committing the patches if you will do
>> this now.  You will have more time and less pressure work if you
>> commit these patches in April.
>
> Hm, I'll test the targets I can get first. I'll figure out the
> other targets later.
>

The compiler farm can provide access to a bunch of targets and the
community may be able to help with access to others if needed.

thanks,
sam


Re: [PATCH V3 4/7] ira: Support subreg copy

2023-11-18 Thread Sam James


Lehua Ding  writes:

> Hi Sam,
>
> On 2023/11/18 16:06, Sam James wrote:
>> Lehua Ding  writes:
>> 
>>> Hi Vladimir,
>>>
>>> On 2023/11/17 22:05, Vladimir Makarov wrote:
>>>> On 11/16/23 21:06, Lehua Ding wrote:
>>>>> Hi Vladimir,
>>>>>
>>>>> Thank you so much for your review. Based on your comments, I feel
>>>>> like there are a lot of issues, especially the long compile time
>>>>> issue. So I'm going to reorganize and refactor the patches so that
>>>>> as many of them as possible can be reviewed separately. this way
>>>>> there will be fewer patches to support subreg in the end. I plan to
>>>>> split it into four separate patches like bellow. What do you think?
>>>>>
>>>> I can wait for the new version patches.  The only issue is stage1 deadline.
>>>> In my opinion, I'd recommend to work on the patches more and start
>>>> their submission right before GCC-14 release (somewhere in April).
>>>
>>> Quite agree, I'll rewrite the patches a bit better before resend new
>>> version patchs, stage 1 is definitely too late. When you say before
>>> GCC-14 release do you mean at GCC 14 stage 3? Is it possible to commit
>>> such changes at stage 3? I was thinking that if I miss GCC 14 stage 1
>>> I should have to wait until GCC 15 stage 1.
>> I took it to mean "submit it during GCC 14 stage 3 for merging
>> during
>> GCC 15 stage 1", as the idea would be that if you're basing it on the
>> state of the tree & doing further/final testing on GCC 14 stage 3,
>> the tree should be in a stable state by then with only regression fixes
>> going in, rather than other changes which might disrupt your testing.
>> This means you are not constantly rebasing and getting new test
>> failures
>> possibly due to changes other than yours. It also means lots of time to
>> review and fix any problems with less pressure.
>
> Oh, looks like I misunderstood. Thanks for the correction.
>
>>>
>>>> You need a lot of testing for the patches: major targets (x86-64,
>>>> aarhc64, ppc64), some big endian targets, a 32-bit targets. Knowing
>>>> how even small changes in RA can affect many targets, e.g. GCC
>>>> testsuite results (there are a lot of different target tests which
>>>> expect a particular output),  it is better to do this on stabilized
>>>> GCC and stage3 is the best time for this.  In any case I'll approve
>>>> patches only if you have successful bootstraps and no GCC testsuite
>>>> regression on x86-64, ppc64le/be, aarhc64, i686.
>>>> Also you have a lot of compile time performance issues which you
>>>> need to address.  So I guess you will be overwhelmed by new
>>>> different target PRs after committing the patches if you will do
>>>> this now.  You will have more time and less pressure work if you
>>>> commit these patches in April.
>>>
>>> Hm, I'll test the targets I can get first. I'll figure out the
>>> other targets later.
>>>
>> The compiler farm can provide access to a bunch of targets and the
>> community may be able to help with access to others if needed.
>
> I applied for cfarm access the other day, I'll try to use it. Thanks.

No problem. If you need some Linux targets not on the cfarm, let me
know. I can probably help with hppa/sparc at least and I know someone
with alpha, mips.


Re: Propagate value ranges of return values

2023-11-19 Thread Sam James


Jan Hubicka  writes:

> Hi,
> this patch implements very basic propaation of return value ranges from VRP
> pass.  This helps std::vector's push_back since we work out value range of
> allocated block.  This propagates only within single translation unit.  I 
> hoped
> we will also do the propagation at WPA stage, but that needs more work on
> ipa-cp side.
>
> I also added code auto-detecting return_nonnull and corresponding 
> -Wsuggest-attribute
>
> Variant of this patch bootstrapped/regtested x86_64-linux, testing with
> this version is running.  I plan to commit the patch at Monday provided
> there are no issues.
>
> gcc/ChangeLog:
>
>   * cgraph.cc (add_detected_attribute_1): New function.
>   (cgraph_node::add_detected_attribute): New member function.
>   * cgraph.h (struct cgraph_node): Declare it.
>   * common.opt: Add Wsuggest-attribute=returns_nonnull.
>   * doc/invoke.texi: Document +Wsuggest-attribute=returns_nonnull.
>   * gimple-range-fold.cc: Include ipa-prop and dependencies.
>   (fold_using_range::range_of_call): Look for return value range.
>   * ipa-prop.cc (struct ipa_return_value_summary): New structure.
>   (class ipa_return_value_sum_t): New summary.
>   (ipa_record_return_value_range): New function.
>   (ipa_return_value_range): New function.
>   * ipa-prop.h (ipa_return_value_range): Declare.
>   (ipa_record_return_value_range): Declare.
>   * ipa-pure-const.cc (warn_function_returns_nonnull): New function.
>   * ipa-utils.h (warn_function_returns_nonnull): Declare.
>   * symbol-summary.h: Fix comment typo.
>   * tree-vrp.cc (execute_ranger_vrp): Record return values.
>
> gcc/testsuite/ChangeLog:
>
>   * gcc.dg/tree-ssa/return-value-range-1.c: New test.
>
> diff --git a/gcc/cgraph.cc b/gcc/cgraph.cc
> index e41e5ad3ae7..71dacf23ce1 100644
> --- a/gcc/cgraph.cc
> +++ b/gcc/cgraph.cc
> @@ -2629,6 +2629,54 @@ cgraph_node::set_malloc_flag (bool malloc_p)
>return changed;
>  }
>  
> +/* Worker to set malloc flag.  */
> +static void
> +add_detected_attribute_1 (cgraph_node *node, const char *attr, bool *changed)
> +{
> +  if (!lookup_attribute (attr, DECL_ATTRIBUTES (node->decl)))
> +{
> +  DECL_ATTRIBUTES (node->decl) = tree_cons (get_identifier (attr),
> +  NULL_TREE, DECL_ATTRIBUTES 
> (node->decl));
> +  *changed = true;
> +}
> +
> +  ipa_ref *ref;
> +  FOR_EACH_ALIAS (node, ref)
> +{
> +  cgraph_node *alias = dyn_cast (ref->referring);
> +  if (alias->get_availability () > AVAIL_INTERPOSABLE)
> + add_detected_attribute_1 (alias, attr, changed);
> +}
> +
> +  for (cgraph_edge *e = node->callers; e; e = e->next_caller)
> +if (e->caller->thunk
> + && (e->caller->get_availability () > AVAIL_INTERPOSABLE))
> +  add_detected_attribute_1 (e->caller, attr, changed);
> +}
> +
> +/* Set DECL_IS_MALLOC on NODE's decl and on NODE's aliases if any.  */
> +
> +bool
> +cgraph_node::add_detected_attribute (const char *attr)
> +{
> +  bool changed = false;
> +
> +  if (get_availability () > AVAIL_INTERPOSABLE)
> +add_detected_attribute_1 (this, attr, &changed);
> +  else
> +{
> +  ipa_ref *ref;
> +
> +  FOR_EACH_ALIAS (this, ref)
> + {
> +   cgraph_node *alias = dyn_cast (ref->referring);
> +   if (alias->get_availability () > AVAIL_INTERPOSABLE)
> + add_detected_attribute_1 (alias, attr, &changed);
> + }
> +}
> +  return changed;
> +}
> +
>  /* Worker to set noreturng flag.  */
>  static void
>  set_noreturn_flag_1 (cgraph_node *node, bool noreturn_p, bool *changed)
> diff --git a/gcc/cgraph.h b/gcc/cgraph.h
> index cedaaac3a45..cfdd9f693a8 100644
> --- a/gcc/cgraph.h
> +++ b/gcc/cgraph.h
> @@ -1190,6 +1190,10 @@ struct GTY((tag ("SYMTAB_FUNCTION"))) cgraph_node : 
> public symtab_node
>  
>bool set_pure_flag (bool pure, bool looping);
>  
> +  /* Add attribute ATTR to cgraph_node's decl and on aliases of the node
> + if any.  */
> +  bool add_detected_attribute (const char *attr);
> +
>/* Call callback on function and aliases associated to the function.
>   When INCLUDE_OVERWRITABLE is false, overwritable aliases and thunks are
>   skipped. */
> diff --git a/gcc/common.opt b/gcc/common.opt
> index d21db5d4a20..0be4f02677c 100644
> --- a/gcc/common.opt
> +++ b/gcc/common.opt
> @@ -781,6 +781,10 @@ Wsuggest-attribute=malloc
>  Common Var(warn_suggest_attribute_malloc) Warning
>  Warn about functions which might be candidates for __attribute__((malloc)).
>  
> +Wsuggest-attribute=returns_nonnull

- or _?

(If changing it, needs adjustment in rest of patch too.)

> +Common Var(warn_suggest_attribute_malloc) Warning
> +Warn about functions which might be candidates for __attribute__((malloc)).
> +

Typo: s/malloc/nonnull/?


Re: [PATCH v3 00/11] : More warnings as errors by default

2023-11-27 Thread Sam James


Florian Weimer  writes:

> * Jeff Law:
>
>> On 11/20/23 02:55, Florian Weimer wrote:
>>> This revision addresses Marek's comment about handing
>>> -Wdeclaration-missing-parameter-type properly in conjunction with
>>> -fpermissive.  A new test (permerror-fpermissive-nowarning.c)
>>> demonstrates the expected behavior.  I added a test for -std=gnu89
>>> -fno-permissive, too.
>>> I'm including the precursor cleanup patches in this posting.
>>> Hopefully
>>> this will make the aarch64 tester happy.
>>> Thanks,
>>> Florian
>>> Florian Weimer (11):
>>>aarch64: Avoid -Wincompatible-pointer-types warning in Linux unwinder
>>>aarch64: Call named function in gcc.target/aarch64/aapcs64/ice_1.c
>>>gm2: Add missing declaration of m2pim_M2RTS_Terminate to test
>>>Add tests for validating future C permerrors
>>>c: Turn int-conversion warnings into permerrors
>>>c: Turn -Wimplicit-function-declaration into a permerror
>>>c: Turn -Wimplicit-int into a permerror
>>>c: Do not ignore some forms of -Wimplicit-int in system headers
>>>c: Turn -Wreturn-mismatch into a permerror
>>>c: Turn -Wincompatible-pointer-types into a permerror
>>>c: Add new -Wdeclaration-missing-parameter-type permerror
>
>> The series is fine by me.
>
> Thanks.
>
>> But give Marek additional time to chime in, particularly given the
>> holidays this week in the US.  Say through this time next week?
>
> [...]
>
> I'm also gathering some numbers regarding autoconf impact and potential
> silent miscompilation.

I'd actually forgot about another element here: FreeBSD 14 which was
just released now ships with Clang 16 so we seem to be getting some
activity from them which is a help.

I've resumed our testing for configure diffs and am going to
focus on that for now. It's just laborious because of how many errors
are actually fine.



Re: [PATCH] i386: Fix regression after refactoring legitimize_pe_coff_symbol, ix86_GOT_alias_set and PE_COFF_LEGITIMIZE_EXTERN_DECL

2024-06-27 Thread Sam James
Evgeny Karpov  writes:

> Thank you for reporting the issues and discussing the root causes. 
> It helped in preparing the patch.

Thanks. I'll test it shortly but it looks equivalent to my local
changes, so LGTM.

>
> This patch fixes 3 bugs reported after merging
> the "Add DLL import/export implementation to AArch64" series.
> https://gcc.gnu.org/pipermail/gcc-patches/2024-June/653955.html
> The series refactors the i386 codebase to reuse it in AArch64, which
> triggers some bugs.
>
> Bug 115661 - [15 Regression] wrong code at -O{2,3} on
> x86_64-linux-gnu since r15-1599-g63512c72df09b4
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115661
>
> Bug 115635 - [15 regression] Bootstrap fails with failed
> self-test with the rust fe (diagnostic-path.cc:1153:
> test_empty_path: FAIL: ASSERT_FALSE
> ((path.interprocedural_p ( since r15-1599-g63512c72df09b4
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115635
>
> Issue 1. In some code, i386 has been relying on the
> legitimize_pe_coff_symbol call on all platforms and should return
> NULL_RTX if it is not supported.
>
> Fix: NULL_RTX handling has been added when the target does not
> support PECOFF.
>
> Issue 2. ix86_GOT_alias_set is used on all platforms and cannot be
> extracted to mingw.
>
> Fix: ix86_GOT_alias_set has been returned as it was and is used on
> all platforms for i386.
>
> Bug 115643 - [15 regression] aarch64-w64-mingw32 support today breaks
> x86_64-w64-mingw32 build cannot represent relocation type
> BFD_RELOC_64 since r15-1602-ged20feebd9ea31
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115643
>
> Issue 3. PE_COFF_EXTERN_DECL_SHOULD_BE_LEGITIMIZED has been added and used
> with a negative operator for a complex expression without braces.
>
> Fix: Braces has been added, and
> PE_COFF_EXTERN_DECL_SHOULD_BE_LEGITIMIZED has been renamed to
> PE_COFF_LEGITIMIZE_EXTERN_DECL.
>
>
> The patch has been attached as a text file because it contains special
> characters that are usually removed by the mail client.
>
> Regards,
> Evgeny
>
> [2. 0001-i386-Fix-regression-after-refactoring-legitimize_pe_.txt --- 
> text/plain; 0001-i386-Fix-regression-after-refactoring-legitimize_pe_.txt]...


Re: [PATCH V2] rs6000: Don't pass -many to the assembler [PR112868]

2024-07-11 Thread Sam James
jeevitha  writes:

> Hi All,
>
> The following patch has been bootstrapped and regtested with default 
> configuration
> [--enable-checking=yes] and with --enable-checking=release on 
> powerpc64le-linux.
>
> This patch removes passing the -many assembler option for release builds. Now,
> GCC no longer passes -many under any conditions to the assembler.
>
> This patch exposes the issue with target_powerpc_ppu_ok, which makes a few
> test cases unsupported. Those changes will be in another patch.

For our part, I think we really need PR113652 fixed first, or it'll end up
regressing builds w/ -mcpu=7450. Other than that, we hit no issues in
our testing downstream in Gentoo.

>
> 2024-07-11  Jeevitha Palanisamy  
>
> gcc/
>   PR target/112868
>   * config/rs6000/rs6000.h (ASM_OPT_ANY): Removed Define.
>   (ASM_CPU_SPEC): Remove ASM_OPT_ANY usage.
>
> diff --git a/gcc/config/rs6000/rs6000.h b/gcc/config/rs6000/rs6000.h
> index 9211f91740a..a5bd8e461a0 100644
> --- a/gcc/config/rs6000/rs6000.h
> +++ b/gcc/config/rs6000/rs6000.h
> @@ -94,12 +94,6 @@
>"%{mdejagnu-*: % SUBTARGET_DRIVER_SELF_SPECS
>  
> -#if CHECKING_P
> -#define ASM_OPT_ANY ""
> -#else
> -#define ASM_OPT_ANY " -many"
> -#endif
> -
>  /* Common ASM definitions used by ASM_SPEC among the various targets for
> handling -mcpu=xxx switches.  There is a parallel list in 
> driver-rs6000.cc to
> provide the default assembler options if the user uses -mcpu=native, so if
> @@ -166,8 +160,7 @@
>   mvsx: -mpower7; \
>   mpowerpc64: -mppc64;: %(asm_default)}; \
>:%eMissing -mcpu option in ASM_CPU_SPEC?\n} \
> -%{mvsx: -mvsx -maltivec; maltivec: -maltivec}" \
> -ASM_OPT_ANY
> +%{mvsx: -mvsx -maltivec; maltivec: -maltivec}"
>  
>  #define CPP_DEFAULT_SPEC ""
>  


signature.asc
Description: PGP signature


Re: [avr,patch,applied] testsuite - Add noipa function attribute to noclone functions.

2024-07-16 Thread Sam James
Georg-Johann Lay  writes:

> Applied as obvious.
>
> Johann

I think you can change all of those to just noipa which implies noclone
and noinline.


[PATCH] testsuite: Add dg-do run to more tests

2024-07-17 Thread Sam James
All of these are for wrong-code bugs. Confirmed to be used before but
with no execution.

2024-07-17  Sam James 

PR/96369
PR/102124
PR/108692
* c-c++-common/pr96369.c: Add dg-do run directive.
* gcc.dg/torture/pr102124.c: Ditto.
* gcc.dg/pr108692.c: Ditto.
---
Inspired by Jakub's fixes earlier for other missing dg-run tests which clearly
needed execution.

Please commit if OK. Thanks.

 gcc/testsuite/c-c++-common/pr96369.c| 1 +
 gcc/testsuite/gcc.dg/pr108692.c | 1 +
 gcc/testsuite/gcc.dg/torture/pr102124.c | 1 +
 3 files changed, 3 insertions(+)

diff --git a/gcc/testsuite/c-c++-common/pr96369.c 
b/gcc/testsuite/c-c++-common/pr96369.c
index 8c468d9fec2f..9cb5aaa50420 100644
--- a/gcc/testsuite/c-c++-common/pr96369.c
+++ b/gcc/testsuite/c-c++-common/pr96369.c
@@ -1,4 +1,5 @@
 /* { dg-do compile } */
+/* { dg-do run } */
 /* { dg-options "-O" } */
 
 int main()
diff --git a/gcc/testsuite/gcc.dg/pr108692.c b/gcc/testsuite/gcc.dg/pr108692.c
index fc25bf54e45d..e5f8af5669bb 100644
--- a/gcc/testsuite/gcc.dg/pr108692.c
+++ b/gcc/testsuite/gcc.dg/pr108692.c
@@ -1,5 +1,6 @@
 /* PR tree-optimization/108692 */
 /* { dg-do compile } */
+/* { dg-do run } */
 /* { dg-options "-O2 -ftree-vectorize" } */
 
 __attribute__((noipa)) int
diff --git a/gcc/testsuite/gcc.dg/torture/pr102124.c 
b/gcc/testsuite/gcc.dg/torture/pr102124.c
index a158b4a60b69..a0eb01521242 100644
--- a/gcc/testsuite/gcc.dg/torture/pr102124.c
+++ b/gcc/testsuite/gcc.dg/torture/pr102124.c
@@ -1,4 +1,5 @@
 /* PR tree-optimization/102124 */
+/* { dg-do run } */
 
 int
 foo (const unsigned char *a, const unsigned char *b, unsigned long len)

-- 
2.45.2



[PATCH v2] testsuite: Add dg-do run to more tests

2024-07-17 Thread Sam James
All of these are for wrong-code bugs. Confirmed to be used before but
with no execution.

Tested on x86_64-pc-linux-gnu and checked test logs before/after.

2024-07-17  Sam James  

PR/96369
PR/102124
PR/108692
* c-c++-common/pr96369.c: Add dg-do run directive.
* gcc.dg/torture/pr102124.c: Ditto.
* gcc.dg/pr108692.c: Ditto.
---
v2: Fix ChangeLog format. Explicitly state how tested in commit msg.
Drop redundant dg-do compile where appropriate.

 gcc/testsuite/c-c++-common/pr96369.c| 2 +-
 gcc/testsuite/gcc.dg/pr108692.c | 2 +-
 gcc/testsuite/gcc.dg/torture/pr102124.c | 1 +
 3 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/gcc/testsuite/c-c++-common/pr96369.c 
b/gcc/testsuite/c-c++-common/pr96369.c
index 8c468d9fec2f..ec58a3fc6c92 100644
--- a/gcc/testsuite/c-c++-common/pr96369.c
+++ b/gcc/testsuite/c-c++-common/pr96369.c
@@ -1,4 +1,4 @@
-/* { dg-do compile } */
+/* { dg-do run } */
 /* { dg-options "-O" } */
 
 int main()
diff --git a/gcc/testsuite/gcc.dg/pr108692.c b/gcc/testsuite/gcc.dg/pr108692.c
index fc25bf54e45d..13a27496ad9f 100644
--- a/gcc/testsuite/gcc.dg/pr108692.c
+++ b/gcc/testsuite/gcc.dg/pr108692.c
@@ -1,5 +1,5 @@
 /* PR tree-optimization/108692 */
-/* { dg-do compile } */
+/* { dg-do run } */
 /* { dg-options "-O2 -ftree-vectorize" } */
 
 __attribute__((noipa)) int
diff --git a/gcc/testsuite/gcc.dg/torture/pr102124.c 
b/gcc/testsuite/gcc.dg/torture/pr102124.c
index a158b4a60b69..a0eb01521242 100644
--- a/gcc/testsuite/gcc.dg/torture/pr102124.c
+++ b/gcc/testsuite/gcc.dg/torture/pr102124.c
@@ -1,4 +1,5 @@
 /* PR tree-optimization/102124 */
+/* { dg-do run } */
 
 int
 foo (const unsigned char *a, const unsigned char *b, unsigned long len)

-- 
2.45.2



[PATCH v3] testsuite: Add dg-do run to more tests

2024-07-17 Thread Sam James
All of these are for wrong-code bugs. Confirmed to be used before but
with no execution.

Tested on x86_64-pc-linux-gnu and checked test logs before/after.

2024-07-18  Sam James  

PR c++/53288
PR c++/57437
PR c/65345
PR libstdc++/88101
PR tree-optimization/96369
PR tree-optimization/102124
PR tree-optimization/108692
* c-c++-common/pr96369.c: Add dg-do run directive.
* gcc.dg/torture/pr102124.c: Ditto.
* gcc.dg/pr108692.c: Ditto.
* gcc.dg/atomic/pr65345-4.c: Ditto.
* g++.dg/cpp0x/lambda/lambda-return1.C: Ditto.
* g++.dg/init/lifetime4.C: Ditto.
* g++.dg/torture/builtin-clear-padding-1.C: Ditto.
* g++.dg/torture/builtin-clear-padding-2.C: Ditto.
* g++.dg/torture/builtin-clear-padding-3.C: Ditto.
* g++.dg/torture/builtin-clear-padding-4.C: Ditto.
* g++.dg/torture/builtin-clear-padding-5.C: Ditto.
---
v3: Add a few other tests I noticed. Tweak ChangeLog format as adding
more PRs started to upset `git gcc-verify`.
v2: Fix ChangeLog format. Explicitly state how tested in commit msg.
Drop redundant dg-do compile where appropriate.

 gcc/testsuite/c-c++-common/pr96369.c   | 2 +-
 gcc/testsuite/g++.dg/cpp0x/lambda/lambda-return1.C | 1 +
 gcc/testsuite/g++.dg/init/lifetime4.C  | 2 +-
 gcc/testsuite/g++.dg/torture/builtin-clear-padding-1.C | 1 +
 gcc/testsuite/g++.dg/torture/builtin-clear-padding-2.C | 1 +
 gcc/testsuite/g++.dg/torture/builtin-clear-padding-3.C | 1 +
 gcc/testsuite/g++.dg/torture/builtin-clear-padding-4.C | 1 +
 gcc/testsuite/g++.dg/torture/builtin-clear-padding-5.C | 1 +
 gcc/testsuite/gcc.dg/atomic/pr65345-4.c| 1 +
 gcc/testsuite/gcc.dg/pr108692.c| 2 +-
 gcc/testsuite/gcc.dg/torture/pr102124.c| 1 +
 11 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/gcc/testsuite/c-c++-common/pr96369.c 
b/gcc/testsuite/c-c++-common/pr96369.c
index 8c468d9fec2f..ec58a3fc6c92 100644
--- a/gcc/testsuite/c-c++-common/pr96369.c
+++ b/gcc/testsuite/c-c++-common/pr96369.c
@@ -1,4 +1,4 @@
-/* { dg-do compile } */
+/* { dg-do run } */
 /* { dg-options "-O" } */
 
 int main()
diff --git a/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-return1.C 
b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-return1.C
index 4b353b64c37e..df533e9a87cc 100644
--- a/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-return1.C
+++ b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-return1.C
@@ -1,4 +1,5 @@
 // PR c++/57437
+// { dg-do run } */
 // { dg-require-effective-target c++11 }
 
 struct A {
diff --git a/gcc/testsuite/g++.dg/init/lifetime4.C 
b/gcc/testsuite/g++.dg/init/lifetime4.C
index 4106af7070cc..3e4825fff52f 100644
--- a/gcc/testsuite/g++.dg/init/lifetime4.C
+++ b/gcc/testsuite/g++.dg/init/lifetime4.C
@@ -1,5 +1,5 @@
 // PR c++/53288
-// { dg-do compile { target c++11 } }
+// { dg-do run { target c++11 } }
 
 struct B {
B(int data) : _data(data) { }
diff --git a/gcc/testsuite/g++.dg/torture/builtin-clear-padding-1.C 
b/gcc/testsuite/g++.dg/torture/builtin-clear-padding-1.C
index 625a047ab1c7..f62dedc6fa6d 100644
--- a/gcc/testsuite/g++.dg/torture/builtin-clear-padding-1.C
+++ b/gcc/testsuite/g++.dg/torture/builtin-clear-padding-1.C
@@ -1,4 +1,5 @@
 /* PR libstdc++/88101 */
+/* { dg-do run } */
 
 struct S {} s1, s2;
 struct T : public S { char a; short b; char c; } t1, t2;
diff --git a/gcc/testsuite/g++.dg/torture/builtin-clear-padding-2.C 
b/gcc/testsuite/g++.dg/torture/builtin-clear-padding-2.C
index 19cc78f66104..3cb55cff8d3e 100644
--- a/gcc/testsuite/g++.dg/torture/builtin-clear-padding-2.C
+++ b/gcc/testsuite/g++.dg/torture/builtin-clear-padding-2.C
@@ -1,4 +1,5 @@
 /* PR libstdc++/88101 */
+/* { dg-do run } */
 
 #include 
 
diff --git a/gcc/testsuite/g++.dg/torture/builtin-clear-padding-3.C 
b/gcc/testsuite/g++.dg/torture/builtin-clear-padding-3.C
index d528196bf2dc..fe81e095e082 100644
--- a/gcc/testsuite/g++.dg/torture/builtin-clear-padding-3.C
+++ b/gcc/testsuite/g++.dg/torture/builtin-clear-padding-3.C
@@ -1,4 +1,5 @@
 /* PR libstdc++/88101 */
+/* { dg-do run } */
 
 struct D { int a; int : 24; int b : 8; };
 struct E {};
diff --git a/gcc/testsuite/g++.dg/torture/builtin-clear-padding-4.C 
b/gcc/testsuite/g++.dg/torture/builtin-clear-padding-4.C
index 5936cdf876b2..88bd6bac65ec 100644
--- a/gcc/testsuite/g++.dg/torture/builtin-clear-padding-4.C
+++ b/gcc/testsuite/g++.dg/torture/builtin-clear-padding-4.C
@@ -1,4 +1,5 @@
 // PR middle-end/101586
+// { dg-do run }
 
 struct A { char a; };
 struct B : virtual A {};
diff --git a/gcc/testsuite/g++.dg/torture/builtin-clear-padding-5.C 
b/gcc/testsuite/g++.dg/torture/builtin-clear-padding-5.C
index b5f019147816..0795011077aa 100644
--- a/gcc/testsuite/g++.dg/torture/builtin-clear-padding-5.C
+++ b/gcc/testsuite/g++.dg/torture/builtin-clear-padding-5.C
@@ -1,4 +1,5 @@
 // PR tree-optimization/102586
+// { dg-do run }
 // { dg-opt

[PATCH] testsuite: Add dg-do run to even more tests, fix typo

2024-07-18 Thread Sam James
All of these are for wrong-code bugs. Confirmed to be used before but
with no execution.

Tested on x86_64-pc-linux-gnu and checked test logs before/after.

2024-07-18  Sam James  

PR target/7559
PR c++/9704
PR c++/16115
PR c++/19317
PR rtl-optimization/11536
PR target/20322
PR tree-optimization/31966
PR rtl-optimization/41033
PR tree-optimization/67947
* g++.dg/cpp1z/byte1.C: Add dg-do run directive.
* g++.dg/init/call1.C: Ditto.
* g++.dg/init/copy5.C: Ditto.
* g++.dg/opt/nrv9.C: Ditto.
* gcc.dg/20021006-1.c: Ditto.
* gcc.dg/20030721-1.c: Ditto.
* gcc.dg/20050307-1.c: Ditto.
* gcc.dg/pr41033.c: Ditto.
* gcc.dg/torture/pr67947.c: Ditto.
* gcc.dg/tree-ssa/pr31966.c: Ditto.
* gcc.dg/tree-ssa/tailcall-3.c: Ditto.
* gcc.dg/tree-ssa/vrp74.c: Ditto.
* gcc.target/nvptx/abort.c: Fix whitespace in dg directive.
---
 gcc/testsuite/g++.dg/cpp1z/byte1.C | 2 +-
 gcc/testsuite/g++.dg/init/call1.C  | 1 +
 gcc/testsuite/g++.dg/init/copy5.C  | 1 +
 gcc/testsuite/g++.dg/opt/nrv9.C| 1 +
 gcc/testsuite/gcc.dg/20021006-1.c  | 1 +
 gcc/testsuite/gcc.dg/20030721-1.c  | 3 ++-
 gcc/testsuite/gcc.dg/20050307-1.c  | 1 +
 gcc/testsuite/gcc.dg/pr41033.c | 3 ++-
 gcc/testsuite/gcc.dg/torture/pr67947.c | 1 +
 gcc/testsuite/gcc.dg/tree-ssa/pr31966.c| 1 +
 gcc/testsuite/gcc.dg/tree-ssa/tailcall-3.c | 2 +-
 gcc/testsuite/gcc.dg/tree-ssa/vrp74.c  | 1 +
 gcc/testsuite/gcc.target/nvptx/abort.c | 2 +-
 13 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/gcc/testsuite/g++.dg/cpp1z/byte1.C 
b/gcc/testsuite/g++.dg/cpp1z/byte1.C
index 1ee5f7cacfc9..9ba2268d755e 100644
--- a/gcc/testsuite/g++.dg/cpp1z/byte1.C
+++ b/gcc/testsuite/g++.dg/cpp1z/byte1.C
@@ -1,5 +1,5 @@
 // Test for std::byte aliasing properties.
-// { dg-do compile { target c++17 } }
+// { dg-do run { target c++17 } }
 // { dg-options "-O3" }
 
 #include 
diff --git a/gcc/testsuite/g++.dg/init/call1.C 
b/gcc/testsuite/g++.dg/init/call1.C
index d44b6dddc953..548d59cc80f4 100644
--- a/gcc/testsuite/g++.dg/init/call1.C
+++ b/gcc/testsuite/g++.dg/init/call1.C
@@ -1,4 +1,5 @@
 // Bug c++/16115
+// { dg-do run }
 // { dg-options "-O2" }
 
 extern "C" void abort(); 
diff --git a/gcc/testsuite/g++.dg/init/copy5.C 
b/gcc/testsuite/g++.dg/init/copy5.C
index cef5a2950ef1..26e3bf81d83f 100644
--- a/gcc/testsuite/g++.dg/init/copy5.C
+++ b/gcc/testsuite/g++.dg/init/copy5.C
@@ -1,3 +1,4 @@
+// { dg-do run }
 // { dg-options "-O2" }
 
 struct BOOL {
diff --git a/gcc/testsuite/g++.dg/opt/nrv9.C b/gcc/testsuite/g++.dg/opt/nrv9.C
index 462506867d43..08bcde8827dd 100644
--- a/gcc/testsuite/g++.dg/opt/nrv9.C
+++ b/gcc/testsuite/g++.dg/opt/nrv9.C
@@ -1,4 +1,5 @@
 // PR c++/19317
+// { dg-do run }
 // If we do both NRV and caller-side return slot opt for ga = f()
 // constructing la sets ga.i to 0 too soon.
 
diff --git a/gcc/testsuite/gcc.dg/20021006-1.c 
b/gcc/testsuite/gcc.dg/20021006-1.c
index 92df2c57f6ef..7904b9f99626 100644
--- a/gcc/testsuite/gcc.dg/20021006-1.c
+++ b/gcc/testsuite/gcc.dg/20021006-1.c
@@ -1,6 +1,7 @@
 /* PR target/7559
This testcase was miscompiled on x86-64 due to wrong access to the struct
members.  */
+/* { dg-do run } */
 
 extern void abort(void);
 
diff --git a/gcc/testsuite/gcc.dg/20030721-1.c 
b/gcc/testsuite/gcc.dg/20030721-1.c
index 5e8ed0b434ad..52be42fc59df 100644
--- a/gcc/testsuite/gcc.dg/20030721-1.c
+++ b/gcc/testsuite/gcc.dg/20030721-1.c
@@ -1,5 +1,6 @@
-/* { dg-options "-O2" } */
 /* PR optimization/11536 */
+/* { dg-do run } */
+/* { dg-options "-O2" } */
 /* Origin: sa...@kam.mff.cuni.cz  */
 /* Testcase by Andrew Pinski  */
 
diff --git a/gcc/testsuite/gcc.dg/20050307-1.c 
b/gcc/testsuite/gcc.dg/20050307-1.c
index 0e8dac69a65f..b370a571acac 100644
--- a/gcc/testsuite/gcc.dg/20050307-1.c
+++ b/gcc/testsuite/gcc.dg/20050307-1.c
@@ -1,4 +1,5 @@
 /* PR target/20322 */
+/* { dg-do run } */
 
 extern void abort (void);
 
diff --git a/gcc/testsuite/gcc.dg/pr41033.c b/gcc/testsuite/gcc.dg/pr41033.c
index 5043be2d1191..4c1863c750aa 100644
--- a/gcc/testsuite/gcc.dg/pr41033.c
+++ b/gcc/testsuite/gcc.dg/pr41033.c
@@ -1,5 +1,6 @@
-/* { dg-options "-O1 -fno-strict-aliasing" } */
 /* PR rtl-optimization/41033 */
+/* { dg-do run } */
+/* { dg-options "-O1 -fno-strict-aliasing" } */
 
 struct X {
   int i;
diff --git a/gcc/testsuite/gcc.dg/torture/pr67947.c 
b/gcc/testsuite/gcc.dg/torture/pr67947.c
index 5664c48390af..368a8b20cbf0 100644
--- a/gcc/testsuite/gcc.dg/torture/pr67947.c
+++ b/gcc/testsuite/gcc.dg/torture/pr67947.c
@@ -1,3 +1,4 @@
+/* { dg-do run } */
 /* { dg-additional-options "-O3" } */
 
 #include 
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr31966.c 
b/gcc/testsuite/gcc.dg/tree-ssa/pr31966.c
i

[PATCH] testsuite: powerpc: fix dg-do run typo

2024-07-18 Thread Sam James
'dg-run' is not a valid dejagnu directive, 'dg-do run' is needed here
for the test to be executed.

2024-07-18  Sam James  

PR target/108699
* gcc.target/powerpc/pr108699.c: Fix 'dg-run' typo.
---
Kewen, could you check this on powerpc to ensure it doesn't execute beforehand
and now it does? I could do it on powerpc but I don't have anything setup
right now.

 gcc/testsuite/gcc.target/powerpc/pr108699.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/testsuite/gcc.target/powerpc/pr108699.c 
b/gcc/testsuite/gcc.target/powerpc/pr108699.c
index f02bac130cc7..beb8b601fd51 100644
--- a/gcc/testsuite/gcc.target/powerpc/pr108699.c
+++ b/gcc/testsuite/gcc.target/powerpc/pr108699.c
@@ -1,4 +1,4 @@
-/* { dg-run } */
+/* { dg-do run } */
 /* { dg-options "-O2 -ftree-vectorize -fno-vect-cost-model" } */
 
 #define N 16

-- 
2.45.2



Re: [PATCH] c-family: Introduce the -Winvalid-noreturn flag from clang with extra tuneability

2024-07-18 Thread Sam James
Julian Waters  writes:

> Darn, not even sending it from my friend's mail account worked to combat the 
> patch formatting issues. Not only is the
> patch a complete wreck on the mailing list archive, but my friend's mail 
> account has now somehow merged into mine on the
> list, resulting in a truly confusing author display name. I really hope 
> you've somehow gotten the patch intact on your
> end...

Just use `git send-email` and nothing else, and make sure to use
plain-text emails when writing anything in your mail client (not HTML).

>
> best regards,
> Julian
> [...]


Re: [PATCH] testsuite: powerpc: fix dg-do run typo

2024-07-19 Thread Sam James
"Kewen.Lin"  writes:

> Hi Sam,

Hi Kewen,

>
> on 2024/7/19 11:28, Sam James wrote:
>> 'dg-run' is not a valid dejagnu directive, 'dg-do run' is needed here
>> for the test to be executed.
>> 
>> 2024-07-18  Sam James  
>> 
>>  PR target/108699
>>  * gcc.target/powerpc/pr108699.c: Fix 'dg-run' typo.
>> ---
>> Kewen, could you check this on powerpc to ensure it doesn't execute 
>> beforehand
>> and now it does? I could do it on powerpc but I don't have anything setup
>> right now.
>
> Oops, thanks for catching and fixing this stupid typo!  Yes, I just confirmed 
> that,
> w/ this fix pr108699.exe gets generated and executed (# of expected passes is 
> changed
> from 1 to 2).

Many thanks! Could you push for me please?

>
> BR,
> Kewen

best,
sam

>
>> 
>>  gcc/testsuite/gcc.target/powerpc/pr108699.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/gcc/testsuite/gcc.target/powerpc/pr108699.c 
>> b/gcc/testsuite/gcc.target/powerpc/pr108699.c
>> index f02bac130cc7..beb8b601fd51 100644
>> --- a/gcc/testsuite/gcc.target/powerpc/pr108699.c
>> +++ b/gcc/testsuite/gcc.target/powerpc/pr108699.c
>> @@ -1,4 +1,4 @@
>> -/* { dg-run } */
>> +/* { dg-do run } */
>>  /* { dg-options "-O2 -ftree-vectorize -fno-vect-cost-model" } */
>>  
>>  #define N 16
>> 


[PATCH] gcc: stop adding -fno-common for checking builds

2024-07-19 Thread Sam James
Originally added in r0-44646-g204250d2fcd084 and r0-44627-gfd350d241fecf6 whic
moved -fno-common from all builds to just checking builds.

Since r10-4867-g6271dd984d7f92, GCC defaults to -fno-common. There's no need
to pass it specially for checking builds.

We could keep it for older bootstrap compilers with checking but I don't see
much value in that, it was already just a bonus before.

gcc/ChangeLog:
* Makefile.in (NOCOMMON_FLAG): Delete.
(GCC_WARN_CFLAGS): Drop NOCOMMON_FLAG.
(GCC_WARN_CXXFLAGS): Drop NOCOMMON_FLAG.
* configure.ac: Ditto.
* configure: Regenerate.

gcc/d/ChangeLog:
* Make-lang.in (WARN_DFLAGS): Drop NOCOMMON_FLAG.
---
This came out of a discussion with pinskia last year but I punted it
until stage1. Been running with it since then.

 gcc/Makefile.in| 8 ++--
 gcc/configure  | 8 ++--
 gcc/configure.ac   | 3 ---
 gcc/d/Make-lang.in | 2 +-
 4 files changed, 5 insertions(+), 16 deletions(-)

diff --git a/gcc/Makefile.in b/gcc/Makefile.in
index f4bb4a88cf31..4fc86ed7938b 100644
--- a/gcc/Makefile.in
+++ b/gcc/Makefile.in
@@ -185,10 +185,6 @@ C_LOOSE_WARN = @c_loose_warn@
 STRICT_WARN = @strict_warn@
 C_STRICT_WARN = @c_strict_warn@
 
-# This is set by --enable-checking.  The idea is to catch forgotten
-# "extern" tags in header files.
-NOCOMMON_FLAG = @nocommon_flag@
-
 NOEXCEPTION_FLAGS = @noexception_flags@
 
 ALIASING_FLAGS = @aliasing_flags@
@@ -215,8 +211,8 @@ VALGRIND_DRIVER_DEFINES = @valgrind_path_defines@
 .-warn = $(STRICT_WARN)
 build-warn = $(STRICT_WARN)
 rtl-ssa-warn = $(STRICT_WARN)
-GCC_WARN_CFLAGS = $(LOOSE_WARN) $(C_LOOSE_WARN) $($(@D)-warn) $(if 
$(filter-out $(STRICT_WARN),$($(@D)-warn)),,$(C_STRICT_WARN)) $(NOCOMMON_FLAG) 
$($@-warn)
-GCC_WARN_CXXFLAGS = $(LOOSE_WARN) $($(@D)-warn) $(NOCOMMON_FLAG) $($@-warn)
+GCC_WARN_CFLAGS = $(LOOSE_WARN) $(C_LOOSE_WARN) $($(@D)-warn) $(if 
$(filter-out $(STRICT_WARN),$($(@D)-warn)),,$(C_STRICT_WARN)) $($@-warn)
+GCC_WARN_CXXFLAGS = $(LOOSE_WARN) $($(@D)-warn) $($@-warn)
 
 # 1 2 3 ... 
 one_to__0:=1 2 3 4 5 6 7 8 9
diff --git a/gcc/configure b/gcc/configure
index 4faae0fa5fb8..01acca7fb5cc 100755
--- a/gcc/configure
+++ b/gcc/configure
@@ -862,7 +862,6 @@ valgrind_command
 valgrind_path_defines
 valgrind_path
 TREECHECKING
-nocommon_flag
 noexception_flags
 warn_cxxflags
 warn_cflags
@@ -7605,17 +7604,14 @@ do
 done
 IFS="$ac_save_IFS"
 
-nocommon_flag=""
 if test x$ac_checking != x ; then
 
 $as_echo "#define CHECKING_P 1" >>confdefs.h
 
-  nocommon_flag=-fno-common
 else
   $as_echo "#define CHECKING_P 0" >>confdefs.h
 
 fi
-
 if test x$ac_extra_checking != x ; then
 
 $as_echo "#define ENABLE_EXTRA_CHECKING 1" >>confdefs.h
@@ -21410,7 +21406,7 @@ else
   lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
   lt_status=$lt_dlunknown
   cat > conftest.$ac_ext <<_LT_EOF
-#line 21413 "configure"
+#line 21409 "configure"
 #include "confdefs.h"
 
 #if HAVE_DLFCN_H
@@ -21516,7 +21512,7 @@ else
   lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
   lt_status=$lt_dlunknown
   cat > conftest.$ac_ext <<_LT_EOF
-#line 21519 "configure"
+#line 21515 "configure"
 #include "confdefs.h"
 
 #if HAVE_DLFCN_H
diff --git a/gcc/configure.ac b/gcc/configure.ac
index 3da1eaa70646..3f20c107b6aa 100644
--- a/gcc/configure.ac
+++ b/gcc/configure.ac
@@ -697,16 +697,13 @@ do
 done
 IFS="$ac_save_IFS"
 
-nocommon_flag=""
 if test x$ac_checking != x ; then
   AC_DEFINE(CHECKING_P, 1,
 [Define to 0/1 if you want more run-time sanity checks.  This one gets a grab
 bag of miscellaneous but relatively cheap checks.])
-  nocommon_flag=-fno-common
 else
   AC_DEFINE(CHECKING_P, 0)
 fi
-AC_SUBST(nocommon_flag)
 if test x$ac_extra_checking != x ; then
   AC_DEFINE(ENABLE_EXTRA_CHECKING, 1,
 [Define to 0/1 if you want extra run-time checking that might affect code
diff --git a/gcc/d/Make-lang.in b/gcc/d/Make-lang.in
index eaea6e039cf7..077668faae64 100644
--- a/gcc/d/Make-lang.in
+++ b/gcc/d/Make-lang.in
@@ -55,7 +55,7 @@ CHECKING_DFLAGS = -frelease
 else
 CHECKING_DFLAGS =
 endif
-WARN_DFLAGS = -Wall -Wdeprecated $(NOCOMMON_FLAG)
+WARN_DFLAGS = -Wall -Wdeprecated
 
 # D front-end doesn't use exceptions, but it does require RTTI.
 NOEXCEPTION_DFLAGS = $(filter-out -fno-rtti, $(NOEXCEPTION_FLAGS))

-- 
2.45.2



[PATCH] testsuite: fix pr115929-1.c with -Wformat-security

2024-07-19 Thread Sam James
Some distributions like Gentoo make -Wformat and -Wformat-security
enabled by default. Pass -Wno-format to the test to avoid a spurious
fail in such environments.

gcc/testsuite/
PR rtl-optimization/115929
* gcc.dg/torture/pr115929-1.c: Pass -Wno-format.
---
Richard, is this OK? I could adjust the testcase if you prefer.

Please commit on my behalf if it's fine. Thanks.

 gcc/testsuite/gcc.dg/torture/pr115929-1.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/testsuite/gcc.dg/torture/pr115929-1.c 
b/gcc/testsuite/gcc.dg/torture/pr115929-1.c
index 19b831ab99ef..d81081e2f2e9 100644
--- a/gcc/testsuite/gcc.dg/torture/pr115929-1.c
+++ b/gcc/testsuite/gcc.dg/torture/pr115929-1.c
@@ -1,5 +1,5 @@
 /* { dg-require-effective-target lp64 } */
-/* { dg-options "-fno-gcse -fschedule-insns -fno-guess-branch-probability 
-fno-tree-fre -fno-tree-ch" } */
+/* { dg-options "-fno-gcse -fschedule-insns -fno-guess-branch-probability 
-fno-tree-fre -fno-tree-ch -Wno-format" } */
 
 int printf(const char *, ...);
 int a[6], b, c;

-- 
2.45.2



Re: [PATCH] testsuite: fix pr115929-1.c with -Wformat-security

2024-07-19 Thread Sam James
Xi Ruoyao  writes:

> On Sat, 2024-07-20 at 06:52 +0100, Sam James wrote:
>> Some distributions like Gentoo make -Wformat and -Wformat-security
>> enabled by default. Pass -Wno-format to the test to avoid a spurious
>> fail in such environments.
>> 
>> gcc/testsuite/
>>  PR rtl-optimization/115929
>>  * gcc.dg/torture/pr115929-1.c: Pass -Wno-format.
>> ---
>
> IMO if you are patching GCC downstream to enable some options, you can
> patch the test case in the same .patch file anyway instead of pushing it
> upstream.

It's a fair argument.

That said, you did say the same thing reflexively about -fhardened, even
if this is a different situation ;)

One might argue that especially for torture/, supporting "not
unreasonable" random flags is not a bad thing.

>
> If we take the responsibility to make the test suite anticipate random
> downstream changes, the test suite will ended up filled with different
> workarounds for 42 distros.
>

My counterpoint would be that there are certain warnings we know various
distros enable by default, and various warnings where it's not
inconceivable we might do them upstream at some point.

Being loose about conformance in test cases is part of why the C99
enforcement took a while.

> If we have to anticipate downstream changes we should make a policy
> about which changes we must anticipate (hmm and if we'll anticipate -
> Wformat by default why not add a configuration option for it by the
> way?), or do it in a more generic way (using a .spec file to explicitly
> give the "baseline" options for testing?)
>

Anyway, I take the point, but it was cheaper to ask with a patch
attached than have it in my head and not know what the position was.

I like the .spec idea. In the past, we used custom .spec extensively
downstream, and we stopped before my tenure. My intention long-term with
Arsen is to bring it back as it feels like a better fit.

I'm also happy to adjust the testcase given I reproduced the original
issue fine. Or to leave it if the consensus is to. Whatever works for me.

I dare say we're spending time talking about it than the occasional
-Wno- costs though.

>> Richard, is this OK? I could adjust the testcase if you prefer.
>> 
>> [...]

thanks,
sam


signature.asc
Description: PGP signature


[PATCH] modula2: Fully respect DESTDIR in texi rule

2024-05-20 Thread Sam James
This was originally reported in Gentoo at https://bugs.gentoo.org/930014.

2024-05-20  Sam James 
gcc/m2/
* Make-lang.in (m2.install-info): Pass --destdir for dir index.
---
 gcc/m2/Make-lang.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/m2/Make-lang.in b/gcc/m2/Make-lang.in
index 0abd8ce14555..da4226123dff 100644
--- a/gcc/m2/Make-lang.in
+++ b/gcc/m2/Make-lang.in
@@ -425,7 +425,7 @@ m2.install-info: installdirs
else true; fi
-if [ -f gm2$(exeext) ] && [ -f $(DESTDIR)$(infodir)/m2.info ]; then \
  if $(SHELL) -c 'install-info --version' >/dev/null 2>&1; then \
-   install-info --dir-file=$(infodir)/dir 
$(DESTDIR)$(infodir)/m2.info; \
+   install-info --dir-file=$(DESTDIR)$(infodir)/dir 
$(DESTDIR)$(infodir)/m2.info; \
  else true; fi; \
else true; fi
 
-- 
2.45.1



Re: [PATCH] Fix -Wstringop-overflow warning in 23_containers/vector/types/1.cc

2024-05-27 Thread Sam James
François Dumont  writes:

> In C++98 this test fails with:

For this, and your other -Wfree-nonheap-object patches, could you see if
it helps with any of the bugs reported for both -Wstringop-overflow and
-Wfree-nonheap-object in libstdc++? There's a bunch of (possible) dupes
that it'd be worth tagging if any of them are applicable.

>
> Excess errors:
> /home/fdumont/dev/gcc/build/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/stl_algobase.h:452:
> warning: 'void* __builtin_memcpy(void*, const void*, long unsigned
> int)' writing between 2 and 9223372036854775806 bytes into a region of
> size 0 overflows the destination [-Wstringop-overflow=]
>
> The attached patch avoids this warning.
>
>     libstdc++: Fix -Wstringop-overflow warning coming from std::vector
>
>     Make vector<>::_M_range_insert implementation more transparent to
> the compiler checks.
>
>     Extend local copies of members to the whole method scope so that
> all branches benefit
>     from those.
>
>     libstdc++-v3/ChangeLog:
>
>     * include/bits/vector.tcc
>     (std::vector<>::_M_range_insert(iterator, _FwdIt, _FwdIt,
> forward_iterator_tag)):
>     Use local copies of members to call the different algorithms.
>
> Ok to commit if all tests passes ?
>
> François
>
> [2. text/plain; vector_range_insert_patch.txt]...


Re: [PATCH] testsuite: Fix fam-in-union-alone-in-struct-2.c with unsigned char [PR116148]

2024-08-09 Thread Sam James
"Kewen.Lin"  writes:

> Hi,
>
> As PR116148#c7 shows, fam-in-union-alone-in-struct-2.c still
> fails on hppa which is a BE environment, but by checking more
> (also confirmed by John in PR116148#c12), it's due to that
> signedness of plain char on hppa is signed therefore the value
> of with_fam_3_v.a[7] "8f" get sign extended as "ff8f" then
> the verification will fail.  This patch is to change plain char
> with unsigned char to avoid that.
>
> Tested well powerpc64-linux-gnu (BE) and powerpc64le-linux-gnu
> (LE), also confirmed to work on hppa by John.
>
> Is it ok for trunk?
>
> BR,
> Kewen
> -
>   PR testsuite/116148
>
> gcc/testsuite/ChangeLog:
>
>   * c-c++-common/fam-in-union-alone-in-struct-2.c: Change the type of
>   member a[] of union with_fam_3 with unsigned char.
> ---
>  gcc/testsuite/c-c++-common/fam-in-union-alone-in-struct-2.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/gcc/testsuite/c-c++-common/fam-in-union-alone-in-struct-2.c 
> b/gcc/testsuite/c-c++-common/fam-in-union-alone-in-struct-2.c
> index 3743f9e7dac..920b4c4b7a7 100644
> --- a/gcc/testsuite/c-c++-common/fam-in-union-alone-in-struct-2.c
> +++ b/gcc/testsuite/c-c++-common/fam-in-union-alone-in-struct-2.c
> @@ -14,7 +14,7 @@ union with_fam_2 {
>  } with_fam_2_v = {.a = 0x1f2f3f4f};
>
>  union with_fam_3 {
> -  char a[];
> +  unsigned char a[];
>int b[];
>  } with_fam_3_v = {.b = {0x1f2f3f4f, 0x5f6f7f7f}};

LGTM, thank you!


signature.asc
Description: PGP signature


Re: [PATCH] [x86] Mention _Float16 and __bf16 changes in GCC14.

2024-08-12 Thread Sam James
Gerald Pfeifer  writes:

> On Wed, 31 Jul 2024, liuhongt wrote:
>> +   The _Float16 and __bf16 type are supported
>> +independent of SSE2. W/o SSE2, these types are storage-only, compiler 
>> will
>> +issue an error when they're used in conversion, unary operation,
>> +binary operation, parameter passing or value return. 
>
> "types" (plural)
> "independently"
> "Without" (spelt out)
> "the compiler"
>
> And personally I would use an Oxford comma, so "..., or value return".
>
>> +instead of __FLT16_MAX__(or other similar Macros).
>
> "macros" (lowercase)
>
>
> --- a/htdocs/gcc-14/porting_to.html
> +++ b/htdocs/gcc-14/porting_to.html
>
> I don't think we need this in porting_to.html as well; the release notes 
> are sufficient.

Note that Qt at least did have to be ported. It might be worth keeping
it in there. (The bug was prompted by observed breakage.)


Re: [PATCH] ifcvt: Fix force_operand ICE due to noce_convert_multiple_sets [PR116353]

2024-08-13 Thread Sam James
Manolis Tsamis  writes:

> Now that more operations are allowed for noce_convert_multiple_sets, we need 
> to
> check noce_can_force_operand on the sequence before calling 
> try_emit_cmove_seq.
> Otherwise an inappropriate argument may be given to copy_to_mode_reg and 
> result
> in an ICE.
>
> Fix-up for the recent ifcvt commit 72c9b5f438f22cca493b4e2a8a2a31ff61bf1477

Thanks! Bootstrapped & tested with all FEs with no regressions on amd64.

>
>   PR tree-optimization/116353
>
> gcc/ChangeLog:
>
>   * ifcvt.cc (bb_ok_for_noce_convert_multiple_sets): Check
>   noce_can_force_operand.
>
> gcc/testsuite/ChangeLog:
>
>   * gcc.target/i386/pr116353.c: New test.
>
> Tested-by: Christoph Müllner 
> Signed-off-by: Manolis Tsamis 
> ---

sam


signature.asc
Description: PGP signature


Re: [PATCH v2] genoutput: Accelerate the place_operands function.

2024-08-13 Thread Sam James
Xianmiao Qu  writes:

> On Wed, Aug 14, 2024 at 01:01:35AM +0800, Xianmiao Qu wrote:
>>  static void scan_operands (class data *, rtx, int, int);
>> -static int compare_operands (struct operand_data *,
>> - struct operand_data *);
>>  static void place_operands (class data *);
>
> Oh, there is an mistake here. I shouldn't have deleted these two lines of 
> code. Sorry!

(Mind sending a v3?)

>
>
> BR,
> Xianmiao


Re: Ping: [PATCH] testsuite: Fix struct size check [PR116155]

2024-08-13 Thread Sam James
Hans-Peter Nilsson  writes:

> I stumbled on this being a regression for cris-elf as well;
> the patch expectedly fixes the test-case for CRIS as well.
> It's been a week since the patch was posted and as I see no
> replies, I'm pinging this in behalf of Dimitar.

I can't formally approve it but I think we should commit it. it's
obviously right, the test author suggested it, and it wasn't being run
until recently (i.e. we fully understand why this is only appearing
now).

thanks.

>
>> From: Dimitar Dimitrov 
>> Date: Mon,  5 Aug 2024 21:29:35 +0300
>
>> The size of "struct only_fam_2" is dependent on the alignment of the
>> flexible array member "b", and not on the type of the preceding
>> bit-fields.  For most targets the two are equal.  But on default_packed
>> targets like pru-unknown-elf, the alignment of int is not equal to the
>> size of int, so the test failed.
>> 
>> Patch was suggested by Qing Zhao.  Tested on pru-unknown-elf and
>> x86_64-pc-linux-gnu.
>> 
>> Ok for master?
>> 
>>  PR testsuite/116155
>> 
>> gcc/testsuite/ChangeLog:
>> 
>>  * c-c++-common/fam-in-union-alone-in-struct-1.c: Adjust
>>  check to account for default_packed targets.
>> 
>> Signed-off-by: Dimitar Dimitrov 
>> ---
>>  gcc/testsuite/c-c++-common/fam-in-union-alone-in-struct-1.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/gcc/testsuite/c-c++-common/fam-in-union-alone-in-struct-1.c 
>> b/gcc/testsuite/c-c++-common/fam-in-union-alone-in-struct-1.c
>> index 39ebf17850b..9979e96fe70 100644
>> --- a/gcc/testsuite/c-c++-common/fam-in-union-alone-in-struct-1.c
>> +++ b/gcc/testsuite/c-c++-common/fam-in-union-alone-in-struct-1.c
>> @@ -45,7 +45,7 @@ int main ()
>>  __builtin_abort ();
>>if (sizeof (struct only_fam) != 0)
>>  __builtin_abort ();
>> -  if (sizeof (struct only_fam_2) != sizeof (int))
>> +  if (sizeof (struct only_fam_2) != __alignof__ (int))
>>  __builtin_abort ();
>>return 0;
>>  }
>> -- 
>> 2.45.2
>> 


Re: [PATCH v4] genoutput: Accelerate the place_operands function.

2024-08-13 Thread Sam James
Xianmiao Qu  writes:

> With the increase in the number of modes and patterns for some
> backend architectures, the place_operands function becomes a
> bottleneck int the speed of genoutput, and may even become a
> bottleneck int the overall speed of building the GCC project.
> This patch aims to accelerate the place_operands function,
> the optimizations it includes are:
> 1. Use a hash table to store operand information,
>improving the lookup time for the first operand.
> 2. Move mode comparison to the beginning to avoid the scenarios of most 
> strcmp.
>
> I tested the speed improvements for the following backends,
>   Improvement Ratio
> x86_64197.9%
> aarch64   954.5%
> riscv 2578.6%
> If the build machine is slow, then this improvement can save a lot of time.
>
> I tested the genoutput output for x86_64/aarch64/riscv backends,
> and there was no difference compared to before the optimization,
> so this shouldn't introduce any functional issues.
>
> gcc/
>   * genoutput.cc (struct operand_data): Add member 'eq_next' to
>   point to the next member with the same hash value in the
>   hash table.
>   (compare_operands): Move the comparison of the mode to the very
>   beginning to accelerate the comparison of the two operands.
>   (struct operand_data_hasher): New, a class that takes into account
>   the necessary elements for comparing the equality of two operands
>   in its hash value.
>   (operand_data_hasher::hash): New.
>   (operand_data_hasher::equal): New.
>   (operand_datas): New, hash table of konwn pattern operands.
>   (place_operands): Use a hash table instead of traversing the array
>   to find the same operand.
>   (main): Add initialization of the hash table 'operand_datas'.
> ---
>  gcc/genoutput.cc | 105 ++-
>  1 file changed, 85 insertions(+), 20 deletions(-)
>
> diff --git a/gcc/genoutput.cc b/gcc/genoutput.cc
> index efd81766bb5b..cca1b6622d47 100644
> --- a/gcc/genoutput.cc
> +++ b/gcc/genoutput.cc
> @@ -91,6 +91,7 @@ along with GCC; see the file COPYING3.  If not see
>  #include "errors.h"
>  #include "read-md.h"
>  #include "gensupport.h"
> +#include "hash-table.h"
>  
>  /* No instruction can have more operands than this.  Sorry for this
> arbitrary limit, but what machine will have an instruction with
> @@ -112,6 +113,8 @@ static int next_operand_number = 1;
>  struct operand_data
>  {
>struct operand_data *next;
> +  /* Point to the next member with the same hash value in the hash table.  */
> +  struct operand_data *eq_next;
>int index;
>const char *predicate;
>const char *constraint;
> @@ -127,7 +130,7 @@ struct operand_data
>  
>  static struct operand_data null_operand =
>  {
> -  0, 0, "", "", E_VOIDmode, 0, 0, 0, 0, 0
> +  0, 0, 0, "", "", E_VOIDmode, 0, 0, 0, 0, 0
>  };
>  
>  static struct operand_data *odata = &null_operand;
> @@ -532,6 +535,13 @@ compare_operands (struct operand_data *d0, struct 
> operand_data *d1)
>  {

Did the const change Richard suggested not work?

(Also, note that he said it's fine to commit once his suggested changes
are made.)

> [...]

thanks,
samOA


Re: Ping: [PATCH] testsuite: Fix struct size check [PR116155]

2024-08-13 Thread Sam James
Hans-Peter Nilsson  writes:

>> From: Sam James 
>> Date: Tue, 13 Aug 2024 18:17:29 +0100
>
>> Hans-Peter Nilsson  writes:
>> 
>> > I stumbled on this being a regression for cris-elf as well;
>> > the patch expectedly fixes the test-case for CRIS as well.
>> > It's been a week since the patch was posted and as I see no
>> > replies, I'm pinging this in behalf of Dimitar.
>> 
>> I can't formally approve it but I think we should commit it. it's
>> obviously right, the test author suggested it, and it wasn't being run
>> until recently (i.e. we fully understand why this is only appearing
>> now).
>
> Oh!  I see it was part of that dg- fixup!  Yes, agreed:
> given the history I'll commit it as obvious later today.
> I don't mind if someone beats me to the punch.  Thanks.
>

Thanks! I'll have more of those soon, but I wanted to let things settle
down a bit first (trying to keep a close eye on any bugs and
gcc-testresults mails for regressions).

best,
sam

> [...]


signature.asc
Description: PGP signature


Re: [PATCH] ifcvt: Fix force_operand ICE due to noce_convert_multiple_sets [PR116353]

2024-08-13 Thread Sam James
"H.J. Lu"  writes:

> On Tue, Aug 13, 2024 at 4:57 AM Manolis Tsamis  
> wrote:
>>
>> Now that more operations are allowed for noce_convert_multiple_sets, we need 
>> to
>> check noce_can_force_operand on the sequence before calling 
>> try_emit_cmove_seq.
>> Otherwise an inappropriate argument may be given to copy_to_mode_reg and 
>> result
>> in an ICE.
>>
>> Fix-up for the recent ifcvt commit 72c9b5f438f22cca493b4e2a8a2a31ff61bf1477
>>
>> PR tree-optimization/116353
>>
>> gcc/ChangeLog:
>>
>> * ifcvt.cc (bb_ok_for_noce_convert_multiple_sets): Check
>> noce_can_force_operand.
>>
>> gcc/testsuite/ChangeLog:
>>
>> * gcc.target/i386/pr116353.c: New test.
>>
>> Tested-by: Christoph Müllner 
>> Signed-off-by: Manolis Tsamis 
>> ---
>>
>>  gcc/ifcvt.cc |  6 ++-
>>  gcc/testsuite/gcc.target/i386/pr116353.c | 55 
>>  2 files changed, 59 insertions(+), 2 deletions(-)
>>  create mode 100644 gcc/testsuite/gcc.target/i386/pr116353.c
>>
>> diff --git a/gcc/ifcvt.cc b/gcc/ifcvt.cc
>> index 3e25f30b67e..da59c907891 100644
>> --- a/gcc/ifcvt.cc
>> +++ b/gcc/ifcvt.cc
>> @@ -3938,8 +3938,10 @@ bb_ok_for_noce_convert_multiple_sets (basic_block 
>> test_bb, unsigned *cost)
>>rtx src = SET_SRC (set);
>>
>>/* Do not handle anything involving memory loads/stores since it might
>> -violate data-race-freedom guarantees.  */
>> -  if (!REG_P (dest) || contains_mem_rtx_p (src))
>> +violate data-race-freedom guarantees.  Make sure we can force SRC
>> +to a register as that may be needed in try_emit_cmove_seq.  */
>> +  if (!REG_P (dest) || contains_mem_rtx_p (src)
>> + || !noce_can_force_operand (src))
>> return false;
>>
>>/* Destination and source must be appropriate.  */
>> diff --git a/gcc/testsuite/gcc.target/i386/pr116353.c 
>> b/gcc/testsuite/gcc.target/i386/pr116353.c
>> new file mode 100644
>> index 000..8e254653d5d
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/i386/pr116353.c
>
> Does this test contain x86 specific code?
>

Yes, please move it to the generic suite (maybe even torture, as it has
a few nice properties).

>> @@ -0,0 +1,55 @@
>> +/* PR tree-optimization/116353 */
>> +/* { dg-do compile } */
>> +/* { dg-options "-O2" } */
>> +
>> +enum desmode { C };
>> +struct {
>> +  unsigned char des_ivec[];
>> +} _des_crypt_desp;
>> +int des_SPtrans_6_0, des_SPtrans_4_0, des_encrypt_encrypt, des_encrypt_i;
>> +long des_encrypt_s_0, _des_crypt_tin1, _des_crypt_tout0, _des_crypt_tout1,
>> +_des_crypt_tin0;
>> +enum desmode _des_crypt_desp_0;
>> +unsigned long _des_crypt_tbuf[2];
>> +char _des_crypt_out;
>> +void des_encrypt(unsigned long *buf) {
>> +  long l, r, t;
>> +  l = buf[0];
>> +  r = buf[1];
>> +  t = r;
>> +  r ^= l ^= t < 6;
>> +  if (des_encrypt_encrypt)
>> +for (;; des_encrypt_i += 4)
>> +  des_encrypt_s_0 ^= des_SPtrans_4_0 | des_SPtrans_6_0;
>> +  buf[1] = r;
>> +}
>> +void _des_crypt() {
>> +  long xor0, xor1;
>> +  unsigned char *in;
>> +  int cbc_mode = _des_crypt_desp_0;
>> +  in = _des_crypt_desp.des_ivec;
>> +  xor0 = xor1 = 0;
>> +  for (;;) {
>> +_des_crypt_tin0 = *in++;
>> +_des_crypt_tin0 |= *in++ << 8;
>> +_des_crypt_tin0 |= *in++ << 16;
>> +_des_crypt_tin0 |= (long)*in << 24;
>> +_des_crypt_tin1 = *in++;
>> +_des_crypt_tin1 |= *in++ << 8;
>> +_des_crypt_tin1 |= *in++ << 16;
>> +_des_crypt_tin1 |= (long)*in << 24;
>> +_des_crypt_tbuf[0] = _des_crypt_tin0;
>> +_des_crypt_tbuf[1] = _des_crypt_tin1;
>> +des_encrypt(_des_crypt_tbuf);
>> +if (cbc_mode) {
>> +  _des_crypt_tout0 = xor0;
>> +  _des_crypt_tout1 = _des_crypt_tbuf[1] ^ xor1;
>> +  xor0 = _des_crypt_tin0;
>> +  xor1 = _des_crypt_tin1;
>> +} else {
>> +  _des_crypt_tout0 = _des_crypt_tbuf[0];
>> +  _des_crypt_tout1 = _des_crypt_tbuf[1];
>> +}
>> +_des_crypt_out = _des_crypt_tout0 * _des_crypt_tout1;
>> +  }
>> +}
>> --
>> 2.34.1
>>


signature.asc
Description: PGP signature


Re: [PATCH v5] genoutput: Accelerate the place_operands function.

2024-08-13 Thread Sam James
Xianmiao Qu  writes:

> With the increase in the number of modes and patterns for some
> backend architectures, the place_operands function becomes a
> bottleneck int the speed of genoutput, and may even become a
> bottleneck int the overall speed of building the GCC project.
> This patch aims to accelerate the place_operands function,
> the optimizations it includes are:
> 1. Use a hash table to store operand information,
>improving the lookup time for the first operand.
> 2. Move mode comparison to the beginning to avoid the scenarios of most 
> strcmp.
>
> I tested the speed improvements for the following backends,
>   Improvement Ratio
> x86_64197.9%
> aarch64   954.5%
> riscv 2578.6%
> If the build machine is slow, then this improvement can save a lot of time.
>
> I tested the genoutput output for x86_64/aarch64/riscv backends,
> and there was no difference compared to before the optimization,
> so this shouldn't introduce any functional issues.
>
> gcc/
>   * genoutput.cc (struct operand_data): Add member 'eq_next' to
>   point to the next member with the same hash value in the
>   hash table.
>   (compare_operands): Move the comparison of the mode to the very
>   beginning to accelerate the comparison of the two operands.
>   (struct operand_data_hasher): New, a class that takes into account
>   the necessary elements for comparing the equality of two operands
>   in its hash value.
>   (operand_data_hasher::hash): New.
>   (operand_data_hasher::equal): New.
>   (operand_datas): New, hash table of konwn pattern operands.
>   (place_operands): Use a hash table instead of traversing the array
>   to find the same operand.
>   (main): Add initialization of the hash table 'operand_datas'.

LGTM.

thanks,
sam


signature.asc
Description: PGP signature


[PATCH] ltmain.sh: allow more flags at link-time

2024-08-14 Thread Sam James
libtool defaults to filtering flags passed at link-time.

This brings the filtering in GCC's 'fork' of libtool into sync with
upstream libtool commit 22a7e547e9857fc94fe5bc7c921d9a4b49c09f8e.

In particular, this now allows some harmless diagnostic flags (especially
useful for things like -Werror=odr), more optimization flags, and some
Clang-specific options.

GCC's -flto documentation mentions:
> To use the link-time optimizer, -flto and optimization options should be
> specified at compile time and during the final link. It is recommended
> that you compile all the files participating in the same link with the
> same options and also specify those options at link time.

This allows compliance with that.

* ltmain.sh (func_mode_link): Allow various flags through filter.
---
We have been using this for a while now downstream.

H.J., please take a look.

I think this also explains 
https://src.fedoraproject.org/rpms/binutils/blob/rawhide/f/binutils.spec#_947.

 ltmain.sh | 46 ++
 1 file changed, 34 insertions(+), 12 deletions(-)

diff --git a/ltmain.sh b/ltmain.sh
index 493e83c36f14..79cd7c57f42e 100644
--- a/ltmain.sh
+++ b/ltmain.sh
@@ -4966,19 +4966,41 @@ func_mode_link ()
arg="$func_quote_for_eval_result"
;;
 
-  # -64, -mips[0-9] enable 64-bit mode on the SGI compiler
-  # -r[0-9][0-9]* specifies the processor on the SGI compiler
-  # -xarch=*, -xtarget=* enable 64-bit mode on the Sun compiler
-  # +DA*, +DD* enable 64-bit mode on the HP compiler
-  # -q* pass through compiler args for the IBM compiler
-  # -m*, -t[45]*, -txscale* pass through architecture-specific
-  # compiler args for GCC
-  # -F/path gives path to uninstalled frameworks, gcc on darwin
-  # -p, -pg, --coverage, -fprofile-* pass through profiling flag for GCC
-  # @file GCC response files
-  # -tp=* Portland pgcc target processor selection
+  # Flags to be passed through unchanged, with rationale:
+  # -64, -mips[0-9]  enable 64-bit mode for the SGI compiler
+  # -r[0-9][0-9]*specify processor for the SGI compiler
+  # -xarch=*, -xtarget=* enable 64-bit mode for the Sun compiler
+  # +DA*, +DD*   enable 64-bit mode for the HP compiler
+  # -q*  compiler args for the IBM compiler
+  # -m*, -t[45]*, -txscale* architecture-specific flags for GCC
+  # -F/path  path to uninstalled frameworks, gcc on darwin
+  # -p, -pg, --coverage, -fprofile-*  profiling flags for GCC
+  # -fstack-protector*   stack protector flags for GCC
+  # @fileGCC response files
+  # -tp=*Portland pgcc target processor selection
+  # -O*, -g*, -flto*, -fwhopr*, -fuse-linker-plugin GCC link-time 
optimization
+  # -specs=* GCC specs files
+  # -stdlib=*select c++ std lib with clang
+  # -fdiagnostics-color* simply affects output
+  # -frecord-gcc-switches used to verify flags were respected
+  # -fsanitize=* Clang/GCC memory and address sanitizer
+  # -fno-sanitize*   Clang/GCC memory and address sanitizer
+  # -shared-libsan   Link with shared sanitizer runtimes (Clang)
+  # -static-libsan   Link with static sanitizer runtimes (Clang)
+  # -fuse-ld=*   Linker select flags for GCC
+  # -rtlib=* select c runtime lib with clang
+  # --unwindlib=*select unwinder library with clang
+  # -f{file|debug|macro|profile}-prefix-map=* needed for lto linking
+  # -Wa,*Pass flags directly to the assembler
+  # -Werror, -Werror=*   Report (specified) warnings as errors
   -64|-mips[0-9]|-r[0-9][0-9]*|-xarch=*|-xtarget=*|+DA*|+DD*|-q*|-m*| \
-  -t[45]*|-txscale*|-p|-pg|--coverage|-fprofile-*|-F*|@*|-tp=*)
+  -t[45]*|-txscale*|-p|-pg|--coverage|-fprofile-*|-F*|@*|-tp=*| \
+  -O*|-g*|-flto*|-fwhopr*|-fuse-linker-plugin|-fstack-protector*| \
+  -stdlib=*|-rtlib=*|--unwindlib=*| \
+  -specs=*|-fsanitize=*|-fno-sanitize*|-shared-libsan|-static-libsan| \
+  
-ffile-prefix-map=*|-fdebug-prefix-map=*|-fmacro-prefix-map=*|-fprofile-prefix-map=*|
 \
+  -fdiagnostics-color*|-frecord-gcc-switches| \
+  -fuse-ld=*|-Wa,*|-Werror|-Werror=*)
 func_quote_for_eval "$arg"
arg="$func_quote_for_eval_result"
 func_append compile_command " $arg"
-- 
2.45.2



Re: [PATCH-1v4] Value Range: Add range op for builtin isinf

2024-08-14 Thread Sam James
Vineet Gupta  writes:

> Ping - looks like this is blocking the patches for builtin_isnormal and 
> builtin_isfinite !
>

See 
https://inbox.sourceware.org/gcc-patches/d9459db0-7301-40f6-a3cf-077017b8c...@gmail.com/.

It appears to be approved.

(Please also avoid topposting.)

> Thx,
> -Vineet
>
> On 8/5/24 07:51, Jeff Law wrote:
>>
>> On 7/23/24 4:39 PM, Andrew MacLeod wrote:
>>> the range is in r, and is set to [0,0].  this is the false part of what 
>>> is being returned for the range.
>>>
>>> the "return true" indicates we determined a range, so use what is in R.
>>>
>>> returning false means we did not find a range to return, so r is garbage.
>> Duh.  I guess I should have realized that.  I'll have to take another 
>> look at Hao's patch.  It's likely OK, but let me take another looksie.
>>
>> jeff
>>
>>


Re: [PATCH] ltmain.sh: allow more flags at link-time

2024-08-14 Thread Sam James
Eric Gallager  writes:

> On Wed, Aug 14, 2024 at 8:50 AM Sam James  wrote:
>>
>> libtool defaults to filtering flags passed at link-time.
>>
>> This brings the filtering in GCC's 'fork' of libtool into sync with
>> upstream libtool commit 22a7e547e9857fc94fe5bc7c921d9a4b49c09f8e.
>
> I think it'd be worthwhile to link to the upstream commit in the
> ChangeLog / commit message, too. Also, are you sure that's the right
> one? It looks just like a version revbump commit to me:
> https://git.savannah.gnu.org/cgit/libtool.git/commit/?id=22a7e547e9857fc94fe5bc7c921d9a4b49c09f8e

'as of' meaning "this is the state of the repository when I
checked", so if you want to check my work, you should checkout
libtool.git at that commit and compare the product.

There is no single commit which does this, it was done over
many commits over many years. It's not worth trying to dig those many
commits up, IMO.


signature.asc
Description: PGP signature


[PATCH 1/2] Makefile.tpl: drop leftover intermodule cruft

2024-08-14 Thread Sam James
intermodule supported was dropped in r0-103106-gde6ba7aee152a0 with some
remaining bits for Fortran removed in r14-1696-gecc96eb5d2a0e5.

Remove some small leftovers.

* Makefile.in: Regenerate.
* Makefile.tpl (STAGE1_CONFIGURE_FLAGS: Remove --disable-intermodule.
---
 Makefile.in  | 11 ---
 Makefile.tpl | 11 ---
 2 files changed, 8 insertions(+), 14 deletions(-)

diff --git a/Makefile.in b/Makefile.in
index 34c5550beca2..a1a56bb5dd2c 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -610,14 +610,11 @@ STAGEautofeedback_CONFIGURE_FLAGS = 
$(STAGE_CONFIGURE_FLAGS)
 STAGE1_CFLAGS = @stage1_cflags@
 STAGE1_CHECKING = @stage1_checking@
 STAGE1_LANGUAGES = @stage1_languages@
-# * We force-disable intermodule optimizations, even if
-#   --enable-intermodule was passed, since the installed compiler
-#   probably can't handle them.  Luckily, autoconf always respects
-#   the last argument when conflicting --enable arguments are passed.
-# * Likewise, we force-disable coverage flags, since the installed
-#   compiler probably has never heard of them.
+# * We force-disable coverage flags, since the installed compiler probably
+#   has never heard of them. Luckily, autoconf always respects the last
+#   argument when conflicting --enable arguments are passed.
 # * We also disable -Wformat, since older GCCs don't understand newer %s.
-STAGE1_CONFIGURE_FLAGS = --disable-intermodule $(STAGE1_CHECKING) \
+STAGE1_CONFIGURE_FLAGS = $(STAGE1_CHECKING) \
  --disable-coverage --enable-languages="$(STAGE1_LANGUAGES)" \
  --disable-build-format-warnings
 
diff --git a/Makefile.tpl b/Makefile.tpl
index 8f4bf297918c..cbb3c6789dcf 100644
--- a/Makefile.tpl
+++ b/Makefile.tpl
@@ -533,14 +533,11 @@ STAGE[+id+]_CONFIGURE_FLAGS = $(STAGE_CONFIGURE_FLAGS)
 STAGE1_CFLAGS = @stage1_cflags@
 STAGE1_CHECKING = @stage1_checking@
 STAGE1_LANGUAGES = @stage1_languages@
-# * We force-disable intermodule optimizations, even if
-#   --enable-intermodule was passed, since the installed compiler
-#   probably can't handle them.  Luckily, autoconf always respects
-#   the last argument when conflicting --enable arguments are passed.
-# * Likewise, we force-disable coverage flags, since the installed
-#   compiler probably has never heard of them.
+# * We force-disable coverage flags, since the installed compiler probably
+#   has never heard of them. Luckily, autoconf always respects the last
+#   argument when conflicting --enable arguments are passed.
 # * We also disable -Wformat, since older GCCs don't understand newer %s.
-STAGE1_CONFIGURE_FLAGS = --disable-intermodule $(STAGE1_CHECKING) \
+STAGE1_CONFIGURE_FLAGS = $(STAGE1_CHECKING) \
  --disable-coverage --enable-languages="$(STAGE1_LANGUAGES)" \
  --disable-build-format-warnings
 
-- 
2.45.2



[PATCH 2/2] Makefile.tpl: fix whitespace in licence header

2024-08-14 Thread Sam James
* Makefile.tpl: Fix whitespace.
---
 Makefile.tpl | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Makefile.tpl b/Makefile.tpl
index cbb3c6789dcf..da38dca697ad 100644
--- a/Makefile.tpl
+++ b/Makefile.tpl
@@ -13,12 +13,12 @@ in
 # it under the terms of the GNU General Public License as published by
 # the Free Software Foundation; either version 3 of the License, or
 # (at your option) any later version.
-# 
+#
 # This program is distributed in the hope that it will be useful,
 # but WITHOUT ANY WARRANTY; without even the implied warranty of
 # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 # GNU General Public License for more details.
-# 
+#
 # You should have received a copy of the GNU General Public License
 # along with this program; see the file COPYING3.  If not see
 # .
-- 
2.45.2



  1   2   3   4   5   >