Re: introduce target tmpnam and require it in tests relying on it

[Bernhard Reutner-Fischer via Gcc-patches]

On 24 April 2020 11:59:50 CEST, Alexandre Oliva  wrote:
>On Apr 23, 2020, Martin Sebor  wrote:
>
>> Sure.  I'd go with _fileio but that's just a suggestion.
>
>Okiedokie, here's the patch using fileio instead of tmpnam for the
>effective target name.  I'm going to check it in shortly.
>
>
>introduce target fileio and require it in tests that use tmpnam

ISTM the corresponding documentation hunk for sourcebuild.texi  is missing.
TIA,
>
>Some target C libraries that aren't recognized as freestanding don't
>have filesystem support, so calling tmpnam, fopen/open and
>remove/unlink fails to link.
>
>This patch introduces a fileio effective target to the testsuite, and
>requires it in the tests that call tmpnam.
>
>
>for  gcc/testsuite/ChangeLog
>
>   * lib/target-supports.exp (check_effective_target_fileio): New.
>   * gcc.c-torture/execute/fprintf-2.c: Require it.
>   * gcc.c-torture/execute/printf-2.c: Likewise.
>   * gcc.c-torture/execute/user-printf.c: Likewise.
>---
> gcc/testsuite/gcc.c-torture/execute/fprintf-2.c   |1 +
> gcc/testsuite/gcc.c-torture/execute/printf-2.c|1 +
> gcc/testsuite/gcc.c-torture/execute/user-printf.c |1 +
> gcc/testsuite/lib/target-supports.exp |   13 +
> 4 files changed, 16 insertions(+)

Re: [PATCH 1/1] testsuite: Handle --save-temps in schedule-cleanups

[Bernhard Reutner-Fischer via Gcc-patches]

On 20 April 2020 13:43:42 CEST, Christophe Lyon via Gcc-patches 
 wrote:
>Some tests use --save-temps, but schedule-cleanups strictly matches
>-save-temps, so we leave many temporary files after validation.
>Instead of fixing every offending testcase, it's simpler and
>future-proof to make schedule-cleanups handle both --save-temps and
>-save-temps.

LGTM.

>
>2020-04-20  Christophe Lyon  
>
>   gcc/testsuite/
>   * lib/gcc-dg.exp (schedule-cleanups): Accept --save-temps.
>---
> gcc/testsuite/lib/gcc-dg.exp | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/gcc/testsuite/lib/gcc-dg.exp
>b/gcc/testsuite/lib/gcc-dg.exp
>index cccd3ce..27cc7c1 100644
>--- a/gcc/testsuite/lib/gcc-dg.exp
>+++ b/gcc/testsuite/lib/gcc-dg.exp
>@@ -171,7 +171,7 @@ proc schedule-cleanups { opts } {
>   verbose "dg-keep-saved-temps ${keep_saved_temps_suffixes}" 2
> }
> # -save-temps -> cleanup-saved-temps()
>-if [regexp -- {(^|\s+)-save-temps(\s+|$)} $opts] {
>+if [regexp -- {(^|\s+)-?-save-temps(\s+|$)} $opts] {
>   verbose "Cleanup -save-temps seen" 4
>   if [info exists keep_saved_temps_suffixes] {
>   append finalcode "cleanup-saved-temps
>${keep_saved_temps_suffixes}\n"

Re: [PATCH] reject scalar array initialization with nullptr [PR94510]

[Bernhard Reutner-Fischer via Gcc-patches]

On 17 April 2020 23:18:05 CEST, Martin Sebor via Gcc-patches 
 wrote:

+   zero-initialialization for its type, taking pointers to members

s/initialialization/initialization/

Re: introduce target tmpnam and require it in tests relying on it

[Bernhard Reutner-Fischer via Gcc-patches]

On 17 April 2020 21:21:41 CEST, Martin Sebor via Gcc-patches 
 wrote:
>On 4/17/20 11:48 AM, Alexandre Oliva wrote:
>> On Apr  9, 2020, Alexandre Oliva  wrote:
>> 
>>> Some target C libraries that aren't recognized as freestanding don't
>>> have filesystem support, so calling tmpnam, fopen/open and
>>> remove/unlink fails to link.
>> 
>>> This patch introduces a tmpnam effective target to the testsuite,
>and
>>> requires it in the tests that call tmpnam.
>> 
>>> Tested on x86_64-linux-gnu, and with a cross to arm-eabi.
>>> Ok to install?
>> 
>> 
>>> for  gcc/testsuite/ChangeLog
>> 
>>> * lib/target-supports.exp (check_effective_target_tmpnam): New.
>>> * gcc.c-torture/execute/fprintf-2.c: Require it.
>>> * gcc.c-torture/execute/printf-2.c: Likewise.
>>> * gcc.c-torture/execute/user-printf.c: Likewise.
>> 
>> Ping?
>> 
>> https://gcc.gnu.org/pipermail/gcc-patches/2020-April/543672.html
>
>I'm okay with the changes to the tests.
>
>The target-supports.exp changes look reasonable to me as well but
>I can't approve them.  Since you said it's for targets that don't
>have file I/O functions I wonder if the name would better reflect
>that if it were called, say, check_effective_target_fileio?

Since tmpnam is obsolescent in SUSv4 and hence a libc is fine to omit it, I'd 
rather fix the tests to use functions that are known to stay.

If you want a fileio predicate then please do not keys it off obsolescent 
functions.

TIA,
>
>I don't expect it's necessary to worry about handling errors in
>the .exp test.
>
>Martin

Re: introduce target tmpnam and require it in tests relying on it

[Bernhard Reutner-Fischer via Gcc-patches]

On Tue, 21 Apr 2020 at 16:59, Martin Sebor  wrote:

> >>> https://gcc.gnu.org/pipermail/gcc-patches/2020-April/543672.html
> >>
> >> I'm okay with the changes to the tests.
> >>
> >> The target-supports.exp changes look reasonable to me as well but
> >> I can't approve them.  Since you said it's for targets that don't
> >> have file I/O functions I wonder if the name would better reflect
> >> that if it were called, say, check_effective_target_fileio?
> >
> > Since tmpnam is obsolescent in SUSv4 and hence a libc is fine to omit it, 
> > I'd rather fix the tests to use functions that are known to stay.
>
> I would be okay with replacing tmpnam with something else, although
> I don't think it's necessary.  tmpnam is a standard C function that
> conforming C (and so POSIX) implementation are required to provide.

IMO it's perfectly fine to omit obsolescent functions from a
conforming implementation ¹).
But one should use the recommended replacement functions either way,
e.g. mkstemp ²)

thanks,
¹)
[OB] [Option Start] Obsolescent [Option End]
The functionality described may be removed in a future version of this
volume of POSIX.1-2017. Strictly Conforming POSIX Applications and
Strictly Conforming XSI Applications shall not use obsolescent
features.
²)
https://pubs.opengroup.org/onlinepubs/9699919799/functions/tmpnam.html
---8<---
APPLICATION USAGE
  Applications should use the tmpfile(), mkstemp(), or mkdtemp()
functions instead of the obsolescent tmpnam() function.
---8<---

Re: [PATCH] fortran/95509 - fix spellcheck-operator.f90 regression

[Bernhard Reutner-Fischer via Gcc-patches]

On 5 June 2020 17:12:58 CEST, Thomas Koenig via Fortran  
wrote:
>Hi Tom,
>
>> My earlier patch to add case handling to the spell checker caused a
>> Fortran regression.  I believe I must have misread the test results.
>> 
>> This patch fixes the problem by changing the cutoff.  I chose this
>> value because the previous patch effectively multiplied the result of
>> get_edit_distance by 2 (unless a case change is involved).
>
>OK.  Thanks for the patch!

Yes, this is probably OK although Fortran is case- insensitive. I think we 
lowercase names coming in from the source (at least for the internal identifier 
nodes), so we should not be affected by any case change.

thanks,

Re: [PATCH] fortran/95509 - fix spellcheck-operator.f90 regression

[Bernhard Reutner-Fischer via Gcc-patches]

On Tue, 9 Jun 2020 08:14:55 +0200
Thomas Koenig  wrote:

> Hi Tom,
> 
> > Bernhard> Yes, this is probably OK although Fortran is case-
> > Bernhard> insensitive. I think we lowercase names coming in from the
> > Bernhard> source (at least for the internal identifier nodes), so we
> > Bernhard> should not be affected by any case change.  
> > 
> > I already checked it in, but FWIW it would be pretty easy to change
> > get_edit_distance to have a case-insensitive mode.  
> 
> For Fortran identifiers, that would be really good.  Would you do that?

Is it really needed?

We do not enter internal helper names (class names, anything
artificial, anything leading underscore or leading uppercase) to the
suggestions proposals so should never actually recommend any of those?

And we lowercase identifiers from the user:
$ printf "iIIi = 42\nend" | gfortran-10 -fimplicit-none -ffree-form
-xf95 - :1:4:

Error: Symbol ‘’ at (1) has no IMPLICIT type

Why would we need a case-insensitive switch for the distance?
TIA,

Re: RFA: Add option -fretry-compilation

[Bernhard Reutner-Fischer via Gcc-patches]

On 16 May 2021 20:21:13 CEST, Joern Rennecke  
wrote:

>The attached patch adds a new option -fretry-compilation that allows
>you to specify a list - or
>lists - of options to use for a compilation retry, which is
>implemented in the compiler driver.

That's gross ;)

+If the compiler fails, retry with named options appeded.  Separate multiple 
options with ',', and multiple alternatives with ':' .
s/appeded/appended/

>
>Bootstrapped on x86_64-pc-linux-gnu.

Re: [Patch] Testsuite/Fortran: gfortran.dg/pr96711.f90 - fix expected value for PowerPC [PR96983]

[Bernhard Reutner-Fischer via Gcc-patches]

On Wed, 19 May 2021 20:35:26 +0200
Tobias Burnus  wrote:

> Hi Segher,
> 
> Quick version: Jump to the new patch, which I like much more.

> Namely, as the attached updated patch does.
> 
> As I like that patch and believe it is obvious, I intent to

/intent/s/tent/tend/

> commit it as such – unless there are further comments.

No real comment except that it sounds odd to arrive at 53 instead of
the quad bits precision on an arch that allegedly does hardware FP?
Did not look close though so i claim to haven't said a word, let alone
2*2 nor four.

thanks,
> 
> It passes on both x86-64-gnu-linux and powerpc64le-none-linux-gnu.
> I think the radix == 2 is a good bet, but if we ever run into issues,
> it can also be changed to use radix(...) as well ...
> 
> Tobias
> 
> -
> Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München 
> Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank 
> Thürauf

Re: [Patch] Testsuite/Fortran: gfortran.dg/pr96711.f90 - fix expected value for PowerPC [PR96983]

[Bernhard Reutner-Fischer via Gcc-patches]

On Wed, 19 May 2021 22:39:13 +0200
Bernhard Reutner-Fischer  wrote:

> On Wed, 19 May 2021 20:35:26 +0200
> Tobias Burnus  wrote:

> > commit it as such – unless there are further comments.  
> 
> No real comment except ..

why don't we end up with IEEE binary128 quadruple precision here per
default. Suspicious, no?
thanks,

Re: [PATCH] Simplify option handling for -fsanitize-coverage

[Bernhard Reutner-Fischer via Gcc-patches]

On 20 May 2021 12:43:17 CEST, "Martin Liška"  wrote:

>  /* Given ARG, an unrecognized sanitizer option, return the best
> matching sanitizer option, or NULL if there isn't one.
> OPTS is array of candidate sanitizer options.
>-   CODE is OPT_fsanitize_, OPT_fsanitize_recover_ or
>-   OPT_fsanitize_coverage_.
>+   CODE is OPT_fsanitize_or OPT_fsanitize_recover_.

Shouldn't be there a space before "or" in OPT_fsanitize_or ?
thanks,

Re: [PATCH 06/57] rs6000: Add helper functions for parsing

[Bernhard Reutner-Fischer via Gcc-patches]

On 21 May 2021 22:56:09 CEST, Bill Schmidt via Gcc-patches 
 wrote:

>>> +  if (lastpos < pos)
>>> +return 0;
>>> +
>>> +  char *buf = (char *) malloc (lastpos - pos + 2);
>>> +  memcpy (buf, &linebuf[pos], lastpos - pos + 1);
>>> +  buf[lastpos - pos + 1] = '\0';
>>> +
>>> +  pos = lastpos + 1;
>>> +  return buf;
>>> +}
>> Are there no utility routines you can use?  It would be useful to
>have
>> something that all gen* can use (something less bare than what there
>is
>> now...)
>
>I didn't find anything great as I was poking around, hence I wrote my 
>own low level utilities.  It goes back to my desire to track line/pos 
>information for debug.
>
>Thanks for the review!

You saw the unchecked usage of the malloc return value, did you?

We certainly warn about that, I'd hope.
thanks,

Re: [PATCH] arc: Add --with-fpu support for ARCv2 cpus

[Bernhard Reutner-Fischer via Gcc-patches]

On Fri,  4 Jun 2021 10:29:09 +0300
Claudiu Zissulescu via Gcc-patches  wrote:

> Hi Jeff,
> 
> I would like to add spport for selecting the ARCv2 FPU extension at
> configuration-time.
> 
> The --with-fpu configuration option is ignored when -mfpu compiler
> option is specified.
> 
> My concern is using `grep -P` when configuring. Is that ok?

Please don't.
Not every grep(1) has PCRE support so it would be great if you could
rephrase it to use just use normal regexp or ERE.

like e.g.:
grep -q -E "^ARC_CPU \(hs38,[   ]*[emhs]+," config/arc/arc-cpus.def

where the space is <^vi>, i.e. space, tab
Or use an awk script that exits appropriately if the arg matches ARCH.

TIA,

Re: RFC: Sphinx for GCC documentation

[Bernhard Reutner-Fischer via Gcc-patches]

On Mon, 7 Jun 2021 15:30:22 +0200
Martin Liška  wrote:

> Anyway, this is resolved as I use more appropriate directive:
> https://splichal.eu/scripts/sphinx/gfortran/_build/html/intrinsic-procedures/access-checks-file-access-modes.html

ISTM there's a typo s/Tailing/Trailing/ in gcc/fortran/intrinsic.texi

git grep -wi Tailing
seems to highlight a couple more.
Maybe you have time to fix these?

PS: The occurrence in gcc/testsuite/gcc.dg/format/strfmon-1.c sounds
odd.
TIA,

Re: [PATCH] arc: Add --with-fpu support for ARCv2 cpus

[Bernhard Reutner-Fischer via Gcc-patches]

On Tue, 8 Jun 2021 10:05:28 +0300
Claudiu Zissulescu  wrote:

> Thank you for your input.
> 
> I have made an update using grep's ERE. Please let me know if it is ok.

I would have written [[:space:]]* instead of [[:space:]]+ to handle
potentially missing space, at least after the comma but also before the
comma to avoid surprises for new names in the future.
Furthermore | alone would be [[:blank:]]* but as you prefer.

grep ... > /dev/null would be grep -q which is mandated by POSIX since
at least SUSv2 so can be used safely since quite some time now.

Instead of the redundant 'true' calls, i'd usually write :
E.g.
if grep -q ... ; then :
else echo "nah"; exit 1
fi

Which could be shortened to
if ! grep -q ...
then
  echo "nah"
  exit 1
fi

to avoid any questions about an empty arm in the first place.

ISTM you only set the expected flags in the switch so i would have
set only that variable and have grepped only once after the switch for
brevity.

Either way, thanks for not using grep -P :)
thanks,

Re: [PATCH] warn for integer overflow in allocation calls (PR 96838)

[Bernhard Reutner-Fischer via Gcc-patches]

On 15 September 2020 21:47:46 CEST, Martin Sebor via Gcc-patches 
 wrote:
>Overflowing the size of a dynamic allocation (e.g., malloc or VLA)
>can lead to a subsequent buffer overflow corrupting the heap or
>stack.  The attached patch diagnoses a subset of these cases where
>the overflow/wraparound is still detectable.
>
>Besides regtesting GCC on x86_64-linux I also verified the warning
>doesn't introduce any false positives into Glibc or Binutils/GDB
>builds on the same target.

+/* Try to evaluate the artithmetic EXPresssion representing the size of

s/EXPresssion/expression EXP/

You had a bit more s than strictly necessary..
thanks,

>
>Martin

Re: [ Preprocessor ] [ Common ] Feature: Macros for identifying the wide and narrow execution string literal encoding

[Bernhard Reutner-Fischer via Gcc-patches]

On 8 October 2020 23:39:15 CEST, JeanHeyd Meneide via Gcc-patches 
 wrote:
>Dear Joseph,
>
>On Thu, Oct 8, 2020 at 1:36 PM Joseph Myers 
>wrote:
>>
>> This documentation doesn't seem sufficient to use the macros.  Do
>they
>> expand to (narrow) string literals?  To an unquoted sequence of
>> characters?  I think from the implementation that the answer is
>strings
>> (so, in particular, not usable for testing anything in #if
>conditionals),
>> but the documentation ought to say so.  The test ought to verify the
>form
>> of the expansion as well (even if it can't do anything useful at
>execution
>> time, because if you make the macros reflect the command-line options
>they
>> are character set names that are meaningful on the host, and any
>> conversion functionality on the target may not use the same names as
>the
>> host).
>
> You're right; sorry about that, I should have been more thorough!
>I thought about adding a test to check the name itself (e.g, for
>"UTF-8"), but that might make tests fail on platforms where the
>default SOURCE_CHARSET from the dev files is not, in fact, UTF-8. I
>could also try to pass some options but then I'd have to guarantee
>that the encoding was available on all testable platforms, too...!
>
>In the end, for the tests, I just initialize two "const char[]"
>directly from the macro expansions to make sure we are getting
>strings. It seems to work okay. Attached is the revised patch with
>better docs and test!

Typo:  comple-time

>2020-10-08  JeanHeyd "ThePhD" Meneide  
>
>* gcc/c-family/c-cppbuiltin.c: Add predefined macro
>definitions for charsets

I think you should put the macro names in braces after the filename and drop 
the trailing "for charsets".

>* gcc/doc/cpp.texi: Document new predefined macro.
>* gcc/testsuite/c-c++-common/cpp/wide-narrow-predef-macros.c (new):

I think you should drop "(new)" above.
thanks,

>  New test for macro definitions to always exist.
>* libcpp/include/cpplib.h: Add functions declarations for
>  retrieving charset names
>* libcpp/directives.c: Add function definitions to retrieve charset
>  names.
>* libcpp/internal.h: Add to/from name preservations

Re: [PATCH v9] Practical improvement to libgcc complex divide

[Bernhard Reutner-Fischer via Gcc-patches]

On Fri, 26 Mar 2021 23:14:41 +
Patrick McGehearty via Gcc-patches  wrote:

> Changes in Version 9 since Version 8:
> 
> Revised code to meet gcc coding standards in all files, especially
> with respect to adding spaces around operations and removing
> excess () in #define macro definitions.

Did you gather cycle counter diffs for current against new for the
normal and some edge cases?

Can you please share data for -Os between current and your proposed new?

TIA,

Re: [PATCH v9] Practical improvement to libgcc complex divide

[Bernhard Reutner-Fischer via Gcc-patches]

On Fri, 26 Mar 2021 23:14:41 +
Patrick McGehearty via Gcc-patches  wrote:

> diff --git a/gcc/c-family/c-cppbuiltin.c b/gcc/c-family/c-cppbuiltin.c
> index 9f993c4..c0d9e57 100644
> --- a/gcc/c-family/c-cppbuiltin.c
> +++ b/gcc/c-family/c-cppbuiltin.c
> @@ -1277,8 +1277,10 @@ c_cpp_builtins (cpp_reader *pfile)
>   {
> scalar_float_mode mode = mode_iter.require ();
> const char *name = GET_MODE_NAME (mode);
> +   const int name_len = strlen (name);

strlen returns a size_t

Funny that we do not have a pre-seeded mode_name_len array but call
strlen on modes over and over again.

> +   char float_h_prefix[16] = "";
> char *macro_name
> - = (char *) alloca (strlen (name)
> + = (char *) alloca (name_len
>  + sizeof ("__LIBGCC__MANT_DIG__"));
> sprintf (macro_name, "__LIBGCC_%s_MANT_DIG__", name);
> builtin_define_with_int_value (macro_name,
> @@ -1286,20 +1288,29 @@ c_cpp_builtins (cpp_reader *pfile)
> if (!targetm.scalar_mode_supported_p (mode)
> || !targetm.libgcc_floating_mode_supported_p (mode))
>   continue;
> -   macro_name = (char *) alloca (strlen (name)
> +   macro_name = (char *) alloca (name_len
>   + sizeof ("__LIBGCC_HAS__MODE__"));
> sprintf (macro_name, "__LIBGCC_HAS_%s_MODE__", name);
> cpp_define (pfile, macro_name);
> -   macro_name = (char *) alloca (strlen (name)
> +   macro_name = (char *) alloca (name_len
>   + sizeof ("__LIBGCC__FUNC_EXT__"));
> sprintf (macro_name, "__LIBGCC_%s_FUNC_EXT__", name);

The above should have been split out as separate independent patchlet
to get it out of the way.

As noted already the use of alloca is a pre-existing (coding-style) bug
and we should use XALLOCAVEC or appropriately sized fixed buffers to
begin with. The amount of alloca in defining all these is amazing.

> diff --git a/gcc/testsuite/gcc.c-torture/execute/ieee/cdivchkd.c 
> b/gcc/testsuite/gcc.c-torture/execute/ieee/cdivchkd.c
> new file mode 100644
> index 000..409123f
> --- /dev/null
> +++ b/gcc/testsuite/gcc.c-torture/execute/ieee/cdivchkd.c
> @@ -0,0 +1,126 @@
> +/*
> +  Program to test complex divide for correct results on selected values.
> +  Checking known failure points.
> +*/
> +
> +#include 
> +
> +extern void abort ();
> +extern void exit ();

As Joseph pointed out in the v8 review already, please use prototyped
function declarations in all new tests.

> diff --git a/gcc/testsuite/gcc.c-torture/execute/ieee/cdivchkld.c 
> b/gcc/testsuite/gcc.c-torture/execute/ieee/cdivchkld.c
> new file mode 100644
> index 000..28d707d
> --- /dev/null
> +++ b/gcc/testsuite/gcc.c-torture/execute/ieee/cdivchkld.c
> @@ -0,0 +1,167 @@
> +/*
> +  Program to test complex divide for correct results on selected values.
> +  Checking known failure points.
> +*/
> +
> +#include 
> +
> +extern void abort ();
> +extern void exit ();

Likewise.

> +#if (LDBL_MAX_EXP < 2048)
> +  /*
> +Test values when mantissa is 11 or fewer bits.  Either LDBL is
> +using DBL on this platform or we are using IBM extended double
> +precision Test values will be automatically trucated the available
> +precision.

s/trucated/truncated/g; # with an 'n'
I have trouble parsing the sentence nevertheless. There might be a
missing "which" somewhere and maybe a "to"?

> +#else
> +  /*
> +Test values intended for either IEEE128 or Intel80 formats.  In
> +either case, 15 bits of exponent are available.  Test values will
> +be automatically trucated the available precision.
> +  */

again, truNcated and a couple of words missing?

> diff --git a/libgcc/config/rs6000/_divkc3.c b/libgcc/config/rs6000/_divkc3.c
> index d261f40..f57e14f 100644
> --- a/libgcc/config/rs6000/_divkc3.c
> +++ b/libgcc/config/rs6000/_divkc3.c
> @@ -37,31 +37,118 @@ see the files COPYING3 and COPYING.RUNTIME respectively. 
>  If not, see
>  #define __divkc3 __divkc3_sw
>  #endif
>  
> +#define RBIG   (__LIBGCC_TF_MAX__ / 2)
> +#define RMIN   (__LIBGCC_TF_MIN__)
> +#define RMIN2  (__LIBGCC_TF_EPSILON__)
> +#define RMINSCAL (1 / __LIBGCC_TF_EPSILON__)
> +#define RMAX2  (RBIG * RMIN2)
> +
> +
>  TCtype
>  __divkc3 (TFtype a, TFtype b, TFtype c, TFtype d)
>  {
>TFtype denom, ratio, x, y;
>TCtype res;
>  
> -  /* ??? We can get better behavior from logarithmic scaling instead of
> - the division.  But that would mean starting to link libgcc against
> - libm.  We could implement something akin to ldexp/frexp as gcc builtins
> - fairly easily...  */
> +  /* long double has significant potential underflow/overflow errors than
> + can be greatly reduced with a limited number of tests and adjustments.
> +  */

"than" doesn't parse. that?

>else
>  {
> +  /* Prevent underflow when denominator is near max representable.  */
> +  if (FABS (c) >= RBIG)
> + {
> +   

Re: [PATCH] x86: Don't generate uiret with -mcmodel=kernel

[Bernhard Reutner-Fischer via Gcc-patches]

On 1 April 2021 18:54:07 CEST, "H.J. Lu via Gcc-patches" 
 wrote:
>Since uiret should be used only for user interrupt handler return,
>don't
>generate uiret in interrupt handler return with -mcmodel=kernel even if
>UINTR is enabled.
>
>gcc/
>
>   PR target/99870
>   * config/i386/i386.md (interrupt_return): Don't generate uiret
>   for -mcmodel=kernel.
>
>gcc/testsuite/
>
>   * gcc.target/i386/pr99870.c: New test.
>---
> gcc/config/i386/i386.md |  3 ++-
> gcc/testsuite/gcc.target/i386/pr99870.c | 19 +++
> 2 files changed, 21 insertions(+), 1 deletion(-)
> create mode 100644 gcc/testsuite/gcc.target/i386/pr99870.c
>
>diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
>index 9ff35d9a607..1055b4187b2 100644
>--- a/gcc/config/i386/i386.md
>+++ b/gcc/config/i386/i386.md
>@@ -13885,7 +13885,8 @@ (define_insn "interrupt_return"
>(unspec [(const_int 0)] UNSPEC_INTERRUPT_RETURN)]
>   "reload_completed"
> {
>-  return TARGET_64BIT ? (TARGET_UINTR ? "uiret" : "iretq") : "iret";
>+  return TARGET_64BIT ? ((TARGET_UINTR && ix86_cmodel != CM_KERNEL)
>+   ? "uiret" : "iretq") : "iret";
> })
> 
>;; Used by x86_machine_dependent_reorg to avoid penalty on single byte
>RET
>diff --git a/gcc/testsuite/gcc.target/i386/pr99870.c
>b/gcc/testsuite/gcc.target/i386/pr99870.c
>new file mode 100644
>index 000..0dafa001ba9
>--- /dev/null
>+++ b/gcc/testsuite/gcc.target/i386/pr99870.c
>@@ -0,0 +1,19 @@
>+/* { dg-do compile } */
>+/* { dg-options "-O2 -march=sapphirerapids -mgeneral-regs-only" } */
>+/* { dg-additional-options "-mcmodel=kernel" { target { ! ia32 } } }
>*/
>+
>+void
>+__attribute__((interrupt))
>+fn (void *frame)
>+{
>+}
>+
>+typedef void (*fn_t) (void *) __attribute__((interrupt));
>+
>+fn_t fns[] =
>+{
>+  fn,
>+};
>+
>+/* { dg-final { scan-assembler-times "\\tiret" 1 { target ia32 } } }

This matches ia32 and the others as well (x32 and amd64 or x86_64) doesn't it.
>*/
>+/* { dg-final { scan-assembler-times "\\tiretq" 1 { target { ! ia32 }
>} } } */

So this is superfluous without a trailing anchor for the first, fwiw.
Thanks,

Re: [PATCH] x86: Don't generate uiret with -mcmodel=kernel

[Bernhard Reutner-Fischer via Gcc-patches]

On 1 April 2021 20:31:13 CEST, Bernhard Reutner-Fischer  
wrote:
>On 1 April 2021 18:54:07 CEST, "H.J. Lu via Gcc-patches"
> wrote:
>>Since uiret should be used only for user interrupt handler return,
>>don't
>>generate uiret in interrupt handler return with -mcmodel=kernel even
>if
>>UINTR is enabled.
>>
>>gcc/
>>
>>  PR target/99870
>>  * config/i386/i386.md (interrupt_return): Don't generate uiret
>>  for -mcmodel=kernel.
>>
>>gcc/testsuite/
>>
>>  * gcc.target/i386/pr99870.c: New test.
>>---
>> gcc/config/i386/i386.md |  3 ++-
>> gcc/testsuite/gcc.target/i386/pr99870.c | 19 +++
>> 2 files changed, 21 insertions(+), 1 deletion(-)
>> create mode 100644 gcc/testsuite/gcc.target/i386/pr99870.c
>>
>>diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
>>index 9ff35d9a607..1055b4187b2 100644
>>--- a/gcc/config/i386/i386.md
>>+++ b/gcc/config/i386/i386.md
>>@@ -13885,7 +13885,8 @@ (define_insn "interrupt_return"
>>(unspec [(const_int 0)] UNSPEC_INTERRUPT_RETURN)]
>>   "reload_completed"
>> {
>>-  return TARGET_64BIT ? (TARGET_UINTR ? "uiret" : "iretq") : "iret";
>>+  return TARGET_64BIT ? ((TARGET_UINTR && ix86_cmodel != CM_KERNEL)
>>+  ? "uiret" : "iretq") : "iret";
>> })
>> 
>>;; Used by x86_machine_dependent_reorg to avoid penalty on single byte
>>RET
>>diff --git a/gcc/testsuite/gcc.target/i386/pr99870.c
>>b/gcc/testsuite/gcc.target/i386/pr99870.c
>>new file mode 100644
>>index 000..0dafa001ba9
>>--- /dev/null
>>+++ b/gcc/testsuite/gcc.target/i386/pr99870.c
>>@@ -0,0 +1,19 @@
>>+/* { dg-do compile } */
>>+/* { dg-options "-O2 -march=sapphirerapids -mgeneral-regs-only" } */
>>+/* { dg-additional-options "-mcmodel=kernel" { target { ! ia32 } } }
>>*/
>>+
>>+void
>>+__attribute__((interrupt))
>>+fn (void *frame)

Oh and of course that named param should trigger an unused parm warning making 
the test fail as is.
Why wasn't that flagged? Isn't this warning on by default?

>>+{
>>+}
>>+
>>+typedef void (*fn_t) (void *) __attribute__((interrupt));
>>+
>>+fn_t fns[] =
>>+{
>>+  fn,
>>+};
>>+
>>+/* { dg-final { scan-assembler-times "\\tiret" 1 { target ia32 } } }
>
>This matches ia32 and the others as well (x32 and amd64 or x86_64)
>doesn't it.
>>*/
>>+/* { dg-final { scan-assembler-times "\\tiretq" 1 { target { ! ia32 }
>>} } } */
>
>So this is superfluous without a trailing anchor for the first, fwiw.
>Thanks,

Re: [PATCH] ada/adaint.c (__gnat_copy_attribs): RTEMS should use utime()

[Bernhard Reutner-Fischer via Gcc-patches]

On 1 April 2021 20:32:34 CEST, Joel Sherrill  wrote:
>Change the preprocessor logic so RTEMS uses utime().
>gcc/ada/
>   * adaint.c (__gnat_copy_attribs): RTEMS should use utime().

RTEMS probably doesn't care alot about accurate time, from the looks. Otherwise 
it would not mandate use of the obsolescent utime() (AFA SUS is concerned WRT 
nanoseconds precision) it seems?
They probably know what they're doing I suppose.
thanks,
PS I shouldn't reply to none of my business, I know..
>---
> gcc/ada/adaint.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/gcc/ada/adaint.c b/gcc/ada/adaint.c
>index 0a90c92402c..d3b83f61076 100644
>--- a/gcc/ada/adaint.c
>+++ b/gcc/ada/adaint.c
>@@ -3270,7 +3270,7 @@ __gnat_copy_attribs (char *from ATTRIBUTE_UNUSED,
>char *to ATTRIBUTE_UNUSED,
>  return -1;
>   }
> 
>-#if (defined (__vxworks) && _WRS_VXWORKS_MAJOR < 7)
>+#if (defined (__vxworks) && _WRS_VXWORKS_MAJOR < 7) ||
>defined(__rtems__)
> 
>   /* VxWorks prior to 7 only has utime.  */
> 

Re: [PATCH] ada/adaint.c (__gnat_copy_attribs): RTEMS should use utime()

[Bernhard Reutner-Fischer via Gcc-patches]

On 1 April 2021 21:01:27 CEST, Bernhard Reutner-Fischer  
wrote:
>On 1 April 2021 20:32:34 CEST, Joel Sherrill  wrote:
>>Change the preprocessor logic so RTEMS uses utime().
>>gcc/ada/
>>  * adaint.c (__gnat_copy_attribs): RTEMS should use utime().
>
>RTEMS probably doesn't care alot about accurate time, from the looks.
>Otherwise it would not mandate use of the obsolescent utime() (AFA SUS
>is concerned WRT nanoseconds precision) it seems?
>They probably know what they're doing I suppose.
>thanks,
>PS I shouldn't reply to none of my business, I know..

Meh. Okok. You got me. April 1st ;)
grats :)

Re: [PATCH 2/2] Add IEEE 128-bit min/max support on PowerPC

[Bernhard Reutner-Fischer via Gcc-patches]

On Fri, 09 Apr 2021 11:54:59 -0500
will schmidt via Gcc-patches  wrote:

> On Fri, 2021-04-09 at 10:43 -0400, Michael Meissner wrote:

> > gcc/
> > 2021-04-09 Michael Meissner  

> > (movcc_fpmask): Replace
> > movcc_p9.  Add IEEE 128-bit fp support.
> > (movcc_invert_fpmask): Replace
> > movcc_invert_p9.  Add IEEE 128-bit fp
> > support.
> > (fpmask): Add IEEE 128-bit fp support.  Enable generator to
> > build te RTL.
> > (xxsel): Add IEEE 128-bit fp support.  Enable generator to
> > build te RTL.  
> ok

te? the?

> > diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md
> > index 17b2fdc1cdd..ca4a4d01f05 100644
> > --- a/gcc/config/rs6000/rs6000.md
> > +++ b/gcc/config/rs6000/rs6000.md


> >  ;; Handle inverting the fpmask comparisons.
> > -(define_insn_and_split "*movcc_invert_p9"
> > -  [(set (match_operand:SFDF 0 "vsx_register_operand" 
> > "=&,")
> > -   (if_then_else:SFDF
> > +(define_insn_and_split "*movcc_invert_fpmask"
> > +  [(set (match_operand:FPMASK 0 "vsx_register_operand" "=")
> > +   (if_then_else:FPMASK
> >  (match_operator:CCFP 1 "invert_fpmask_comparison_operator"
> > -   [(match_operand:SFDF2 2 "vsx_register_operand" 
> > ",")
> > -(match_operand:SFDF2 3 "vsx_register_operand" 
> > ",")])
> > -(match_operand:SFDF 4 "vsx_register_operand" ",")
> > -(match_operand:SFDF 5 "vsx_register_operand" ",")))
> > -   (clobber (match_scratch:V2DI 6 "=0,&wa"))]
> > +   [(match_operand:FPMASK2 2 "vsx_register_operand" "")
> > +(match_operand:FPMASK2 3 "vsx_register_operand" "")])
> > +(match_operand:FPMASK 4 "vsx_register_operand" "")
> > +(match_operand:FPMASK 5 "vsx_register_operand" "")))
> > +   (clobber (match_scratch:V2DI 6 "=&"))]
> >"TARGET_P9_MINMAX"
> >"#"
> >"&& 1"
> > -  [(set (match_dup 6)
> > -   (if_then_else:V2DI (match_dup 9)
> > -  (match_dup 7)
> > -  (match_dup 8)))
> > -   (set (match_dup 0)
> > -   (if_then_else:SFDF (ne (match_dup 6)
> > -  (match_dup 8))
> > -  (match_dup 5)
> > -  (match_dup 4)))]
> > +  [(pc)]
> >  {
> > -  rtx op1 = operands[1];
> > -  enum rtx_code cond = reverse_condition_maybe_unordered (GET_CODE (op1));
> > +  rtx dest = operands[0];
> > +  rtx old_cmp = operands[1];
> > +  rtx cmp_op1 = operands[2];
> > +  rtx cmp_op2 = operands[3];
> > +  enum rtx_code cond = reverse_condition_maybe_unordered (GET_CODE 
> > (old_cmp));

I think you can drop the enum keyword.
Didn't read the pattern in detail.

> > +  rtx cmp_rev = gen_rtx_fmt_ee (cond, CCFPmode, cmp_op1, cmp_op2);
> > +  rtx move_f = operands[4];
> > +  rtx move_t = operands[5];
> > +  rtx mask_reg = operands[6];
> > +  rtx mask_m1 = CONSTM1_RTX (V2DImode);
> > +  rtx mask_0 = CONST0_RTX (V2DImode);
> > +  machine_mode move_mode = mode;
> > +  machine_mode compare_mode = mode;
> > 
> > -  if (GET_CODE (operands[6]) == SCRATCH)
> > -operands[6] = gen_reg_rtx (V2DImode);
> > +  if (GET_CODE (mask_reg) == SCRATCH)
> > +mask_reg = gen_reg_rtx (V2DImode);
> > 
> > -  operands[7] = CONSTM1_RTX (V2DImode);
> > -  operands[8] = CONST0_RTX (V2DImode);
> > +  /* Emit the compare and set mask instruction.  */
> > +  emit_insn (gen_fpmask (mask_reg, cmp_rev, cmp_op1, cmp_op2,
> > +  mask_m1, mask_0));
> > 
> > -  operands[9] = gen_rtx_fmt_ee (cond, CCFPmode, operands[2], operands[3]);
> > +  /* If we have a 64-bit comparison, but an 128-bit move, we need to 
> > extend the
> > + mask.  Because we are using the splat builtin to extend the V2DImode, 
> > we
> > + need to use element 1 on little endian systems.  */
> > +  if (!FLOAT128_IEEE_P (compare_mode) && FLOAT128_IEEE_P (move_mode))
> > +{
> > +  rtx element = WORDS_BIG_ENDIAN ? const0_rtx : const1_rtx;
> > +  emit_insn (gen_vsx_xxspltd_v2di (mask_reg, mask_reg, element));
> > +}
> > +
> > +  /* Now emit the XXSEL insn.  */
> > +  emit_insn (gen_xxsel (dest, mask_reg, mask_0, move_t, 
> > move_f));
> > +  DONE;
> >  }
> > - [(set_attr "length" "8")
> > + ;; length is 12 in case we need to add XXPERMDI
> > + [(set_attr "length" "12")
> >(set_attr "type" "vecperm")])
> > 
> > -(define_insn "*fpmask"
> > -  [(set (match_operand:V2DI 0 "vsx_register_operand" "=wa")
> > +(define_insn "fpmask"
> > +  [(set (match_operand:V2DI 0 "vsx_register_operand" "=")
> > (if_then_else:V2DI
> >  (match_operator:CCFP 1 "fpmask_comparison_operator"
> > -   [(match_operand:SFDF 2 "vsx_register_operand" "")
> > -(match_operand:SFDF 3 "vsx_register_operand" "")])
> > +   [(match_operand:FPMASK 2 "vsx_register_operand" "")
> > +(match_operand:FPMASK 3 "vsx_register_operand" "")])
> >  (match_operand:V2DI 4 "all_ones_constant" "")
> >  (match_operand:V2DI 5 "zero_constant" "")))]
> >"TARGET_P9_MINMAX"
> > 

Re: [patch] OpenMP: Fortran - support omp flush's memorder clauses

[Bernhard Reutner-Fischer via Gcc-patches]

On 22 October 2020 17:02:36 CEST, Jakub Jelinek via Gcc-patches 
 wrote:
>On Thu, Oct 22, 2020 at 04:52:10PM +0200, Tobias Burnus wrote:
>> +  else
>> +{
>> +  enum memmodel mo = MEMMODEL_LAST;
>> +  switch (code->ext.omp_clauses->memorder)
>> +{
>> +case OMP_MEMORDER_ACQ_REL: mo = MEMMODEL_ACQ_REL; break;
>> +case OMP_MEMORDER_RELEASE: mo = MEMMODEL_RELEASE; break;
>> +case OMP_MEMORDER_ACQUIRE: mo = MEMMODEL_ACQUIRE; break;
>> +case OMP_MEMORDER_LAST: gcc_unreachable (); break;
>
>I'd probably use
>   default: gcc_unreachable (); break;
>here instead.
>Otherwise LGTM, thanks.

I have one trivial remark though. In

+{
+  if (gfc_match ("acq_rel") == MATCH_YES)
+   mo = OMP_MEMORDER_ACQ_REL;
+  else if (gfc_match ("release") == MATCH_YES)
+   mo = OMP_MEMORDER_RELEASE;
+  else if (gfc_match ("acquire") == MATCH_YES)
+   mo = OMP_MEMORDER_ACQUIRE;
+  else
+   {
+ gfc_error ("Expected AQC_REL, RELEASE, or ACQUIRE at %C");

I suggest s/AQC/ACQ/
LGTM too, otherwise ;)

Re: New modref/ipa_modref optimization passes

[Bernhard Reutner-Fischer via Gcc-patches]

On 16 October 2020 11:20:23 CEST, Richard Biener  wrote:

>IMHO the cleanest way would be to swap the CAF token field and
>the dim[] field (which is an ABI change for -fcoarray)

I think coarrays are new anyway so I suppose an ABI break is fine?

Re: [PATCH 2/2] Add IEEE 128-bit min/max support on PowerPC

[Bernhard Reutner-Fischer via Gcc-patches]

On 14 April 2021 21:01:15 CEST, Segher Boessenkool  
wrote:

>> > > --- /dev/null
>> > > +++ b/gcc/testsuite/gcc.target/powerpc/float128-cmove.c
>> > > @@ -0,0 +1,93 @@
>> > > +/* { dg-do compile } */
>> > > +/* { dg-require-effective-target ppc_float128_hw } */
>> > > +/* { dg-require-effective-target power10_ok } */
>> > > +/* { dg-options "-mdejagnu-cpu=power10 -O2" } */
>> > > +/* { dg-final { scan-assembler {\mxscmpeq[dq]p\M} } } */
>> > > +/* { dg-final { scan-assembler {\mxxpermdi\M} } } */
>> > > +/* { dg-final { scan-assembler {\mxxsel\M}} } */
>> > > +/* { dg-final { scan-assembler-not {\mxscmpu[dq]p\M}  } } */
>> > > +/* { dg-final { scan-assembler-not {\mfcmp[uo]\M} } } */
>> > > +/* { dg-final { scan-assembler-not {\mfsel\M} } } */
>> 
>> I'd have expected scan-assembler-times fwiw.
>
>For what?  scan-assembler-not *is* scan-assembler-times, in effect (but
>simpler of course, and it does work with capturing parens).

I meant -times for the occurrences of scan-assembler, not the -not, in case 
that wasn't clear.

>Having too strict checks for generated code means no end to having to
>update many testcases when we have very small changes in the compiler.
>It's a balancing act.  But maybe some -times would be good here, dunno.
>
>> > > +__float128
>> > > +eq_f128_d (__float128 a, __float128 b, double x, double y)
>> > > +{
>> > > +  return (x != y) ? a : b;
>> > > +}
>> 
>> I would think the above should be == since it's named eq_ and
>> the body would be redundant to ne_f128_d below as is.
>
>Good spot :-)

Well -times would maybe have caught exactly this I suppose.

I know the exact count can be cumbersome to maintain, but in this very specific 
case which checks exactly the desired instruction it may be appropriate.

Just saying, prompted by the typo..
thanks,

Re: [PATCH,FORTRAN 00/29] Move towards stringpool, part 1

[Bernhard Reutner-Fischer via Gcc-patches]

On Fri, 7 Sep 2018 10:30:30 +0200
Bernhard Reutner-Fischer  wrote:

> On Thu, 6 Sep 2018 at 03:25, Jerry DeLisle  wrote:
> >
> > On 09/05/2018 07:57 AM, Bernhard Reutner-Fischer wrote:  
> > > Hi,
> > >
> > > The fortran frontend still uses stack-based handling of (symbol) names
> > > with fixed-sized buffers. Furthermore these buffers often are too small
> > > when dealing with F2003 identifiers which can be up to, including 63
> > > bytes long.
> > >
> > > Other frontends use the stringpool since many years.
> > > This janitorial series is a first step towards using the stringpool in
> > > the frontend.
> > > Consequently this allows us to use pointer-comparison to see if two
> > > given "names" are identical instead of doing lots and lots of string
> > > comparisons.
> > >
> > >
> > > Part 1 switches most of the fortran FE. An eventual part 2 would
> > > continue to switch the few remaining stack-based identifier
> > > manipulations to use the stringpool. My initial plan was to also see if
> > > switching gfc_symtree from treap to a hash_map would bring us any
> > > measurable benefit, but that, too, is left for an eventual part 2.
> > >
> > > Bootstrapped and regtested on x86_64-foo-linux.
> > >
> > > I'd appreciate if someone could double check for regressions on other
> > > setups. Git branch:
> > > https://gcc.gnu.org/git/?p=gcc.git;a=log;h=refs/heads/aldot/fortran-fe-stringpool
> > >  
> >
> > I am not so git savvy as I would like. If you could send me a single
> > patch to trunk I will try to test on a FreeBSD system. It is usually a
> > bit more picky than linux..  
> 
> I've just encountered a regression in module writing, let me have a
> looksie at that first so i don't waste your time.
> 
> PS: You'd:
> git clone git://gcc.gnu.org/git/gcc.git ~/src/gcc-9.0-stringpool
> cd ~/src/gcc-9.0-stringpool
> git checkout aldot/fortran-fe-stringpool
> # this should output:
> # Branch aldot/fortran-fe-stringpool set up to track remote branch
> aldot/fortran-fe-stringpool from origin.
> # if your git client is too old, then do it manually:
> # git checkout -b aldot/fortran-fe-stringpool --track
> origin/aldot/fortran-fe-stringpool
> mkdir -p ~/obj/gcc-9.0-stringpool
> cd !$
> ../../src/gcc-9.0-stringpool/configure --prefix /opt/. && make
> install && make -k check;# as usual
> 
> I'll send you a full patch when i had a chance to track down the
> module writing bug.

IIRC i fixed the abovementioned bug and pushed it to the branch.

Unfortunately the abovementioned personal git branch somehow was
deleted in the meantime.

Now i was about to rebase my local tree in the light of the upcoming
GCC-11 release and i'm confronted with a couple of conflicts that stem
from /me touching all these spots that are troublesome _and_ slow (due
to gazillions of strcmp calls to match identifiers; Part of the .plan
was to see if sacrificing a little bit of memory to maintain hash_map of
identifier_pointers would pay off WRT way faster comparison -- pointer
cmp versus strcmp as noted above. ISTR that's what other frontends do
since decades and i think it would make sense for the fortran FE too).

Iff there is any interest in eventually accepting abovementioned
direction into the tree, either part1 only which has merity on his
own - as you can see from recent asan induced tweaks to our identifier
handling - or also part 2 which encompasses the endavour to speedup the
actual matching of identifiers and hence speed up some small parts of
the frontend then i'm willing to rebase the series.

Of course if nobody thinks that the proposed direction is the right
thing to do then i will happily delete the effort so far. Saves quite
some of my sparetime.
For posteriority, a previous series of part1 was archived here:
https://gcc.gnu.org/pipermail/gcc-patches/2018-September/thread.html#506111

As can bee seen in the series and was in part explained in
https://gcc.gnu.org/pipermail/gcc-patches/2018-September/506849.html
this series changes module format a little bit. Specifically, several
spots do (or did back then, at least) use NULL module names in certain
situations in a non-obvious way (i wouldn't dare to say hackish for
i do not remember the gory details offhand i admit). I believe i
initially wrote the series even before submodules but AFAIR they did
not interfere in any noticable way. Modules were misbehaving but
submodules not, IIRC.

The fact that i need to change the module content was the main reason
why i did not apply the batch back then, even if Jerry was kind
enough to OK the submission as is, as you can see in above thread.
So i again reach out for consensus so to maybe get this improvement
into GCC-12 iff deemed appropriate and useful. Or i'll just drop it for
good.

thanks,

[PATCH] config/i386: Commentary typo fix

[Bernhard Reutner-Fischer via Gcc-patches]

From: Bernhard Reutner-Fischer 

gcc/ChangeLog:

* config/i386/x86-tune-sched-bd.c (dispatch_group): Commentary
typo fix.
---
 gcc/config/i386/x86-tune-sched-bd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/config/i386/x86-tune-sched-bd.c 
b/gcc/config/i386/x86-tune-sched-bd.c
index ad0edf713f5..be38e48b271 100644
--- a/gcc/config/i386/x86-tune-sched-bd.c
+++ b/gcc/config/i386/x86-tune-sched-bd.c
@@ -67,7 +67,7 @@ along with GCC; see the file COPYING3.  If not see
 #define BIG 100
 
 
-/* Dispatch groups.  Istructions that affect the mix in a dispatch window.  */
+/* Dispatch groups.  Instructions that affect the mix in a dispatch window.  */
 enum dispatch_group {
   disp_no_group = 0,
   disp_load,
-- 
2.31.1

Re: [PATCH] Use STATIC_ASSERT for OVL_OP_MAX.

[Bernhard Reutner-Fischer via Gcc-patches]

On Thu, 22 Apr 2021 14:28:24 -0600
Martin Sebor via Gcc-patches  wrote:

> > enum E { e = 5 };
> > struct A { E e: 3; };
> > 
> > constexpr int number_of_bits ()
> > {
> > A a = { };
> > a.e = (E)-1;
> > return 32 - __builtin_clz(a.e);
> > }
> >   
> 
> I had the same thought about using clz.  It works in this case but
> not in if one of the enumerators is negative, or if the underlying
> type is signed.

or for -fshort-enums in this case as is.

Re: [Patch] Fortran: Async I/O - avoid unlocked unlocking [PR100352]

[Bernhard Reutner-Fischer via Gcc-patches]

On Sat, 1 May 2021 14:59:01 +0200
Tobias Burnus  wrote:

> As this patch is rather obvious, I intent to commit it tomorrow
> as obvious, unless there is an OK earlier or other comments :-)
> (And backport to GCC 11 in a couple of days.)
> 
> Before PR100352 (r11-7647),
> st_write_done did:
>st_write_done_worker (dtd)
>unlock_unit (dtp->u.p.current_unit);
> 
> but st_write_done_worker did:
> LOCK (&unit_lock);
> newunit_free (dtp->common.unit);
> UNLOCK (&unit_lock);
> 
> And this locking could lead to a deadlock.
> 
> Hence, 'unlock_unit' has been moved to st_write_done_worker
> before the LOCK (&unit_lock).
> 
> That solved the issue, but async I/O does not use this lock
> but, in async.c's  async_io():
>LOCK (&au->lock);
>st_write_done_worker (au->pdt);
>UNLOCK (&au->io_lock);
> 
> Which worked before the previous patch fine, but now
> there is a bogus  unlock_unit() alias UNLOCK (&u->lock);
> although the unit is not locked.
> 
> Solution: Guard the unlock_unit.

Doesn't look all that pretty TBH.
Doesn't it suggest that the worker's callers should eventually take
care of the locking (and newunit_free()ing) ?
I.e. have the workers return a bool whether to free the newunit or not.

But then, neither did i look closely nor do i volunteer to provide a
fix..
HTH,

Re: [PATCH] ipa: Avoid invalid gimple when IPA-CP and IPA-SRA disagree on types (108384)

[Bernhard Reutner-Fischer via Gcc-patches]

On 3 February 2023 12:35:32 CET, Richard Biener via Gcc-patches
>
>I think it's OK as-is given this explanation.
>

s/derefernce/dereference/

thanks,

Re: [PATCH 8/8] OpenMP: Fortran "!$omp declare mapper" support

[Bernhard Reutner-Fischer via Gcc-patches]

On Tue, 5 Sep 2023 12:28:28 -0700
Julian Brown  wrote:

> +  static bool
> +  equal (const omp_name_type &a,
> +  const omp_name_type &b)
> +  {
> +if (a.name == NULL_TREE && b.name == NULL_TREE)
> +  return a.type == b.type;

I'm curious if (and why) the type comparison above is safe and does not
use gfc_compare_types () ?

thanks,

> +else if (a.name == NULL_TREE || b.name == NULL_TREE)
> +  return false;
> +else
> +  return a.name == b.name && gfc_compare_types (a.type, b.type);
> +  }

Re: [PATCH 2/5] c++: Set the locus of the function result decl

[Bernhard Reutner-Fischer via Gcc-patches]

On Thu, 17 Nov 2022 18:52:36 -0500
Jason Merrill  wrote:

> On 11/17/22 14:02, Bernhard Reutner-Fischer wrote:
> > On Thu, 17 Nov 2022 09:53:32 -0500
> > Jason Merrill  wrote:

> >> Instead, you want to copy the location for instantiations, i.e. check
> >> DECL_TEMPLATE_INSTANTIATION instead of !DECL_USE_TEMPLATE.  
> > 
> > No, that makes no difference.  
> 
> Hmm, when I stop there when processing the instantiation the template's 
> DECL_RESULT has the right location information, e.g. for
> 
> template  int f() { return 42; }
> 
> int main()
> {
>f();
> }
> 
> #1  0x00f950e8 in instantiate_body (pattern= 0x77ff5080 f>, args=, d= 0x7fffe971e600 f>, nested_p=false) at /home/jason/gt/gcc/cp/pt.cc:26470
> #0  start_preparsed_function (decl1=, 
> attrs=, flags=1) at /home/jason/gt/gcc/cp/decl.cc:17252
> (gdb) p expand_location (input_location)
> $13 = {file = 0x4962370 "wa.C", line = 1, column = 24, data = 0x0, sysp 
> = false}
> (gdb) p expand_location (DECL_SOURCE_LOCATION (DECL_RESULT 
> (DECL_TEMPLATE_RESULT (DECL_TI_TEMPLATE (decl1)
> $14 = {file = 0x4962370 "wa.C", line = 1, column = 20, data = 0x0, sysp 
> = false}

Yes, that works. Sorry if i was not clear: The thing in the cover
letter in this series does not work, the mini_vector reduced testcase
from the libstdc++-v3/include/ext/bitmap_allocator.h.
class template member function return type location, would that be it?

AFAIR the problem was that that these member functions get their result
decl late. When they get them, there are no
declspecs->locations[ds_type_spec] around anywhere to tuck that on the
resdecl. While the result decl is clear, there is no obvious way where
to store the ds_type_spec (somewhere in the template, as you told me).

Back then I tried moving the resdecl building from
start_preparsed_function to grokfndecl but that did not work out easily
IIRC and i ultimately gave up to move stuff around rather blindly.
I also tried to find a spot where i could store the ds_type_spec locus
somewhere in grokmethod, but i think the problem was the same, i had
just the type where i cannot store a locus and did not find a place
where i could smuggle the locus along.

So, to make that clear. Your template function (?) works:

$ XXX=1 ./xg++ -B. -S -o /dev/null ../tmp4/return-narrow-2j.cc 
../tmp4/return-narrow-2j.cc: In function ‘int f()’:
../tmp4/return-narrow-2j.cc:1:20: warning: result decl locus sample
1 | template  int f() { return 42; }
  |^~~
  |the return type
../tmp4/return-narrow-2j.cc: In function ‘int main()’:
../tmp4/return-narrow-2j.cc:3:1: warning: result decl locus sample
3 | int main()
  | ^~~
  | the return type
../tmp4/return-narrow-2j.cc: In instantiation of ‘int f() [with T = int]’:
../tmp4/return-narrow-2j.cc:5:10:   required from here
../tmp4/return-narrow-2j.cc:1:20: warning: result decl locus sample
1 | template  int f() { return 42; }
  |^~~
  |the return type


The class member fn not so much (IMHO, see attached):

$ XXX=1 ./xg++ -B. -S -o /dev/null ../tmp4/return-narrow-2.cc 
../tmp4/return-narrow-2.cc: In member function ‘const long unsigned int 
__mini_vector<  >::_M_space_left()’:
../tmp4/return-narrow-2.cc:9:3: warning: result decl locus sample
9 |   { return _M_finish != 0; }
  |   ^
  |   the return type
../tmp4/return-narrow-2.cc: In instantiation of ‘const long unsigned int 
__mini_vector<  >::_M_space_left() [with 
 = std::pair]’:
../tmp4/return-narrow-2.cc:11:17:   required from here
../tmp4/return-narrow-2.cc:9:3: warning: result decl locus sample
9 |   { return _M_finish != 0; }
  |   ^
  |   the return type
../tmp4/return-narrow-2.cc: In instantiation of ‘const long unsigned int 
__mini_vector<  >::_M_space_left() [with 
 = int]’:
../tmp4/return-narrow-2.cc:12:17:   required from here
../tmp4/return-narrow-2.cc:9:3: warning: result decl locus sample
9 |   { return _M_finish != 0; }
  |   ^
  |   the return type


> 
> > But really I'm not interested in the template case, i only mentioned
> > them because they don't work and in case somebody wanted to have correct
> > locations.
> > I remember just frustration when i looked at those a year ago.  
> 
> I'd like to get the template case right while we're looking at it.  I 
> guess I can add that myself if you're done trying.
> 
> > Is the hunk for normal functions OK for trunk?  
> 
> You also need a testcase for the desired behavior, with e.g.
> { dg-error "23:" }

I'd have to think about how to test that with trunk, yes.
There are no existing warnings that want to point to the return type,
are there?

Maybe a g++.dg/plugin/result_decl_plugin.c then.

set plugin_test_list [list
hmz. That strikes me as not all that flexible.
We could glob *_plugin.[cC][c]*, and have foo_plugin.lst contain it's
files. Whatever.

thanks,
diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
index d28889ed865..e0f057

Re: [PATCH 2/5] c++: Set the locus of the function result decl

[Bernhard Reutner-Fischer via Gcc-patches]

On Fri, 18 Nov 2022 11:06:29 -0500
Jason Merrill  wrote:

> Ah, so the problem is deferred parsing of methods, rather than 
> templates.  Building the DECL_RESULT sooner does seem like the right 
> approach to handling that, whether that's in grokfndecl or grokmethod.

> >> I'd like to get the template case right while we're looking at it.  I
> >> guess I can add that myself if you're done trying.

Please do, i'd be glad if you could take care of these locations.
It icks me that they are wrong, and be it just for the sake of QOI :)

> >>> Is the hunk for normal functions OK for trunk?  
> >>
> >> You also need a testcase for the desired behavior, with e.g.
> >> { dg-error "23:" }  
> > 
> > I'd have to think about how to test that with trunk, yes.
> > There are no existing warnings that want to point to the return type,
> > are there?  
> 
> Good point.  Do any of your later patches add such a warning?

I didn't mean to have that -Wtype-demotion applied in it's current
form, or at all, so no. I was curious if anybody liked the idea of
pointing out such code though. I've had no feedback but everybody is or
was busy with end of stage3 and real work, so that's expected. The only
real purpose i had for it was to find places in the Fortran FE that
could use narrower types, bools for the most part.
IMHO it would be a nice thing to have, but then, embedded software
usually is cautious to use sensible types in the first place and the
rest doesn't really care anyway, supposedly.

Maybe it would have made more sense to just do an IPA pass that does the
demotion silently where it's feasable.

As to the test, i don't think these locations in the c++ FE are changed
all that often, so chances are rather low that they would be broken
once in.
So, short of trying to use the result decl locus for any existing
-Wreturn-type, -Waggregate-return, -Wno-return-local-addr,
-Wsuggest-attribute=[pure|const|noreturn|format|malloc] or another
existing warning that would be concerned, we could, as said, have a
plugin with fix-it hints and ideally -fdiagnostics-generate-patch to
test these bits. Patch generation has the advantage that it will ICE
more often than not if asked to generate patches for locations that
have a negative relative start (think: memcpy(...,..., -7)), which you
can get easily if the locations are off IMHO.

> > Maybe a g++.dg/plugin/result_decl_plugin.c then.

Re: [PATCH 2/5] c++: Set the locus of the function result decl

[Bernhard Reutner-Fischer via Gcc-patches]

Hi Jason!

Possible test.

An existing test might be to equip the existing warning for
bool unsigned double meh(void) {return 0;}
with a fix-it hint instead of the brief
 error: two or more data types in declaration of ‘meh’.
Likewise for
bool unsigned meh(void) {return 0;}
 error: ‘unsigned’ specified with ‘bool’

so we wouldn't need a plugin, and it might even be useful? ;)

cheers,

* g++.dg/plugin/plugin.exp: Add new test.
* g++.dg/plugin/result-decl-plugin-test-1.C: New test.
* g++.dg/plugin/result-decl-plugin-test-2.C: New test.
* g++.dg/plugin/result_decl_plugin.C: New test.
---
 gcc/testsuite/g++.dg/plugin/plugin.exp|  3 +
 .../g++.dg/plugin/result-decl-plugin-test-1.C | 28 +
 .../g++.dg/plugin/result-decl-plugin-test-2.C | 61 +++
 .../g++.dg/plugin/result_decl_plugin.C| 57 +
 4 files changed, 149 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/plugin/result-decl-plugin-test-1.C
 create mode 100644 gcc/testsuite/g++.dg/plugin/result-decl-plugin-test-2.C
 create mode 100644 gcc/testsuite/g++.dg/plugin/result_decl_plugin.C

diff --git a/gcc/testsuite/g++.dg/plugin/plugin.exp 
b/gcc/testsuite/g++.dg/plugin/plugin.exp
index b5fb42fa77a..f2b526b4704 100644
--- a/gcc/testsuite/g++.dg/plugin/plugin.exp
+++ b/gcc/testsuite/g++.dg/plugin/plugin.exp
@@ -80,6 +80,9 @@ set plugin_test_list [list \
  show-template-tree-color-labels.C \
  show-template-tree-color-no-elide-type.C } \
 { comment_plugin.c comments-1.C } \
+{ result_decl_plugin.C \
+  result-decl-plugin-test-1.C \
+  result-decl-plugin-test-2.C } \
 ]
 
 foreach plugin_test $plugin_test_list {
diff --git a/gcc/testsuite/g++.dg/plugin/result-decl-plugin-test-1.C 
b/gcc/testsuite/g++.dg/plugin/result-decl-plugin-test-1.C
new file mode 100644
index 000..bd323181d70
--- /dev/null
+++ b/gcc/testsuite/g++.dg/plugin/result-decl-plugin-test-1.C
@@ -0,0 +1,28 @@
+/* Verify that class member functions result decl have the correct location.  
*/
+// { dg-options "-fdiagnostics-generate-patch" }
+namespace std { template < typename, typename > struct pair; }
+template < typename > struct __mini_vector
+{
+  int _M_finish;
+  const
+  unsigned long
+  __attribute__((deprecated))
+  _M_space_left()
+  { return _M_finish != 0; }
+};
+ template class __mini_vector< std::pair< long, long > >;
+ template class __mini_vector< int >;
+#if 0
+{ dg-begin-multiline-output "" }
+@@ -5,7 +5,7 @@ template < typename > struct __mini_vect
+ {
+   int _M_finish;
+   const
+-  unsigned long
++  bool
+   __attribute__((deprecated))
+   _M_space_left()
+   { return _M_finish != 0; }
+
+{ dg-end-multiline-output "" }
+#endif
diff --git a/gcc/testsuite/g++.dg/plugin/result-decl-plugin-test-2.C 
b/gcc/testsuite/g++.dg/plugin/result-decl-plugin-test-2.C
new file mode 100644
index 000..385a7ef482f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/plugin/result-decl-plugin-test-2.C
@@ -0,0 +1,61 @@
+/* Verify that template functions result decl have the correct location.  */
+// { dg-options "-fdiagnostics-generate-patch" }
+template 
+int
+f()
+{
+  return 42;
+}
+int main()
+{
+   f();
+}
+unsigned long long huh(void)
+{
+  return 1ULL;
+}
+#if 0
+{ dg-begin-multiline-output "" }
+g++.dg/plugin/result-decl-plugin-test-2.C:4:1: warning: Function ‘f’ result 
location
+4 | int
+  | ^~~
+  | bool
+g++.dg/plugin/result-decl-plugin-test-2.C:9:1: warning: Function ‘main’ result 
location
+9 | int main()
+  | ^~~
+  | bool
+g++.dg/plugin/result-decl-plugin-test-2.C:13:28: warning: Function ‘huh’ 
result location
+   13 | unsigned long long huh(void)
+  |^
+  |bool
+g++.dg/plugin/result-decl-plugin-test-2.C: In instantiation of ‘int f() [with 
T = int]’:
+g++.dg/plugin/result-decl-plugin-test-2.C:11:10:   required from here
+g++.dg/plugin/result-decl-plugin-test-2.C:4:1: warning: Function ‘f’ result 
location
+4 | int
+  | ^~~
+  | bool
+--- g++.dg/plugin/result-decl-plugin-test-2.C
 g++.dg/plugin/result-decl-plugin-test-2.C
+@@ -1,16 +1,16 @@
+ /* Verify that template functions result decl have the correct location.  */
+ // { dg-options "-fdiagnostics-generate-patch" }
+ template 
+-int
++bool
+ f()
+ {
+   return 42;
+ }
+-int main()
++bool main()
+ {
+f();
+ }
+-unsigned long long huh(void)
++unsigned long long huh(voidbool
+ {
+   return 1ULL;
+ }
+{ dg-end-multiline-output "" }
+#endif
+// Note: f() should not +bbool with an off-by-one for the start 'b' !
diff --git a/gcc/testsuite/g++.dg/plugin/result_decl_plugin.C 
b/gcc/testsuite/g++.dg/plugin/result_decl_plugin.C
new file mode 100644
index 000..40f54a6acfe
--- /dev/null
+++ b/gcc/testsuite/g++.dg/plugin/result_decl_plugin.C
@@ -0,0 +1,57 @@
+/* A plugin example that points at the location of function decl result decl */
+/* This file is part of GCC */
+/* { dg-options "-O" } */
+#

Re: [PATCH 2/5] c++: Set the locus of the function result decl

[Bernhard Reutner-Fischer via Gcc-patches]

Hi Jason!

The "meh" of result-decl-plugin-test-2.C should likely be omitted,
grokdeclarator would need some changes to add richloc hints and we would not
be able to make a reliable guess what to remove precisely.
C.f. /* Check all other uses of type modifiers.  */
Furthermore it is unrelated to DECL_RESULT so not of direct interest
here. The other tests in test-2.C, f() and huh() should work though.

I don't know if it's acceptable to change ipa-pure-const to make the
missing noreturn warning more precise and emit a fixit-hint. At least it
would be a real test for the DECL_RESULT and would spare us the plugin.

HTH,

gcc/cp/ChangeLog:

* decl.cc (start_preparsed_function): Set the result decl source
location to the location of the typespec.
(start_function): Likewise.

gcc/ChangeLog:

* ipa-pure-const.cc (suggest_attribute): Add fixit-hint for the
noreturn attribute.

gcc/testsuite/ChangeLog:

* c-c++-common/pr68833-1.c: Adjust noreturn warning line number.
* gcc.dg/noreturn-1.c: Likewise.
* g++.dg/plugin/plugin.exp: Add new plugin test.
* g++.dg/other/resultdecl-1.C: New test.
* g++.dg/plugin/result-decl-plugin-test-1.C: New test.
* g++.dg/plugin/result-decl-plugin-test-2.C: New test.
* g++.dg/plugin/result_decl_plugin.C: New test.
---
 gcc/cp/decl.cc| 26 +++-
 gcc/ipa-pure-const.cc | 14 -
 gcc/testsuite/c-c++-common/pr68833-1.c|  2 +-
 gcc/testsuite/g++.dg/other/resultdecl-1.C | 32 ++
 gcc/testsuite/g++.dg/plugin/plugin.exp|  3 +
 .../g++.dg/plugin/result-decl-plugin-test-1.C | 31 ++
 .../g++.dg/plugin/result-decl-plugin-test-2.C | 59 +++
 .../g++.dg/plugin/result_decl_plugin.C| 53 +
 gcc/testsuite/gcc.dg/noreturn-1.c |  2 +-
 9 files changed, 218 insertions(+), 4 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/other/resultdecl-1.C
 create mode 100644 gcc/testsuite/g++.dg/plugin/result-decl-plugin-test-1.C
 create mode 100644 gcc/testsuite/g++.dg/plugin/result-decl-plugin-test-2.C
 create mode 100644 gcc/testsuite/g++.dg/plugin/result_decl_plugin.C

diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
index d28889ed865..0c053b6d19f 100644
--- a/gcc/cp/decl.cc
+++ b/gcc/cp/decl.cc
@@ -17235,6 +17235,17 @@ start_preparsed_function (tree decl1, tree attrs, int 
flags)
   cp_apply_type_quals_to_decl (cp_type_quals (restype), resdecl);
 }
 
+  /* Set the result decl source location to the location of the typespec.  */
+  if (DECL_RESULT (decl1)
+  && DECL_TEMPLATE_INSTANTIATION (decl1)
+  && DECL_TEMPLATE_INFO (decl1)
+  && DECL_TI_TEMPLATE (decl1)
+  && DECL_TEMPLATE_RESULT (DECL_TI_TEMPLATE (decl1))
+  && DECL_RESULT (DECL_TEMPLATE_RESULT (DECL_TI_TEMPLATE (decl1
+  DECL_SOURCE_LOCATION (DECL_RESULT (decl1))
+   = DECL_SOURCE_LOCATION (
+   DECL_RESULT (DECL_TEMPLATE_RESULT (DECL_TI_TEMPLATE (decl1;
+
   /* Record the decl so that the function name is defined.
  If we already have a decl for this name, and it is a FUNCTION_DECL,
  use the old decl.  */
@@ -17532,7 +17543,20 @@ start_function (cp_decl_specifier_seq *declspecs,
 gcc_assert (same_type_p (TREE_TYPE (TREE_TYPE (decl1)),
 integer_type_node));
 
-  return start_preparsed_function (decl1, attrs, /*flags=*/SF_DEFAULT);
+  bool ret = start_preparsed_function (decl1, attrs, /*flags=*/SF_DEFAULT);
+
+  /* decl1 might be ggc_freed here.  */
+  decl1 = current_function_decl;
+
+  tree result;
+  /* Set the result decl source location to the location of the typespec.  */
+  if (ret
+  && TREE_CODE (decl1) == FUNCTION_DECL
+  && declspecs->locations[ds_type_spec] != UNKNOWN_LOCATION
+  && (result = DECL_RESULT (decl1)) != NULL_TREE
+  && DECL_SOURCE_LOCATION (result) == input_location)
+DECL_SOURCE_LOCATION (result) = declspecs->locations[ds_type_spec];
+  return ret;
 }
 
 /* Returns true iff an EH_SPEC_BLOCK should be created in the body of
diff --git a/gcc/ipa-pure-const.cc b/gcc/ipa-pure-const.cc
index 572a6da274f..1c80034f38d 100644
--- a/gcc/ipa-pure-const.cc
+++ b/gcc/ipa-pure-const.cc
@@ -63,6 +63,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "ipa-fnsummary.h"
 #include "symtab-thunks.h"
 #include "dbgcnt.h"
+#include "gcc-rich-location.h"
 
 /* Lattice values for const and pure functions.  Everything starts out
being const, then may drop to pure and then neither depending on
@@ -212,7 +213,18 @@ suggest_attribute (int option, tree decl, bool 
known_finite,
   if (warned_about->contains (decl))
 return warned_about;
   warned_about->add (decl);
-  warning_at (DECL_SOURCE_LOCATION (decl),
+
+  gcc_rich_location richloc (option == OPT_Wsuggest_attribute_noreturn
+? DECL_SOURCE_LOCATION (DECL_RESULT (decl))
+: DECL_

Re: [PATCH 1/2] symtab: also change RTL decl name

[Bernhard Reutner-Fischer via Gcc-patches]

On Mon, 21 Nov 2022 20:02:49 +0100
Jan Hubicka  wrote:

> > Hi Honza, Ping.
> > Regtests cleanly for c,fortran,c++,ada,d,go,lto,objc,obj-c++
> > Ok?
> > I'd need this for attribute target_clones for the Fortran FE.  
> Sorry for delay here.
> > >  void
> > > @@ -303,6 +301,10 @@ symbol_table::change_decl_assembler_name (tree decl, 
> > > tree name)
> > >   warning (0, "%qD renamed after being referenced in assembly", decl);
> > >  
> > >SET_DECL_ASSEMBLER_NAME (decl, name);
> > > +  /* Set the new name in rtl.  */
> > > +  if (DECL_RTL_SET_P (decl))
> > > + XSTR (XEXP (DECL_RTL (decl), 0), 0) = IDENTIFIER_POINTER (name);  
> 
> I am not quite sure how safe this is.  We generally produce DECL_RTL
> when we produce assembly file.  So if DECL_RTL is set then we probably
> already output the original function name and it is too late to change
> it.

AFAICS we make_decl_rtl in the fortran FE in trans_function_start.

> 
> Also RTL is shared so changing it in-place is going to rewrite all the
> existing RTL expressions using it.
> 
> Why the DECL_RTL is produced for function you want to rename?

I think the fortran FE sets it quite early when lowering a function.
Later, when the ME creates the target_clones, it wants to rename the
initial function to initial_fun.default for the default target.
That's where the change_decl_assembler_name is called (only on the
decl).
But nobody changes the RTL name, so the ifunc (which should be the
initial, unchanged name) is properly emitted but
assemble_start_function uses the same, unchanged, initial fnname it
later obtains by get_fnname_from_decl which fetches the (wrong) initial
name where it should use the .default target name.
See
https://gcc.gnu.org/pipermail/gcc-patches/2022-November/605081.html

I'm open to other suggestions to make this work in a different way, of
course. Maybe we're missing some magic somewhere that might share the
name between the fndecl and the RTL XSTR so the RTL is magically
updated by that single SET_ECL_ASSEMBLER_NAME in
change_decl_assembler_name? But i didn't quite see where that'd be?

thanks,

> Honza
> > > +
> > >if (alias)
> > >   {
> > > IDENTIFIER_TRANSPARENT_ALIAS (name) = 1;  
> >   

Re: [PATCH 2/2] Fortran: Add attribute flatten

[Bernhard Reutner-Fischer via Gcc-patches]

On Mon, 21 Nov 2022 12:24:11 +0100
Mikael Morin  wrote:

> > --- a/gcc/fortran/decl.cc
> > +++ b/gcc/fortran/decl.cc  
> (...)
> > @@ -11849,7 +11850,9 @@ gfc_match_gcc_attributes (void)
> > if (strcmp (name, ext_attr_list[id].name) == 0)
> >   break;
> >   
> > -  if (id == EXT_ATTR_LAST)
> > +  if (strcmp (name, "flatten") == 0)
> > +   known_attr0args = true; /* Handled below.  We do not need a bit.  */  
> 
> I don't see the point to have all the attributes needing a bit except 
> one that doesn't but needs a specific handling.
> What does it look like without the 1/2 patch and if one bit is also used 
> for flatten, like the other attributes?

I've changed target_clones not to use a bit locally because it's not
needed. From my understanding, we only need the bits for attributes
that change the calling convention or the caller(at least so far, but
that does make sense to me). Remember that we store these bits in the
module. Presumably because we have to make sure that a program/module
uses the correct calling convention for a module function annotated
with such an attribute (think cdecl, stdcall, fastcall, dllimport,
dllexport, or the non-implemented regparm, sseregparm) or for
attributes that otherwise influence the callers or callees (like
deprecated or no_arg_check).

Attributes like target_clones or flatten or (probably) optimize etc, do
not influence the callees, so we really do not need to store them in
the module.

Can you think of a reason to store them nevertheless?

> > +  else if (id == EXT_ATTR_LAST)
> > {
> >   gfc_error ("Unknown attribute in !GCC$ ATTRIBUTES statement at %C");
> >   return MATCH_ERROR;  
> 
> > diff --git a/gcc/fortran/gfortran.texi b/gcc/fortran/gfortran.texi
> > index 06e4c8c00a1..be650f28b62 100644
> > --- a/gcc/fortran/gfortran.texi
> > +++ b/gcc/fortran/gfortran.texi
> > @@ -3280,6 +3280,14 @@ contains
> >   end module mymod
> >   @end smallexample
> >   
> > +@node flatten
> > +
> > +Procedures annotated with the @code{flatten} attribute have their
> > +callees inlined, if possible.  
> I'm not a native speaker, but I find this sentence confusing.
> The words of the gcc manual you are refering to seem more clear: "every 
> call inside the function is inlined, if possible".

Me neither and it was a bit too brief. I've changed this to:
Every call inside a procedure annotated with the @code{flatten} attribute
is inlined, if possible.  Please refer to
@ref{Top,,Common Function Attributes,gcc,Using the GNU Compiler Collection (GCC>
for details about the respective attribute.

Is that better?

That said, i think this whole attribute section in the manual is not
structured too well. I'd prefer to have a list of attributes like in the
"Common Function Attributes" section in the extend.texi.
Maybe it would be better to just start a new list of attributes at the
end of the current @subsection ATTRIBUTES directive, a subsubsection
with "Other attributes" and just itemize the new ones? We'd point
people to the Top docs once for further details and then just briefly
list the attributes we support. Would that be acceptable?

Many thanks for your comments!

> 
> > +Please refer to
> > +@ref{Top,,Common Function Attributes,gcc,Using the GNU Compiler Collection 
> > (GCC)}
> > +for details about the respective attribute.
> > +
> >   The attributes are specified using the syntax
> >   
> >   @code{!GCC$ ATTRIBUTES} @var{attribute-list} @code{::} 
> > @var{variable-list}  
> 

Re: [PATCH 1/2] Fortran: Cleanup struct ext_attr_t

[Bernhard Reutner-Fischer via Gcc-patches]

On Mon, 21 Nov 2022 12:08:20 +0100
Mikael Morin  wrote:

> > * gfortran.h (struct ext_attr_t): Remove middle_end_name.
> > * trans-decl.cc (add_attributes_to_decl): Move building
> > tree_list to ...
> > * decl.cc (gfc_match_gcc_attributes): ... here. Add the attribute to
> > the tree_list for the middle end.
> >   
> I prefer to not do any middle-end stuff at parsing time, so I would 
> rather not do this change.
> Not OK.

Ok, that means we should filter-out those bits that we don't want to
write to the module (right?). We've plenty of bits left, more than Dave
Love would want to have added, i hope, so that should not be much of a
concern.

What that table really wants to say is whether or not this attribute
should be passed to the ME. Would it be acceptable to remove these
duplicate strings and just have a bool/char/int that is true if it
should be lowered (in trans-decl, as before)? But now i admit it's just
bikeshedding and we can as well leave it alone for now.. It was just a
though.

thanks,

Re: [PATCH 2/2] Fortran: add attribute target_clones

[Bernhard Reutner-Fischer via Gcc-patches]

On Mon, 21 Nov 2022 20:13:40 +0100
Mikael Morin  wrote:

> Hello,
> 
> Le 09/11/2022 à 20:02, Bernhard Reutner-Fischer via Fortran a écrit :
> > Hi!
> > 
> > Add support for attribute target_clones:
> > !GCC$ ATTRIBUTES target_clones("arch1", "arch3","default") :: mysubroutine

> > +/* Internal helper to parse attribute argument list.
> > +   If REQUIRE_STRING is true, then require a string.
> > +   If ALLOW_MULTIPLE is true, allow more than one arg.
> > +   If multiple arguments are passed, require braces around them.
> > +   Returns a tree_list of arguments or NULL_TREE.  */
> > +static tree
> > +gfc_match_gcc_attribute_args (bool require_string, bool allow_multiple)

> > +   do {

> > +   } while (num_quotes % 2 && gfc_match_eos () != MATCH_YES);  
> The do-while loops are wrongly indented.
> It should be:
>do
>  {
>...
>  }
>while (...)

oops, right.

> > +   tree str = build_string (pos, name);
> > +   /* Compare with c-family/c-common.cc: fix_string_type.  */
> > +   tree i_type = build_index_type (size_int (pos));
> > +   tree a_type = build_array_type (char_type_node, i_type);
> > +   TREE_TYPE (str) = a_type;
> > +   TREE_READONLY (str) = 1;
> > +   TREE_STATIC (str) = 1;
> > +   attr_arg = build_tree_list (NULL_TREE, str);
> > +   attr_args = chainon (attr_args, attr_arg);  
> Same comment as for the flatten attribute:
> please no tree stuff out of the trans-*.cc files.

yes ok, noted. It's a pity in this context, where we purely pass a blob
on to the ME but ok.

> This includes gfortran.h, so the attribute arguments need to be carried 
> around using the front-end structures (gfc_actual_arglist for example).

That's a sane rule of thumb, yes.
Usually, the parser deals with language grammar and not with pure
passthrough remarks, so that's fair. Not so much in the case of such
attribs but i see your point :)
 
> > +  if (allow_multiple && gfc_match_char (')') != MATCH_YES)
> > +{
> > +  gfc_error ("expected ')' at %C");
> > +  return NULL_TREE;
> > +}
> > +
> > +  return attr_args;
> > +}  
> I'm not sure this function need to do all the parsing manually.
> I would rather use gfc_match_actual_arglist, or maybe implement the 
> function as a wrapper around it.
> What is allowed here?  Are non-literal constants allowed, for example 
> parameter variables?  Is line continuation supported ?

Line continuation is supported i think.
Parameter variables supposedly are or should not be supported. Why would
you do that in the context of an attribute target decl?
Either way, if the ME does not find such an fndecl, it will complain
and ignore the attribute.
I don't understand non-literal constants in this context.
This very attribute applies to decls, so the existing code supposedly
matches a comma separated list of identifiers. The usual dollar-ok
caveats apply.

As to gfc_match_actual_arglist, probably.
target_clones has
+  { "target_clones",  1, -1, true, false, false, false,
+ dummy, NULL },
with tree-core.h struct attribute_spec, so
name, min=1, max=unbounded, decl_required=yes, ...ignore...

hence applies to functions and subroutines and the like. It does take an
unbounded list of strings, isa1, isa2, isa4, default. We could add
"default" unless seen, but i'd rather want it spelled out by the user
for the user is supposed to know what she's doing, as in c or c++.
The ME has code to sanity-check the attributes, including conflicting
(ME) attributes.

The reason why i contemplated with a separate parser was that for stuff
like regparm or sseregparm, you would want to require a single number
for the equivalent of

__attribute__((regparm(3),stdcall)

which you would provide in 2 separate !GCC$ attributes i assume.

> 
> Nothing (bad) to say about the rest, but there is enough to change with 
> the above comments.

Yes, many thanks for your comments.
I think there is no other non-intrusive way to pass the data through the
frontend. So for an acceptable way this means touching quite some spots
for every single ME attribute anybody would like to add in the future.
But that's how it is.

Re: [PATCH 2/5] c++: Set the locus of the function result decl

[Bernhard Reutner-Fischer via Gcc-patches]

On 2 December 2022 20:30:56 CET, Jason Merrill  wrote:

>Here's what I'm applying:

I meant to play with it during the weekend,
looks good, many thanks for taking care of this!
cheers,

Re: [PATCH] analyzer: consider empty ranges and zero byte accesses [PR106845]

[Bernhard Reutner-Fischer via Gcc-patches]

On 11 September 2022 10:04:51 CEST, David Malcolm via Gcc-patches 
 wrote:

>> +++ b/gcc/testsuite/gcc.dg/analyzer/pr106845.c
>> @@ -0,0 +1,11 @@
>> +int buf_size;
>> +
>> +int
>> +main (void)
>> +{
>> +  char buf[buf_size];
>> +
>> +  __builtin_memset (&buf[1], 0, buf_size);
>> +
>> +  return 0;
>> +}
>
>...it took me a moment to realize that the analyzer "sees" that this is
>"main", and thus buf_size is 0.

Is this a valid assumption?

What if I have a lib (preloaded maybe) that sets it to 42?

BTW, do we handle -Wl,-init,youre_toast
where main isn't the entry point?

Just curious..
thanks,

>
>Interestingly, if I rename it to not be "main" (and thus buf_size could
>be non-zero), we still don't complain:
>  https://godbolt.org/z/PezfTo9Mz
>Presumably this is a known limitation of the symbolic bounds checking?
>
>Thanks
>Dave
>

Re: [PATCH] c++: modules ICE with typename friend declaration

[Bernhard Reutner-Fischer via Gcc-patches]

On 16 September 2022 17:54:32 CEST, Patrick Palka via Gcc-patches 
 wrote:

>+++ b/gcc/testsuite/g++.dg/modules/typename-friend_a.C
>@@ -0,0 +1,11 @@
>+// { dg-additional-options "-fmodules-ts" }
>+export module foo;
>+// { dg-module-cmi foo }
>+

If that's a constant, repeating pain, you could instrument the test suite so 
that upon seeing -fmodules-ts (or maybe a later std) it greps for export module 
and calls dg-module-cmi on those names automatically on its own.
We do the same for stuff like PCH or fortran module .mod or many other cleanup 
stuff because such manual annotations are IMHO a waste of developer resources..

Just a thought..
cheers,

Re: [PATCH 09/10] fortran: Support clobbering of variable subreferences [PR88364]

[Bernhard Reutner-Fischer via Gcc-patches]

On 17 September 2022 21:33:22 CEST, Mikael Morin  wrote:
>Le 17/09/2022 à 19:03, Thomas Koenig via Fortran a écrit :
>> 
>> Hi Mikael,
>> 
>>> This adds support for clobbering of partial variable references, when
>>> they are passed as actual argument and the associated dummy has the
>>> INTENT(OUT) attribute.
>>> Support includes array elements, derived type component references,
>>> and complex real or imaginary parts.
>>> 
>>> This is done by removing the check for lack of subreferences, which is
>>> basically a revert of r9-4911-gbd810d637041dba49a5aca3d085504575374ac6f.
>>> This removal allows more expressions than just array elements,
>>> components and complex parts, but the other expressions are excluded by
>>> other conditions: substrings are excluded by the check on expression
>>> type (CHARACTER is excluded), KIND and LEN references are rejected by
>>> the compiler as not valid in a variable definition context.
>>> 
>>> The check for scalarness is also updated as it was only valid when there
>>> was no subreference.
>> 
>> First, thanks a lot for digging into this subject. I have looked through
>> the patch series, and it looks very good so far.

I second that!
The series looks plausible IMO.

>> 
>> I have a concern about this part, though.  My understanding at the
>> time was that it is not possible to clobber an individual array
>> element, but that this clobbers anything off the pointer that this
>> is based on.
>> 
>Well, we need the middle-end guys to give a definitive answer on this topic, 
>but I think it would be a very penalizing limitation if that was the case.  I 
>have assumed that the clobber spanned the value it was applied on, neither 
>more nor less, so just the array element in case of array elements.

I would assume the same, fwiw.
Let's blame the ME iff something goes amiss then, but I doubt it will.

>> So,
>> 
>>    integer, dimension(3) :: a
>> 
>>    a(1) = 1
>>    a(3) = 3
>>    call foo(a(1))
>> 
>> would also invalidate the store to a(3).  Is my understanding correct?
>
>I think it was the case before patch 2 in in the series, because the clobber 
>was applied to the symbol decl, so in the case of the expression A(1), it was 
>applied to A which is the full array.  After patch 2, the clobber is applied 
>to the expression A(1), so the element alone.

Yep.

>> If so, I think this we cannot revert that patch (which was introduced
>> because of a regression).
>> 
>The testcase from the patch was not specifically checking lack of side-effect 
>clobbers, so I have double-checked with the following testcase, which should 
>lift your concerns.
>I propose to keep the patch with the testcase added to it.  What do you think?

I cannot approve it but the series looks good to me.

Thanks!

Re: [PATCH 09/10] fortran: Support clobbering of variable subreferences [PR88364]

[Bernhard Reutner-Fischer via Gcc-patches]

On 17 September 2022 21:50:20 CEST, Mikael Morin  wrote:
>Le 17/09/2022 à 21:33, Mikael Morin a écrit :
>> The testcase from the patch was not specifically checking lack of 
>> side-effect clobbers, so I have double-checked with the following testcase, 
>> which should lift your concerns.
>> 
>The dump matches didn’t fail as expected with patch 2/10 reversed.
>This testcase should be better.

! { dg-final { scan-tree-dump-times "456" 0 "optimized" { target __OPTIMIZE__ } 
} }

I'd spell this as scan-tree-dump-not, fwiw.

That said, plain scan-tree-dump is usually only viable in arch influenced 
checks which in fortran we do not usually have. Here, we should for the most 
part use -not or a specific -times.

I think you had a check for integer(kind=4) in there, too, which might not work 
all that well for -fdefault-integer-8 or, for the corresponding real scan, 
-fdefault-real-8, eventually. Easily tweaked on top if anyone (certainly will) 
complain later on, though..

fore, either way, I'd say :-)
thanks,

Re: [patch] libgompd: Add thread handles

[Bernhard Reutner-Fischer via Gcc-patches]

On Tue, 27 Sep 2022 03:20:51 +0200
Ahmed Sayed Mousse via Gcc-patches  wrote:

> diff --git a/libgomp/Makefile.am b/libgomp/Makefile.am
> index 6d913a93e7f..23f5bede1bf 100644
> --- a/libgomp/Makefile.am
> +++ b/libgomp/Makefile.am
> @@ -94,7 +94,7 @@ libgomp_la_SOURCES = alloc.c atomic.c barrier.c critical.c 
> env.c error.c \
>   priority_queue.c affinity-fmt.c teams.c allocator.c oacc-profiling.c \
>   oacc-target.c ompd-support.c
>  
> -libgompd_la_SOURCES = ompd-init.c ompd-helper.c ompd-icv.c
> +libgompd_la_SOURCES = ompd-init.c ompd-helper.c ompd-icv.c ompd-threads.c
>  
>  include $(top_srcdir)/plugin/Makefrag.am
>  
> diff --git a/libgomp/Makefile.in b/libgomp/Makefile.in
> index 40f896b5f03..7acdcbf31d5 100644
> --- a/libgomp/Makefile.in
> +++ b/libgomp/Makefile.in
> @@ -233,7 +233,8 @@ am_libgomp_la_OBJECTS = alloc.lo atomic.lo barrier.lo 
> critical.lo \
>   affinity-fmt.lo teams.lo allocator.lo oacc-profiling.lo \
>   oacc-target.lo ompd-support.lo $(am__objects_1)
>  libgomp_la_OBJECTS = $(am_libgomp_la_OBJECTS)
> -am_libgompd_la_OBJECTS = ompd-init.lo ompd-helper.lo ompd-icv.lo
> +am_libgompd_la_OBJECTS = ompd-init.lo ompd-helper.lo ompd-icv.lo \
> + ompd-threads.lo
>  libgompd_la_OBJECTS = $(am_libgompd_la_OBJECTS)
>  AM_V_P = $(am__v_P_@AM_V@)
>  am__v_P_ = $(am__v_P_@AM_DEFAULT_V@)
> @@ -583,7 +584,7 @@ libgomp_la_SOURCES = alloc.c atomic.c barrier.c 
> critical.c env.c \
>   oacc-async.c oacc-plugin.c oacc-cuda.c priority_queue.c \
>   affinity-fmt.c teams.c allocator.c oacc-profiling.c \
>   oacc-target.c ompd-support.c $(am__append_7)
> -libgompd_la_SOURCES = ompd-init.c ompd-helper.c ompd-icv.c
> +libgompd_la_SOURCES = ompd-init.c ompd-helper.c ompd-icv.c ompd-threads.c
>  
>  # Nvidia PTX OpenACC plugin.
>  @PLUGIN_NVPTX_TRUE@libgomp_plugin_nvptx_version_info = -version-info 
> $(libtool_VERSION)
> @@ -801,6 +802,7 @@ distclean-compile:
>  @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/ompd-icv.Plo@am__quote@
>  @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/ompd-init.Plo@am__quote@
>  @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/ompd-support.Plo@am__quote@
> +@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/ompd-threads.Plo@am__quote@
>  @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/ordered.Plo@am__quote@
>  @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/parallel.Plo@am__quote@
>  @AMDEP_TRUE@@am__include@ 
> @am__quote@./$(DEPDIR)/priority_queue.Plo@am__quote@
> diff --git a/libgomp/ompd-support.c b/libgomp/ompd-support.c
> index 27c5ad148e0..5b1afd37788 100644
> --- a/libgomp/ompd-support.c
> +++ b/libgomp/ompd-support.c
> @@ -33,6 +33,8 @@ const unsigned short gompd_sizeof_gomp_thread_handle
>__attribute__ ((used)) OMPD_SECTION = 0;
>  #endif
>  
> +unsigned long gompd_thread_initial_tls_bias __attribute__ ((used));
> +
>  /* Get offset of the member m in struct t.  */
>  #define gompd_get_offset(t, m) \
>const unsigned short gompd_access_##t##_##m __attribute__ ((used)) \
> @@ -67,6 +69,11 @@ gompd_load (void)
>gompd_state |= OMPD_ENABLED;
>ompd_dll_locations = &ompd_dll_locations_array[0];
>ompd_dll_locations_valid ();
> +
> +  #if defined(LIBGOMP_USE_PTHREADS) && !defined(GOMP_NEEDS_THREAD_HANDLE)
> +  gompd_thread_initial_tls_bias = (unsigned long) ((char *) &gomp_tls_data
> +- (char *) pthread_self ());
> +  #endif
>  }
>  
>  #ifndef __ELF__
> diff --git a/libgomp/ompd-threads.c b/libgomp/ompd-threads.c
> new file mode 100644
> index 000..723ef740181
> --- /dev/null
> +++ b/libgomp/ompd-threads.c
> @@ -0,0 +1,222 @@
> +/* Copyright (C) The GNU Toolchain Authors.
> +   Contributed by Ahmed Sayed .
> +   This file is part of the GNU Offloading and Multi Processing Library
> +   (libgomp).
> +
> +   Libgomp 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.
> +
> +   Libgomp 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.
> +
> +   Under Section 7 of GPL version 3, you are granted additional
> +   permissions described in the GCC Runtime Library Exception, version
> +   3.1, as published by the Free Software Foundation.
> +
> +   You should have received a copy of the GNU General Public License and
> +   a copy of the GCC Runtime Library Exception along with this program;
> +   see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
> +   .  */
> +
> +/* This file contains the implementation of functions defined in
> +   Section 5.5 ThreadHandles. */
> +
> +
> +#include "ompd-helper.h"
> +
> +ompd_rc_t
> +ompd_get_thread_in_parallel (ompd_parallel_hand

Re: c++: Add DECL_NTTP_OBJECT_P lang flag

[Bernhard Reutner-Fischer via Gcc-patches]

On Wed, 28 Sep 2022 16:44:29 -0400
Nathan Sidwell via Gcc-patches  wrote:

> +   else if (TREE_CODE (arg) == VAR_DECL && DECL_NTTP_OBJECT_P (arg))

Cosmetics, but I think the first part of the condition could be spelled
as VAR_P (arg)

thanks,

Re: [PATCH] Refine ranges using relations in GORI.

[Bernhard Reutner-Fischer via Gcc-patches]

Hi Andrew!

On Thu, 29 Sep 2022 18:36:53 -0400
Andrew MacLeod via Gcc-patches  wrote:

> diff --git a/gcc/gimple-range-gori.cc b/gcc/gimple-range-gori.cc
> index 57a7e820749..b37d03cddda 100644
> --- a/gcc/gimple-range-gori.cc
> +++ b/gcc/gimple-range-gori.cc
> @@ -934,6 +934,115 @@ gori_compute::compute_logical_operands (vrange 
> &true_range, vrange &false_range,
>  src.get_operand (false_range, name);
>  }
>  
> +
> +// This routine will try to refine the ranges of OP1 and OP2 given a relation
> +// K between them.  In order to perform this refinement, one of the operands
> +// must be in the definition chain of the other.  The use is refined using
> +// op1/op2_range on the statement, and the defintion is then recalculated
> +// using the relation.

s/defintion/definition/g
And i'd add a 'kind' before K: given a relation_kind K

We've accumulated quite some "defintion" in the meantime. I think arm
expresses it quite well:
gcc/config/arm/unspecs.md:;; Unspec defintions.
nvptx OTOH only supports weak defintions.
Either way, maybe you can correct this typo in at
least gimple-range-gori.cc and value-query.cc ?

> +
> +bool
> +gori_compute::refine_using_relation (tree op1, vrange &op1_range,
> +tree op2, vrange &op2_range,
> +fur_source &src, relation_kind k)
> +{
> +  gcc_checking_assert (TREE_CODE (op1) == SSA_NAME);
> +  gcc_checking_assert (TREE_CODE (op2) == SSA_NAME);
> +  gcc_checking_assert (k != VREL_VARYING && k != VREL_UNDEFINED);
> +
> +  bool change = false;
> +  bool op1_def_p = in_chain_p (op2, op1);
> +  if (!op1_def_p)
> +if (!in_chain_p (op1, op2))
> +  return false;
> +
> +  tree def_op = op1_def_p ? op1 : op2;
> +  tree use_op = op1_def_p ? op2 : op1;
> +
> +  if (!op1_def_p)
> +k = relation_swap (k);
> +
> +  // op1_def is true if we want to look up op1, otherwise we want op2.
> +  // if neither is the case, we returned in the above check.

I think this comment should be moved up to before setting def_op and
s/op1_def/op1_def_p/
And i hope this ends up as a single if block even if written like above
:)

> +
> +  gimple *def_stmt = SSA_NAME_DEF_STMT (def_op);
> +  gimple_range_op_handler op_handler (def_stmt);
> +  if (!op_handler)
> +return false;
> +  tree def_op1 = op_handler.operand1 ();
> +  tree def_op2 = op_handler.operand2 ();
> +  // if the def isn't binary, the relation will not be useful.
> +  if (!def_op2)
> +return false;
> +
> +  // Determine if op2 is directly referenced as an operand.
> +  if (def_op1 == use_op)
> +{
> +  // def_stmt has op1 in the 1st operand position.
> +  Value_Range other_op (TREE_TYPE (def_op2));
> +  src.get_operand (other_op, def_op2);
> +
> +  // Using op1_range as the LHS, and relation REL, evaluate op2.
> +  tree type = TREE_TYPE (def_op1);
> +  Value_Range new_result (type);
> +  if (!op_handler.op1_range (new_result, type,
> +  op1_def_p ? op1_range : op2_range,
> +  other_op, k))
> + return false;
> +  if (op1_def_p)
> + {
> +   change |= op2_range.intersect (new_result);
> +   // Recalculate op2.
> +   if (op_handler.fold_range (new_result, type, op2_range, other_op))
> + {
> +   change |= op1_range.intersect (new_result);
> + }
> + }
> +  else
> + {
> +   change |= op1_range.intersect (new_result);
> +   // Recalculate op1.
> +   if (op_handler.fold_range (new_result, type, op1_range, other_op))
> + {
> +   change |= op2_range.intersect (new_result);
> + }
> + }
> +}
> +  else if (def_op2 == use_op)
> +{
> +  // def_stmt has op1 in the 1st operand position.

Maybe i'm confused by the swapping, but that's the 2nd operand
position, isn't it? Maybe you forgot to adjust the comments in one of
the blocks?
thanks,

> +  Value_Range other_op (TREE_TYPE (def_op1));
> +  src.get_operand (other_op, def_op1);
> +
> +  // Using op1_range as the LHS, and relation REL, evaluate op2.
> +  tree type = TREE_TYPE (def_op2);
> +  Value_Range new_result (type);
> +  if (!op_handler.op2_range (new_result, type,
> +  op1_def_p ? op1_range : op2_range,
> +  other_op, k))
> + return false;
> +  if (op1_def_p)
> + {
> +   change |= op2_range.intersect (new_result);
> +   // Recalculate op1.
> +   if (op_handler.fold_range (new_result, type, other_op, op2_range))
> + {
> +   change |= op1_range.intersect (new_result);
> + }
> + }
> +  else
> + {
> +   change |= op1_range.intersect (new_result);
> +   // Recalculate op2.
> +   if (op_handler.fold_range (new_result, type, other_op, op1_range))
> + {
> +   change |= op2_range.intersect (new_result);
> + }
> + }
> +}
> +  return change;
> +}
> +
>  // Calculate a r

Re: [PATCH] Process unsigned overflow relations for plus and minus in range-ops.

[Bernhard Reutner-Fischer via Gcc-patches]

On Thu, 29 Sep 2022 18:38:10 -0400
Andrew MacLeod via Gcc-patches  wrote:

> diff --git a/gcc/range-op.cc b/gcc/range-op.cc
> index 9bb04c361d0..830c64bd6b9 100644
> --- a/gcc/range-op.cc
> +++ b/gcc/range-op.cc
> @@ -1305,22 +1305,123 @@ operator_plus::wi_fold (irange &r, tree type,
>value_range_with_overflow (r, type, new_lb, new_ub, ov_lb, ov_ub);
>  }
>  
> +// Given addition or subtraction, determine the possible NORMAL ranges and
> +// OVERFLOW ranges given an OFFSET range.  ADD_P is true for addition.
> +// Return the relation that exists between the LHS and OP1 in order for the
> +// NORMAL range to apply.
> +// a return value of VREL_VARYING means no ranges were applicable.

Capital A in A return value

> +
> +static relation_kind
> +plus_minus_ranges (irange &r_ov, irange &r_normal, const irange &offset,
> + bool add_p)
> +{
> +  relation_kind kind = VREL_VARYING;
> +  // For now, only deal with constant adds.  This could be extended to ranges
> +  // when someone is so motivated.
> +  if (!offset.singleton_p () || offset.zero_p ())
> +return kind;
> +
> +  // Always work with a positive offset.  ie a+ -2 -> a-2  and a- -2 > a+2
> +  wide_int off = offset.lower_bound ();
> +  if (wi::neg_p (off, SIGNED))
> +{
> +  add_p = !add_p;
> +  off = wi::neg (off);
> +}
> +
> +  wi::overflow_type ov;
> +  tree type = offset.type ();
> +  unsigned prec = TYPE_PRECISION (type);
> +  wide_int ub;
> +  wide_int lb;
> +  // calculate the normal range and relation for the operation.
> +  if (add_p)
> +{
> +  //  [ 0 , INF - OFF]
> +  lb = wi::zero (prec);
> +  ub = wi::sub (wi::to_wide (vrp_val_max (type)), off, UNSIGNED, &ov);
> +  kind = VREL_GT;
> +}
> +  else
> +{
> +  //  [ OFF, INF ]
> +  lb = off;
> +  ub = wi::to_wide (vrp_val_max (type));
> +  kind = VREL_LT;
> +}
> +  int_range<2> normal_range (type, lb, ub);
> +  int_range<2> ov_range (type, lb, ub, VR_ANTI_RANGE);
> +
> +  r_ov = ov_range;
> +  r_normal = normal_range;
> +  return kind;
> +}
> +
> +// Once op1 has been calculated by operator_plus or operator_minus, check
> +// to see if the relation passed causes any part of the calculation to
> +// be not possible.  ie
> +// a_2 = b_3 + 1  with a_2 < b_3 can refine the range of b_3 to [INF, INF]
> +// and that further refines a_2 to [0, 0].
> +// R is the value of op1, OP2 is the offset being added/subtracted, REL is 
> the
> +// relation between LHS relatoin OP1  and ADD_P is true for PLUS, false for
> +// MINUS.IF any adjustment can be made, R will reflect it.

s/relatoin/relation/
Excess space before the last sentense, or should this go to a new line?

> +
> +static void
> +adjust_op1_for_overflow (irange &r, const irange &op2, relation_kind rel,
> +  bool add_p)
> +{
> +  tree type = r.type ();
> +  // Check for unsigned overflow and calculate the overflow part.
> +  signop s = TYPE_SIGN (type);
> +  if (!TYPE_OVERFLOW_WRAPS (type) || s == SIGNED)
> +return;
> +
> +  // Only work with <, <=, >, >= relations.
> +  if (!relation_lt_le_gt_ge_p (rel))
> +return;
> +
> +  // Get the ranges for this offset.
> +  int_range_max normal, overflow;
> +  relation_kind k = plus_minus_ranges (overflow, normal, op2, add_p);
> +
> +  // VREL_VARYING means there are no adjustments.
> +  if (k == VREL_VARYING)
> +return;
> +
> +  // If the relations match use the normal range, otherwise use overflow 
> range.
> +  if (relation_intersect (k, rel) == k)
> +r.intersect (normal);
> +  else
> +r.intersect (overflow);
> +  return;
> +}
> +
>  bool
>  operator_plus::op1_range (irange &r, tree type,
> const irange &lhs,
> const irange &op2,
> -   relation_kind rel ATTRIBUTE_UNUSED) const
> +   relation_kind rel) const
>  {
> -  return range_op_handler (MINUS_EXPR, type).fold_range (r, type, lhs, op2);
> +  if (lhs.undefined_p ())
> +return false;
> +  // Start with the default operation.
> +  range_op_handler minus (MINUS_EXPR, type);
> +  if (!minus)
> +return false;
> +  bool res = minus.fold_range (r, type, lhs, op2);
> +  // Check for a relation refinement.
> +  if (res)
> +adjust_op1_for_overflow (r, op2, rel, true /* PLUS_EXPR */);
> +  return res;
>  }
>  
>  bool
>  operator_plus::op2_range (irange &r, tree type,
> const irange &lhs,
> const irange &op1,
> -   relation_kind rel ATTRIBUTE_UNUSED) const
> +   relation_kind rel) const
>  {
> -  return range_op_handler (MINUS_EXPR, type).fold_range (r, type, lhs, op1);
> +  return op1_range (r, type, lhs, op1, rel);
>  }
>  
>  
> @@ -1472,7 +1573,17 @@ operator_minus::op1_range (irange &r, tree type,
>  const irange &op2,
>  relation_kind rel ATTRIBUTE_UNUSED) const

You could remove ATTRIBUTE_UNUSED above fo

Re: [committed] More gimple const/copy propagation opportunities

[Bernhard Reutner-Fischer via Gcc-patches]

On Fri, 30 Sep 2022 17:32:34 -0600
Jeff Law  wrote:

> +  /* This looks good from a CFG standpoint.  Now look at the guts
> + of PRED.  Basically we want to verify there are no PHI nodes
> + and no real statements.  */
> +  if (! gimple_seq_empty_p (phi_nodes (pred)))
> +return false;

So, given the below, neither DEBUG nor labels do count towards an
empty seq [coming in from any PHI that is, otherwise it's a different
thing], which is a bit surprising but well, ok. It looks at PHI IL, so
probably yes. Allegedly that's what it is. Neat if that's true.

> +
> +  gimple_stmt_iterator gsi;
> +  for (gsi = gsi_last_bb (pred); !gsi_end_p (gsi); gsi_prev (&gsi))
> +{
> +  gimple *stmt = gsi_stmt (gsi);
> +
> +  switch (gimple_code (stmt))
> + {
> +   case GIMPLE_LABEL:
> + if (DECL_NONLOCAL (gimple_label_label (as_a  (stmt
> +   return false;
> + break;
> +
> +   case GIMPLE_DEBUG:
> + break;
> +
> +   default:
> + return false;

don't like, sounds odd. Are we sure there's no other garbage that can
manifest here? int meow=42;, and meow unused won't survive?, pragmas
neither or stuff ?

> + }
> +}
> +
> +  return true;
> +}
> +
>  /* We have finished optimizing BB, record any information implied by
> taking a specific outgoing edge from BB.  */
>  

> @@ -583,6 +656,62 @@ record_edge_info (basic_block bb)
>if (can_infer_simple_equiv && TREE_CODE (inverted) == EQ_EXPR)
>   edge_info->record_simple_equiv (op0, op1);
>  }
> +
> +   /* If this block is a single block loop, then we may be able to
> +  record some equivalences on the loop's exit edge.  */
> +   if (single_block_loop_p (bb))
> + {
> +   /* We know it's a single block loop.  Now look at the loop
> +  exit condition.  What we're looking for is whether or not
> +  the exit condition is loop invariant which we can detect
> +  by checking if all the SSA_NAMEs referenced are defined
> +  outside the loop.  */
> +   if ((TREE_CODE (op0) != SSA_NAME
> +|| gimple_bb (SSA_NAME_DEF_STMT (op0)) != bb)
> +   && (TREE_CODE (op1) != SSA_NAME
> +   || gimple_bb (SSA_NAME_DEF_STMT (op1)) != bb))
> + {
> +   /* At this point we know the exit condition is loop
> +  invariant.  The only way to get out of the loop is
> +  if never traverses the backedge to begin with.  This

s/if /if it /

> +  implies that any PHI nodes create equivalances we can

that any threw me off asking for "that if any". Would have been nicer,
i think?

> +  attach to the loop exit edge.  */

attach it to

> +   int alternative

bool

> + = (EDGE_PRED (bb, 0)->flags & EDGE_DFS_BACK) ? 1 : 0;
> +
> +   gphi_iterator gsi;
> +   for (gsi = gsi_start_phis (bb);
> +!gsi_end_p (gsi);
> +gsi_next (&gsi))
> + {
> +   /* If the other alternative is the same as the result,
> +  then this is a degenerate and can be ignored.  */
> +   if (dst == PHI_ARG_DEF (phi, !alternative))
> + continue;
> +
> +   /* Now get the EDGE_INFO class so we can append
> +  it to our list.  We want the successor edge
> +  where the destination is not the source of
> +  an incoming edge.  */
> +   gphi *phi = gsi.phi ();
> +   tree src = PHI_ARG_DEF (phi, alternative);
> +   tree dst = PHI_RESULT (phi);
> +
> +   if (EDGE_SUCC (bb, 0)->dest
> +   != EDGE_PRED (bb, !alternative)->src)

by now, alternative would be easier to grok if it would have been spelled
from_backedge_p or something. IMHO.
thanks,

> + edge_info = (class edge_info *)EDGE_SUCC (bb, 0)->aux;
> +   else
> + edge_info = (class edge_info *)EDGE_SUCC (bb, 1)->aux;
> +
> +   /* Note that since this processing is done independently
> +  of other edge equivalency processing, we may not
> +  have an EDGE_INFO structure set up yet.  */
> +   if (edge_info == NULL)
> + edge_info = new class edge_info (false_edge);
> +   edge_info->record_simple_equiv (dst, src);
> + }
> + }
> + }
>  }
>  }
>  }

Re: Adding a new thread model to GCC

[Bernhard Reutner-Fischer via Gcc-patches]

On 1 October 2022 20:34:45 CEST, LIU Hao via Gcc-patches 
 wrote:
>Greetings.

>The first patch is necessary because somewhere in libgfortran, `pthread_t` is 
>referenced. If the thread model is not `posix`, it fails to compile.

One of several shortcomings mentioned already on Sun, 02 Sep 2018 15:40:28 
-0700 in
https://www.mail-archive.com/gcc-patches@gcc.gnu.org/msg196212.html

Re: Adding a new thread model to GCC

[Bernhard Reutner-Fischer via Gcc-patches]

On 2 October 2022 14:54:54 CEST, LIU Hao  wrote:
>在 2022-10-02 04:02, Bernhard Reutner-Fischer 写道:
>> On 1 October 2022 20:34:45 CEST, LIU Hao via Gcc-patches 
>>  wrote:
>>> Greetings.
>> 
>>> The first patch is necessary because somewhere in libgfortran, `pthread_t` 
>>> is referenced. If the thread model is not `posix`, it fails to compile.
>> 
>> One of several shortcomings mentioned already on Sun, 02 Sep 2018 15:40:28 
>> -0700 in
>> https://www.mail-archive.com/gcc-patches@gcc.gnu.org/msg196212.html
>> 
>
>Forgive me but I didn't get your point. Is the 'shortcoming' the fact that 
>`pthread_t` must be preferred to `__gthread_t`?

No, sorry for my brevity.
Using __gthread_t like in your patch is correct.

thanks,

>
>For non-posix thread models,  is not included, so `pthread_t` is 
>not declared. I haven't looked at other code in libgfortran, but changing 
>`pthread_t` to `__gthread_t` does allow libgfortran to build. I don't know how 
>to test it though, as I don't write Fortran myself.
>
>

Re: Adding a new thread model to GCC

[Bernhard Reutner-Fischer via Gcc-patches]

On 4 October 2022 10:06:00 CEST, LIU Hao  wrote:
>在 2022-10-03 13:03, Bernhard Reutner-Fischer 写道:
>> 
>> No, sorry for my brevity.
>> Using __gthread_t like in your patch is correct.
>> 
>
>I see. In 'libgfortran/io/async.c' there is
>
>  ```
>async_unit *au = u->au;
>LOCK (&au->lock);
>thread_unit = u;
>au->thread = __gthread_self ();
>  ```
>
>so indeed `thread` should be `__gthread_t`.

Yes.

> By the way I reported this issue four months ago and haven't received any 
> response so far:
>
>  https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105764

So, ideally, you would mention this PR in your patch.

LGTM (obvious even) but I cannot formally approve it.
thanks,

Re: [PATCH 6/8] fortran: use grep -F instead of fgrep

[Bernhard Reutner-Fischer via Gcc-patches]

On Fri, 24 Jun 2022 15:06:32 +0800
Xi Ruoyao via Gcc-patches  wrote:

> fgrep has been deprecated in favor of grep -F for a long time, and the
> next grep release (3.8 or 4.0) will print a warning of fgrep is used.
> Stop using fgrep so we won't see the warning.
> 
> gcc/ChangeLog:
> 
>   * fortran/Make-lang.in: Use grep -F instead of fgrep.
> ---
>  gcc/fortran/Make-lang.in | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/gcc/fortran/Make-lang.in b/gcc/fortran/Make-lang.in
> index 1cb47cb1a52..51279b03aad 100644
> --- a/gcc/fortran/Make-lang.in
> +++ b/gcc/fortran/Make-lang.in
> @@ -278,7 +278,7 @@ $(DESTDIR)$(man1dir)/$(GFORTRAN_INSTALL_NAME)$(man1ext): 
> doc/gfortran.1 \
>   -chmod a-x $@
>  
>  fortran.uninstall:
> - if $(SHELL) -c 'install-info --version | sed 1q | fgrep -s -v -i 
> debian' >/dev/null 2>&1; then \
> + if $(SHELL) -c 'install-info --version | sed 1q | grep -F -s -v -i 
> debian' >/dev/null 2>&1; then \
> echo " install-info --delete --info-dir=$(DESTDIR)$(infodir) 
> $(DESTDIR)$(infodir)/gfortran.info"; \
> install-info --delete --info-dir=$(DESTDIR)$(infodir) 
> $(DESTDIR)$(infodir)/gfortran.info || : ; \
>   else : ; fi; \

I'd replace -s >/dev/null 2>&1 with -q while at it.

But why is -F used here in the first place?
I do not see much in debian that can be interpreted as a regex?

thanks,

Re: [PATCH 6/8] fortran: use grep -F instead of fgrep

[Bernhard Reutner-Fischer via Gcc-patches]

On 24 June 2022 14:35:20 CEST, Rainer Orth  
wrote:
>Hi Xi,
>
>> On Fri, 2022-06-24 at 13:13 +0200, Bernhard Reutner-Fischer wrote:
>>
>>> > -   if $(SHELL) -c 'install-info --version | sed 1q | fgrep -s -v
>>> > -i debian' >/dev/null 2>&1; then \
>>> > +   if $(SHELL) -c 'install-info --version | sed 1q | grep -F -s -v
>>> > -i debian' >/dev/null 2>&1; then \
>>> >   echo " install-info --delete --info-dir=$(DESTDIR)$(infodir)
>>> > $(DESTDIR)$(infodir)/gfortran.info"; \
>>> >   install-info --delete --info-dir=$(DESTDIR)$(infodir)
>>> > $(DESTDIR)$(infodir)/gfortran.info || : ; \
>>> > else : ; fi; \
>>> 
>>> I'd replace -s >/dev/null 2>&1 with -q while at it.
>>> 
>>> But why is -F used here in the first place?
>>> I do not see much in debian that can be interpreted as a regex?
>>
>> I'm not sure.  It was there since 2004.  Perhaps the author thinks fgrep
>> may save several CPU cycles :).  I'll just use a plain grep in PATCH v2.
>>
>> Rainer: do you have some idea about the availability of "-q" on
>> different hosts?  If you agree I'll use it instead of -s > /dev/null
>> too.
>
>again, the autoconf manual warns agains it, even more so against -s.
>That's the first reference for portability issues and shouldn't be
>ignored without good reason.
>
>In the GCC and Solaris context, /bin/grep supports both -q and -s in
>Solaris 11.3 and 11.4.  It doesn't support -q on Solaris 10, though
>(again, no longer relevant for GCC).

The good reason I would bring forward is that the systems mentioned in the 
autoconf docs are all not supported anymore (IRIX, ancient or old Solaris etc).
Furthermore, grep(1) is required by POSIX to support -q since at least 1997; 
see https://pubs.opengroup.org/onlinepubs/007908799/xcu/grep.html

That's about 25 years now, so everybody had plenty of time to implement this 
specific part, which is even trivial to implement for this particular case of 
grep -q.

All in all i think that we should not be held hostage by such systems any 
longer, at least for such cases that are trivial to fix to conformance. Of 
course that may be just /me.

Iff fixing egrep or fgrep occurrences though,  we should use plain grep where 
applicable, like in this case, IMO.

thanks,

Re: [RFC] fortran: restore array indexing for all descriptor arrays [PR102043]

[Bernhard Reutner-Fischer via Gcc-patches]

On 2 July 2022 14:47:01 CEST, Mikael Morin  wrote:
>Le 02/07/2022 à 13:18, Thomas Koenig a écrit :
>> 
>> One thought: If we have to bite the bullet and break the ABI, why not go
>> all the way and use the C descriptors in ISO_Fortran_binding.h as
>> gfortran's native format?
>> 
>As far as I know, CFI descriptors are mutually exclusive with
>non-negative array indexes.
>
>CFI descriptors' base_addr field points to the first element in
>descriptor indexing order, so they can be indexed negatively and we
>can’t safely use array indexing with them.
>
>With an additional offset field it would be possible though (with a
>different semantics for the base_addr field).  Or we just keep the
>different semantics of the base_addr field and assume that there is an
>implicit offset with value sum(extent * max(-sm, 0)).
>
>Anyway, this is the long-delayed array descriptor reform, right?  Any

I think so, yes.

>one remembers how far we are from it?

We have a wiki page with notes IIRC.

Re: Splitting up 27_io/basic_istream/ignore/wchar_t/94749.cc (takes too long)

[Bernhard Reutner-Fischer via Gcc-patches]

On 9 June 2023 19:18:45 CEST, Mike Stump via Gcc-patches 
 wrote:
> simulation ports.  Maybe a 20-100x speedup? If you want to go this way I'd 
> say do it in python at the bottom as it would be nice to switch over to 
> python in the next 5-20 years and away from tcl.

Yes, i guess we have all pondered this for way long enough, but it is a lot of 
work still.

The nice side effect would be that we see such hogs easier and earlier, at 
least more comfortable. But well. Either way, what should we do about remote 
env, if there is one? If the target supports it, send it and skip otherwise?

thanks,

Re: Splitting up 27_io/basic_istream/ignore/wchar_t/94749.cc (takes too long)

[Bernhard Reutner-Fischer via Gcc-patches]

On Sat, 10 Jun 2023 11:29:36 -0700
Mike Stump  wrote:

> On Jun 9, 2023, at 2:47 PM, Bernhard Reutner-Fischer
>  wrote:

> > But well. Either way, what
> > should we do about remote env, if there is one? If the target
> > supports it, send it and skip otherwise?  

> So, to focus a
> conversation, which target, which host, canadian? Which part of the
> environment? What part is missing you want to fix? Want to unify
> between targets... and so on.
> 

The most recent target where this came up again was GCN i think.
See the last block in
https://inbox.sourceware.org/gcc-patches/20230508195217.4897009f@nbbrfq/
and Thomas' reference therein to Tobias'
https://inbox.sourceware.org/gcc-patches/018bcdeb-b3bb-1859-cd0b-a8a92e26d...@codesourcery.com/

thoughts?

thanks,

Re: [i386 PATCH] A minor code clean-up: Use NULL_RTX instead of nullptr

[Bernhard Reutner-Fischer via Gcc-patches]

plonk.

On 26 May 2023 10:31:51 CEST, Bernhard Reutner-Fischer  
wrote:
>On Thu, 25 May 2023 18:58:04 +0200
>Bernhard Reutner-Fischer  wrote:
>
>> On Wed, 24 May 2023 18:54:06 +0100
>> "Roger Sayle"  wrote:
>> 
>> > My understanding is that GCC's preferred null value for rtx is NULL_RTX
>> > (and for tree is NULL_TREE), and by being typed allows strict type 
>> > checking,
>> > and use with function polymorphism and template instantiation.
>> > C++'s nullptr is preferred over NULL and 0 for pointer types that don't
>> > have a defined null of the correct type.
>> > 
>> > This minor clean-up uses NULL_RTX consistently in i386-expand.cc.  
>> 
>> Oh. Well, i can't resist cleanups :)
>
>> (and handle nullptr too, and the same game for tree)
>
>so like the attached. And
>sed -e 's/RTX/TREE/g' -e 's/rtx/tree/g' \
>  < ~/coccinelle/gcc-rtx-null.0.cocci \
>  > ~/coccinelle/gcc-tree-null.0.cocci
>
>I do not know if we want to shorten explicit NULL comparisons.
> foo == NULL => !foo and foo != NULL => foo
>Left them alone in the form they were written.
>
>See the attached result of the rtx hunks, someone would have to build

I've bootstrapped and regtested the hunks for rtx as cited up-thread without 
regressions (as expected).

I know everybody is busy, but I'd like to know if I should swap these out 
completely,   or postpone this until start of stage3 or next stage 1 or 
something.
I can easily keep these local to my personal pre-configure stage for my own 
amusement.

thanks,

>it and hack git-commit-mklog.py --changelog 'Use NULL_RTX.'
>to print("{}.".format(random.choice(['Ditto', 'Same', 'Likewise']))) ;)
>
>> 
>> Just a thought..
>
>cheers,

Re: [PATCH] ipa: Self-DCE of uses of removed call LHSs (PR 108007)

[Bernhard Reutner-Fischer via Gcc-patches]

On 13 June 2023 17:11:13 CEST, Martin Jambor  wrote:
>Ping.

s/funtction/function/
s/runing/running/
>
>Thanks,

Re: [PATCH] Introduce hardbool attribute for C

[Bernhard Reutner-Fischer via Gcc-patches]

On 16 June 2023 07:35:27 CEST, Alexandre Oliva via Gcc-patches 
 wrote:

index 0..634feaed4deef
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/hardbool-err.c
@@ -0,0 +1,28 @@
+/* { dg-do compile } */
+/* { dg-options "" } */
+
+typedef _Bool __attribute__ ((__hardbool__))
+hbbl; /* { dg-error "integral types" } */
+
+typedef double __attribute__ ((__hardbool__))
+hbdbl; /* { dg-error "integral types" } */
+
+enum x;
+typedef enum x __attribute__ ((__hardbool__))
+hbenum; /* { dg-error "integral types" } */
+
+struct s;
+typedef struct s __attribute__ ((__hardbool__))
+hbstruct; /* { dg-error "integral types" } */
+
+typedef int __attribute__ ((__hardbool__ (0, 0)))
+hb00; /* { dg-error "different values" } */
+
+typedef int __attribute__ ((__hardbool__ (4, 16))) hb4x;
+struct s {
+ hb4x m:2;
+}; /* { dg-error "is a GCC extension|different values" } */
+/* { dg-warning "changes value" "warning" { target *-*-* } .-1 } */
+
+hb4x __attribute__ ((vector_size (4 * sizeof (hb4x
+vvar; /* { dg-error "invalid vector type" } */

Arm-chair, tinfoil hat still on, didn't look closely, hence:

I don't see explicit tests with _Complex nor __complex__. Would we want to 
check these here, or are they handled thought the "underlying" tests above?

I'd welcome a fortran interop note in the docs as hinted previously to cover 
out of the box behavior. It's probably reasonably unlikely but better be safe 
than sorry?
cheers,

Re: [Patch, fortran] PR107900 Select type with intrinsic type inside associate causes ICE / Segmenation fault

[Bernhard Reutner-Fischer via Gcc-patches]

Hi!

First of all, many thanks for the patch!
If i may, i have a question concerning the chosen style in the error
message and one nitpick concerning a return type though, the latter
primarily prompting this reply.

On Tue, 20 Jun 2023 11:54:25 +0100
Paul Richard Thomas via Fortran  wrote:

> diff --git a/gcc/fortran/expr.cc b/gcc/fortran/expr.cc
> index d5cfbe0cc55..c960dfeabd9 100644
> --- a/gcc/fortran/expr.cc
> +++ b/gcc/fortran/expr.cc

> @@ -6470,6 +6480,22 @@ gfc_check_vardef_context (gfc_expr* e, bool pointer, 
> bool alloc_obj,
>   }
> return false;
>   }
> +  else if (context && gfc_is_ptr_fcn (assoc->target))
> + {
> +   if (!gfc_notify_std (GFC_STD_F2018, "%qs at %L associated to "
> +"pointer function target being used in a "
> +"variable definition context (%s)", name,
> +&e->where, context))

I'm curious why you decided to put context in braces and not simply use
quotes as per %qs?

> + return false;
> +   else if (gfc_has_vector_index (e))
> + {
> +   gfc_error ("%qs at %L associated to vector-indexed target"
> +  " cannot be used in a variable definition"
> +  " context (%s)",
> +  name, &e->where, context);

Ditto.

> diff --git a/gcc/fortran/match.cc b/gcc/fortran/match.cc
> index e7be7fddc64..0e4b5440393 100644
> --- a/gcc/fortran/match.cc
> +++ b/gcc/fortran/match.cc
> @@ -6377,6 +6377,39 @@ build_class_sym:
>  }
> 
> 
> +/* Build the associate name  */
> +static int
> +build_associate_name (const char *name, gfc_expr **e1, gfc_expr **e2)
> +{

> +return 1;

> +  return 0;
> +}

I've gone through the frontend recently and changed several such
boolean functions to use bool where appropriate. May i ask folks to use
narrower types in new code, please?
Iff later in the pipeline it is considered appropriate or benefical to
promote types, these will eventually be promoted.

> diff --git a/gcc/fortran/trans-decl.cc b/gcc/fortran/trans-decl.cc
> index e6a4337c0d2..18589e17843 100644
> --- a/gcc/fortran/trans-decl.cc
> +++ b/gcc/fortran/trans-decl.cc

> @@ -1906,6 +1915,7 @@ gfc_get_symbol_decl (gfc_symbol * sym)
>   gcc_assert (!sym->value || sym->value->expr_type == EXPR_NULL);
>  }
> 
> +

ISTM that the addition of vertical whitespace like here is in
contradiction with the coding style.

Please kindly excuse my comment and, again, thanks!

>gfc_finish_var_decl (decl, sym);
> 
>if (sym->ts.type == BT_CHARACTER)

Re: [PATCH] Introduce hardbool attribute for C

[Bernhard Reutner-Fischer via Gcc-patches]

On Wed, 21 Jun 2023 22:08:55 -0300
Alexandre Oliva  wrote:

> Thanks for the test.
> 
> Did you mean for me to incorporate it into the patch, or do you mean to
> contribute it separately, if the feature happens to be accepted?

These were your tests that i quoted but i or my MUA botched to add one
level of quotes -- sorry for that.

> 
> On Jun 19, 2023, Bernhard Reutner-Fischer  wrote:
> 
> > I don't see explicit tests with _Complex nor __complex__. Would we
> > want to check these here, or are they handled thought the "underlying"
> > tests above?  
> 
> Good question.  The notion of using complex types to hold booleans
> hadn't even crossed my mind.

Maybe it is not real, it just sparkled through somehow.

> On the one hand, there doesn't seem to be reason to rule them out, and
> that could go for literally any other type.
> 
> On the other, there doesn't seem to be any useful case for them.  Can
> anyone think of one?

We could either not reject other such uses and wait or we could reject
them and equally wait for complaints. I would not dare to bet who pops
up first, fuzzers or users, though arguments of the latter would
probably be interesting.. I don't have an opinion (nor a use-case),
really, it was just a thought (i mentioned tinfoil hat, did i ;).

> 
> > I'd welcome a fortran interop note in the docs  
> 
> Is there any good place for such interop notes?  I'm not sure I'm
> best-suited to write them up, since Fortran is not a language I'm
> very familiar with, but I suppose I could give it a try.
> 

I'd append to your extend.texi hunk below the para about uninitialized a
note to the effect of:
Note: Types annotated with this attribute may not be Fortran
interoperable.

I would not go into too much detail about C_BOOL nor LOGICAL for i
reckon anybody sensibilised to either two of that attribute, C and
Fortran will draw her conclusions.
Didn't really think how easy it would be to handle this on the user
side, but i fear the modern iso_c_binding way would need help from the
compiler for the lazy. I'd expect a user to be able to trivially
translate this in wrappers done the old way though, which is a pity
from an educational and modernisation POV. Didn't look closely, so this
guesstimate might be all wrong, of course.

thanks,

Re: [PATCH V6] VECT: Apply LEN_MASK_{LOAD,STORE} into vectorizer

[Bernhard Reutner-Fischer via Gcc-patches]

On 23 June 2023 01:51:12 CEST, juzhe.zh...@rivai.ai wrote:
>From: Ju-Zhe Zhong 

I am sorry but I somehow overlooked a trivial spot in V5.
Nit which does not warrant an immediate next version, but please consider it 
before pushing iff approved:

>+if (final_len)
>+  {
>+signed char biasval
>+  = LOOP_VINFO_PARTIAL_LOAD_STORE_BIAS (loop_vinfo);
>+
>+bias = build_int_cst (intQI_type_node, biasval);
>+  }
>+
>+/* Arguments are ready.  Create the new vector stmt.  */
>+if (final_len)
>+  {

Fuse the block below into the one above as the condition seems to be identical?
thanks,

>+gcall *call;
> tree ptr = build_int_cst (ref_type, align * BITS_PER_UNIT);
> /* Need conversion if it's wrapped with VnQI.  */
> if (vmode != new_vmode)

Re: [PATCH V6] VECT: Apply LEN_MASK_{LOAD,STORE} into vectorizer

[Bernhard Reutner-Fischer via Gcc-patches]

On 23 June 2023 10:03:45 CEST, Richard Sandiford  
wrote:

>> Fuse the block below into the one above as the condition seems to be 
>> identical?
>
>Yeah, true, but I think the idea is that the code above “Arguments are
>ready” is calculating argument values, and the code after it is creating
>code.  These are two separate steps, and the fact that the two final_len
>blocks end up being consecutive is something of a coincidence.
>
>So personally I think we should keep the structure in the patch.

Sure, works for me.
thanks,

Re: [PATCH] Add auto-resizing capability to irange's [PR109695]

[Bernhard Reutner-Fischer via Gcc-patches]

On Mon, 15 May 2023 12:35:23 +0200
Aldy Hernandez via Gcc-patches  wrote:

> +// For resizable ranges, resize the range up to HARD_MAX_RANGES if the
> +// NEEDED pairs is greater than the current capacity of the range.
> +
> +inline void
> +irange::maybe_resize (int needed)
> +{
> +  if (!m_resizable || m_max_ranges == HARD_MAX_RANGES)
> +return;
> +
> +  if (needed > m_max_ranges)
> +{
> +  m_max_ranges = HARD_MAX_RANGES;
> +  wide_int *newmem = new wide_int[m_max_ranges * 2];
> +  memcpy (newmem, m_base, sizeof (wide_int) * num_pairs () * 2);
> +  m_base = newmem;

Please excuse my ignorance, but where's the old m_base freed? I think
the assignment above does not call the destructor, or does it?

thanks,

> +}
> +}
> +
> +template
> +inline
> +int_range::~int_range ()
> +{
> +  if (RESIZABLE && m_base != m_ranges)
> +delete m_base;
> +}

Re: [PATCH 1/3] gcc: Fix nonportable shell syntax in "test" and "[" commands [PR105831]

[Bernhard Reutner-Fischer via Gcc-patches]

On 18 May 2023 14:56:45 CEST, Jonathan Wakely via Gcc-patches 
 wrote:
>From: Michael B��uerle 
>
>POSIX sh does not support the == for string comparisons, use = instead.
>
>gcc/ChangeLog:
>
>   PR bootstrap/105831

> 
>diff --git a/gcc/configure.ac b/gcc/configure.ac
>index 075424669c9..cc8dd9e20bf 100644
>--- a/gcc/configure.ac
>+++ b/gcc/configure.ac
>@@ -473,7 +473,7 @@ AC_CHECK_SIZEOF(dev_t)
> if test "$enable_largefile" != no; then
>   case "$host, $build" in
> *-*-aix*,*|*,*-*-aix*)
>-  if test "$ac_cv_sizeof_ino_t" == "4" -a "$ac_cv_sizeof_dev_t" == 4; then
>+  if test "$ac_cv_sizeof_ino_t" = "4" -a "$ac_cv_sizeof_dev_t" = 4; then

test(1) -a and -o are marked obsolescent in SUS and should be spelled out as && 
or ||, respectively: 
https://pubs.opengroup.org/onlinepubs/9699919799/utilities/test.html

thanks,

Re: [PATCH 08/14] fortran: use _P() defines from tree.h

[Bernhard Reutner-Fischer via Gcc-patches]

On Sun, 14 May 2023 15:10:12 +0200
Mikael Morin  wrote:

> Le 14/05/2023 à 01:23, Bernhard Reutner-Fischer via Gcc-patches a écrit :
> > From: Bernhard Reutner-Fischer 
> > 
> > gcc/fortran/ChangeLog:
> > 
> > * trans-array.cc (is_pointer_array): Use _P() defines from tree.h.
> > (gfc_conv_scalarized_array_ref): Ditto.
> > (gfc_conv_array_ref): Ditto.
> > * trans-decl.cc (gfc_finish_decl): Ditto.
> > (gfc_get_symbol_decl): Ditto.
> > * trans-expr.cc (gfc_trans_pointer_assignment): Ditto.
> > (gfc_trans_arrayfunc_assign): Ditto.
> > (gfc_trans_assignment_1): Ditto.
> > * trans-intrinsic.cc (gfc_conv_intrinsic_minmax): Ditto.
> > (conv_intrinsic_ieee_value): Ditto.
> > * trans-io.cc (gfc_convert_array_to_string): Ditto.
> > * trans-openmp.cc (gfc_omp_is_optional_argument): Ditto.
> > (gfc_trans_omp_clauses): Ditto.
> > * trans-stmt.cc (gfc_conv_label_variable): Ditto.
> > * trans.cc (gfc_build_addr_expr): Ditto.
> > (get_array_span): Ditto.  
> 
> OK from the fortran side.
> 
> Thanks

Thanks, i'll push it during the weekend.

I've fed gfortran.h into the script and found some CLASS_DATA spots,
see attached bootstrapped and tested patch.
Do we want to have that?
If so, i'd write a proper ChangeLog, of course.

Thanks!
diff --git a/gcc/fortran/class.cc b/gcc/fortran/class.cc
index 9d0c802b867..1466b07e260 100644
--- a/gcc/fortran/class.cc
+++ b/gcc/fortran/class.cc
@@ -889,7 +889,7 @@ copy_vtab_proc_comps (gfc_symbol *declared, gfc_symbol *vtype)
 
   vtab = gfc_find_derived_vtab (declared);
 
-  for (cmp = vtab->ts.u.derived->components; cmp; cmp = cmp->next)
+  for (cmp = CLASS_DATA (vtab); cmp; cmp = cmp->next)
 {
   if (gfc_find_component (vtype, cmp->name, true, true, NULL))
 	continue;
@@ -1078,7 +1078,7 @@ finalize_component (gfc_expr *expr, gfc_symbol *derived, gfc_component *comp,
   gfc_component *c;
 
   vtab = gfc_find_derived_vtab (comp->ts.u.derived);
-  for (c = vtab->ts.u.derived->components; c; c = c->next)
+  for (c = CLASS_DATA (vtab); c; c = c->next)
 	if (strcmp (c->name, "_final") == 0)
 	  break;
 
@@ -1143,7 +1143,7 @@ finalize_component (gfc_expr *expr, gfc_symbol *derived, gfc_component *comp,
 {
   gfc_component *c;
 
-  for (c = comp->ts.u.derived->components; c; c = c->next)
+  for (c = CLASS_DATA (comp); c; c = c->next)
 	finalize_component (e, comp->ts.u.derived, c, stat, fini_coarray, code,
 			sub_ns);
   gfc_free_expr (e);
@@ -1675,7 +1675,7 @@ generate_finalization_wrapper (gfc_symbol *derived, gfc_namespace *ns,
   gfc_component *comp;
 
   vtab = gfc_find_derived_vtab (derived->components->ts.u.derived);
-  for (comp = vtab->ts.u.derived->components; comp; comp = comp->next)
+  for (comp = CLASS_DATA (vtab); comp; comp = comp->next)
 	if (comp->name[0] == '_' && comp->name[1] == 'f')
 	  {
 	ancestor_wrapper = comp->initializer;
@@ -2752,7 +2752,7 @@ yes:
 {
   /* Return finalizer expression.  */
   gfc_component *final;
-  final = vtab->ts.u.derived->components->next->next->next->next->next;
+  final = CLASS_DATA (vtab)->next->next->next->next->next;
   gcc_assert (strcmp (final->name, "_final") == 0);
   gcc_assert (final->initializer
 		  && final->initializer->expr_type != EXPR_NULL);
diff --git a/gcc/fortran/data.cc b/gcc/fortran/data.cc
index d29eb12c1b1..f907bb35eb1 100644
--- a/gcc/fortran/data.cc
+++ b/gcc/fortran/data.cc
@@ -730,7 +730,7 @@ formalize_structure_cons (gfc_expr *expr)
   if (!cur || cur->n.component == NULL)
 return;
 
-  for (order = expr->ts.u.derived->components; order; order = order->next)
+  for (order = CLASS_DATA (expr); order; order = order->next)
 {
   cur = find_con_by_component (order, expr->value.constructor);
   if (cur)
diff --git a/gcc/fortran/dependency.cc b/gcc/fortran/dependency.cc
index b398b29a642..864470afdec 100644
--- a/gcc/fortran/dependency.cc
+++ b/gcc/fortran/dependency.cc
@@ -1253,7 +1253,7 @@ check_data_pointer_types (gfc_expr *expr1, gfc_expr *expr2)
 
   if (sym1->ts.type == BT_DERIVED && !seen_component_ref)
 {
-  for (cm1 = sym1->ts.u.derived->components; cm1; cm1 = cm1->next)
+  for (cm1 = CLASS_DATA (sym1); cm1; cm1 = cm1->next)
 	{
 	  if (cm1->ts.type == BT_DERIVED)
 	return false;
diff --git a/gcc/fortran/expr.cc b/gcc/fortran/expr.cc
index aa01a4d3d22..a6b4ef0a0bf 100644
--- a/gcc/fortran/expr.cc
+++ b/gcc/fortran/expr.cc
@@ -2671,7 +2671,7 @@ check_alloc_comp_init (gfc_expr *e)
   gcc_assert (e->expr_type == EXPR_STRUCTURE);
   gcc_assert (e->ts.t

Re: [PATCH v2] Fortran: Narrow return types [PR78798]

[Bernhard Reutner-Fischer via Gcc-patches]

On Sun, 14 May 2023 14:27:42 +0200
Mikael Morin  wrote:

> Le 10/05/2023 à 18:47, Bernhard Reutner-Fischer via Fortran a écrit :
> > From: Bernhard Reutner-Fischer 
> > 
> > gcc/fortran/ChangeLog:
> > 
> > PR fortran/78798
> > * array.cc (compare_bounds): Use narrower return type.
> > (gfc_compare_array_spec): Likewise.
> > (is_constant_element): Likewise.
> > (gfc_constant_ac): Likewise.  
> (...)
> > ---
> > Bootstrapped without new warnings and regression tested on
> > x86_64-linux with no regressions, OK for trunk?
> >   
> (...)
> > diff --git a/gcc/fortran/check.cc b/gcc/fortran/check.cc
> > index b348bda6e6c..4e3aed84b9d 100644
> > --- a/gcc/fortran/check.cc
> > +++ b/gcc/fortran/check.cc
> > @@ -1156,7 +1156,7 @@ dim_rank_check (gfc_expr *dim, gfc_expr *array, int 
> > allow_assumed)
> >  dimension bi, returning 0 if they are known not to be identical,
> >  and 1 if they are identical, or if this cannot be determined.  */
> >   
> > -static int
> > +static bool
> >   identical_dimen_shape (gfc_expr *a, int ai, gfc_expr *b, int bi)
> >   {
> > mpz_t a_size, b_size;  
> 
> To be consistent, please change as well the local variable "ret" used as 
> return value from int to bool.
> 
> > diff --git a/gcc/fortran/cpp.cc b/gcc/fortran/cpp.cc
> > index c3b7c7f7bd9..d7890a97287 100644
> > --- a/gcc/fortran/cpp.cc
> > +++ b/gcc/fortran/cpp.cc
> > @@ -297,7 +297,7 @@ gfc_cpp_init_options (unsigned int 
> > decoded_options_count,
> > gfc_cpp_option.deferred_opt_count = 0;
> >   }
> >   
> > -int
> > +bool
> >   gfc_cpp_handle_option (size_t scode, const char *arg, int value 
> > ATTRIBUTE_UNUSED)
> >   {
> > int result = 1;  
> 
> Same here, change the type of variable "result".
> 
> (...)
> > diff --git a/gcc/fortran/dependency.cc b/gcc/fortran/dependency.cc
> > index a648d5c7903..b398b29a642 100644
> > --- a/gcc/fortran/dependency.cc
> > +++ b/gcc/fortran/dependency.cc  
> (...)
> 
> > @@ -1091,7 +1091,7 @@ gfc_check_argument_dependency (gfc_expr *other, 
> > sym_intent intent,
> >   /* Like gfc_check_argument_dependency, but check all the arguments in 
> > ACTUAL.
> >  FNSYM is the function being called, or NULL if not known.  */
> >   
> > -int
> > +bool
> >   gfc_check_fncall_dependency (gfc_expr *other, sym_intent intent,
> >  gfc_symbol *fnsym, gfc_actual_arglist *actual,
> >  gfc_dep_check elemental)  
> 
> Why not change the associated subfunctions 
> (gfc_check_argument_dependency, gfc_check_argument_var_dependency) as well ?

I have left these subfunctions alone for now to get the other hunks out
of the way. I have adjusted the patch according to your other comments
and pushed it as r14-973-gc072df1ab14450.

Thanks!

> 
> (...)
> > @@ -2098,7 +2098,7 @@ ref_same_as_full_array (gfc_ref *full_ref, gfc_ref 
> > *ref)
> > there is some kind of overlap.
> > 0 : array references are identical or not overlapping.  */
> >   
> > -int
> > +bool
> >   gfc_dep_resolver (gfc_ref *lref, gfc_ref *rref, gfc_reverse *reverse,
> >   bool identical)
> >   {  
> 
> The function comment states that the function may return 2, which 
> doesn't seem to be the case any more.  So please update the comment.
> 
> (...)> diff --git a/gcc/fortran/symbol.cc b/gcc/fortran/symbol.cc
> > index 221165d6dac..b4b36e27d75 100644
> > --- a/gcc/fortran/symbol.cc
> > +++ b/gcc/fortran/symbol.cc
> > @@ -3216,7 +3216,7 @@ gfc_find_symtree_in_proc (const char* name, 
> > gfc_namespace* ns)
> >  any parent namespaces if requested by a nonzero parent_flag.
> >  Returns nonzero if the name is ambiguous.  */
> >   
> > -int
> > +bool
> >   gfc_find_sym_tree (const char *name, gfc_namespace *ns, int parent_flag,
> >gfc_symtree **result)
> >   {  
> 
> Maybe change nonzero to true in the comment?
> 
> (...)
> 
> OK with all the above fixed.
> 
> Thanks.
> 

Re: [PATCH 01/14] ada: use _P() defines from tree.h

[Bernhard Reutner-Fischer via Gcc-patches]

On Mon, 15 May 2023 12:05:10 +0200
Eric Botcazou  wrote:

> > && DECL_RETURN_VALUE_P (inner))
> > diff --git a/gcc/ada/gcc-interface/utils.cc b/gcc/ada/gcc-interface/utils.cc
> > index 0c4f8b90c8e..460ef6f1f01 100644
> > --- a/gcc/ada/gcc-interface/utils.cc
> > +++ b/gcc/ada/gcc-interface/utils.cc
> > @@ -1966,7 +1966,7 @@ finish_record_type (tree record_type, tree field_list,
> > int rep_level, bool debug_info_p)
> >  {
> >const enum tree_code orig_code = TREE_CODE (record_type);
> > -  const bool had_size = TYPE_SIZE (record_type) != NULL_TREE;
> > +  const bool had_size = COMPLETE_TYPE_P (record_type);
> >const bool had_align = TYPE_ALIGN (record_type) > 0;
> >/* For all-repped records with a size specified, lay the QUAL_UNION_TYPE
> >   out just like a UNION_TYPE, since the size will be fixed.  */  
> 
> This one is not an improvement but more of a coincidence; the rest is OK.
> 

I've dropped this hunk and installed the rest as
r14-974-g04682fe764004b.
Thanks!

Re: [PATCH 01/14] ada: use _P() defines from tree.h

[Bernhard Reutner-Fischer via Gcc-patches]

On Sun, 14 May 2023 17:03:55 -0600
Jeff Law  wrote:

> On 5/13/23 17:23, Bernhard Reutner-Fischer via Gcc-patches wrote:
> > From: Bernhard Reutner-Fischer 
> > 
> > gcc/ada/ChangeLog:
> > 
> > * gcc-interface/decl.cc (gnat_to_gnu_entity): Use _P defines

> The series as a whole is OK.

Thanks.
I've dropped the go and rust hunks and installed the rest (with tweaks
as requested) as r14-974-g04682fe764004b .. r14-985-gca2007a9bb3074

Re: [PATCH] avr: Set param_min_pagesize to 0 [PR105523]

[Bernhard Reutner-Fischer via Gcc-patches]

On 19 May 2023 07:58:48 CEST, "SenthilKumar.Selvaraj--- via Gcc-patches" 
 wrote:

Just a nit:

>+static bool
>+avr_addr_space_zero_address_valid (addr_space_t as ATTRIBUTE_UNUSED)
>+{
>+  return flag_delete_null_pointer_checks == 0;
>+}

Since we are c++ nowadays, you can omit the parameter name for unused 
arguments. I.e.:

static bool
avr_addr_space_zero_address_valid (addr_space_t)
{
  ...

Re: [PATCH 08/14] fortran: use _P() defines from tree.h

[Bernhard Reutner-Fischer via Gcc-patches]

On Thu, 18 May 2023 21:20:41 +0200
Mikael Morin  wrote:

> Le 18/05/2023 à 17:18, Bernhard Reutner-Fischer a écrit :

> > I've fed gfortran.h into the script and found some CLASS_DATA spots,
> > see attached bootstrapped and tested patch.
> > Do we want to have that?  
> Some of it makes sense, but not all of it.
> 
> It is a macro to access the _data component of a class container.
> So for class-related stuff it makes sense to use CLASS_DATA, and 
> typically there will be a check that the type is BT_CLASS before.
> But for cases where we loop over all of the components of a type that is 
> not necessarily a class container, it doesn't make sense to use CLASS_DATA.
> 
> So I suggest to only keep the following hunks.
[]
> OK for those hunks.

Pushed those as r14-1001-g05b7cc7daac8b3
Many thanks!

PS: I'm attaching the fugly script i used to do these macro
replacements FYA.


use-defines.1.awk
Description: application/awk

Re: [V7][PATCH 1/2] Handle component_ref to a structre/union field including flexible array member [PR101832]

[Bernhard Reutner-Fischer via Gcc-patches]

On Fri, 19 May 2023 20:49:47 +
Qing Zhao via Gcc-patches  wrote:

> GCC extension accepts the case when a struct with a flexible array member
> is embedded into another struct or union (possibly recursively).

Do you mean TYPE_TRAILING_FLEXARRAY()?

> diff --git a/gcc/tree.h b/gcc/tree.h
> index 0b72663e6a1..237644e788e 100644
> --- a/gcc/tree.h
> +++ b/gcc/tree.h
> @@ -786,7 +786,12 @@ extern void omp_clause_range_check_failed (const_tree, 
> const char *, int,
> (...) prototype, where arguments can be accessed with va_start and
> va_arg), as opposed to an unprototyped function.  */
>  #define TYPE_NO_NAMED_ARGS_STDARG_P(NODE) \
> -  (TYPE_CHECK (NODE)->type_common.no_named_args_stdarg_p)
> +  (FUNC_OR_METHOD_CHECK (NODE)->type_common.no_named_args_stdarg_p)
> +
> +/* True if this RECORD_TYPE or UNION_TYPE includes a flexible array member
> +   at the last field recursively.  */
> +#define TYPE_INCLUDE_FLEXARRAY(NODE) \
> +  (RECORD_OR_UNION_CHECK (NODE)->type_common.no_named_args_stdarg_p)

Until i read the description above i read TYPE_INCLUDE_FLEXARRAY as an
option to include or not include something. The description hints more
at TYPE_INCLUDES_FLEXARRAY (with an S) to be a type which has at least
one member which has a trailing flexible array or which itself has a
trailing flexible array.

>  
>  /* In an IDENTIFIER_NODE, this means that assemble_name was called with
> this string as an argument.  */

Re: [V7][PATCH 1/2] Handle component_ref to a structre/union field including flexible array member [PR101832]

[Bernhard Reutner-Fischer via Gcc-patches]

On 24 May 2023 16:09:21 CEST, Qing Zhao  wrote:
>Bernhard,
>
>Thanks a lot for your comments.
>
>> On May 19, 2023, at 7:11 PM, Bernhard Reutner-Fischer 
>>  wrote:
>> 
>> On Fri, 19 May 2023 20:49:47 +
>> Qing Zhao via Gcc-patches  wrote:
>> 
>>> GCC extension accepts the case when a struct with a flexible array member
>>> is embedded into another struct or union (possibly recursively).
>> 
>> Do you mean TYPE_TRAILING_FLEXARRAY()?
>
>The following might be more accurate description:
>
>GCC extension accepts the case when a struct with a flexible array member
> is embedded into another struct or union (possibly recursively) as the last 
> field.
>
>
>
>> 
>>> diff --git a/gcc/tree.h b/gcc/tree.h
>>> index 0b72663e6a1..237644e788e 100644
>>> --- a/gcc/tree.h
>>> +++ b/gcc/tree.h
>>> @@ -786,7 +786,12 @@ extern void omp_clause_range_check_failed (const_tree, 
>>> const char *, int,
>>>(...) prototype, where arguments can be accessed with va_start and
>>>va_arg), as opposed to an unprototyped function.  */
>>> #define TYPE_NO_NAMED_ARGS_STDARG_P(NODE) \
>>> -  (TYPE_CHECK (NODE)->type_common.no_named_args_stdarg_p)
>>> +  (FUNC_OR_METHOD_CHECK (NODE)->type_common.no_named_args_stdarg_p)
>>> +
>>> +/* True if this RECORD_TYPE or UNION_TYPE includes a flexible array member
>>> +   at the last field recursively.  */
>>> +#define TYPE_INCLUDE_FLEXARRAY(NODE) \
>>> +  (RECORD_OR_UNION_CHECK (NODE)->type_common.no_named_args_stdarg_p)
>> 
>> Until i read the description above i read TYPE_INCLUDE_FLEXARRAY as an
>> option to include or not include something. The description hints more
>> at TYPE_INCLUDES_FLEXARRAY (with an S) to be a type which has at least
>> one member which has a trailing flexible array or which itself has a
>> trailing flexible array.
>
>Yes, TYPE_INCLUDES_FLEXARRAY (maybe with a S is a better name) means the 
>structure/union TYPE includes a flexible array member or includes a struct 
>with a flexible array member as the last field.
>

So ANY_TRAILING_FLEXARRAY or TYPE_CONTAINS_FLEXARRAY, TYPE_INCLUDES_FLEXARRAY 
or something like that would be more clear, i don't know.
I'd probably use the first, but that's enough bike shedding for me now. Let's 
see what others think.

thanks,

>Hope this is clear.
>thanks.
>
>Qing
>> 
>>> 
>>> /* In an IDENTIFIER_NODE, this means that assemble_name was called with
>>>this string as an argument.  */
>> 
>

Re: [i386 PATCH] A minor code clean-up: Use NULL_RTX instead of nullptr

[Bernhard Reutner-Fischer via Gcc-patches]

On Wed, 24 May 2023 18:54:06 +0100
"Roger Sayle"  wrote:

> My understanding is that GCC's preferred null value for rtx is NULL_RTX
> (and for tree is NULL_TREE), and by being typed allows strict type checking,
> and use with function polymorphism and template instantiation.
> C++'s nullptr is preferred over NULL and 0 for pointer types that don't
> have a defined null of the correct type.
> 
> This minor clean-up uses NULL_RTX consistently in i386-expand.cc.

Oh. Well, i can't resist cleanups :)

Given
$ cat /tmp/inp0.c ; echo EOF
rtx myfunc (int i, int j)
{
  rtx ret;
  if (i)
return NULL;
  if (j)
   ret = NULL;
  if (ret == NULL) {
ret = NULL_RTX;
  }
  if (!ret)
return (rtx)2;
  return NULL_RTX;
}
EOF
$ spatch --c++=11 --smpl-spacing --in-place --sp-file 
~/coccinelle/rtx-null.0.cocci /tmp/inp0.c
init_defs_builtins: /usr/bin/../lib/coccinelle/standard.h
--- /tmp/inp0.c
+++ /tmp/cocci-output-76891-58af4a-inp0.c
@@ -2,10 +2,10 @@ rtx myfunc (int i, int j)
 {
   rtx ret;
   if (i)
-return NULL;
+return NULL_RTX;
   if (j)
-   ret = NULL;
-  if (ret == NULL) {
+   ret = NULL_RTX;
+  if (ret == NULL_RTX) {
 ret = NULL_RTX;
   }
   if (!ret)
HANDLING: /tmp/inp0.c
diff = 

So you if you would feel like, someone could
find ./ \( -name "testsuite" -o -name "contrib" -o -name "examples" -o -name 
".git" -o -name "zlib" -o -name "intl" \) -prune -o \( -name "*.[chpx]*" -a 
-type f \) -exec spatch --c++=11 --smpl-spacing --in-place $opts --sp-file 
~/coccinelle/rtx-null.0.cocci {} \;
with the attached rtx-null coccinelle script.
(and handle nullptr too, and the same game for tree)

Just a thought..


rtx-null.0.cocci
Description: Binary data

Re: [i386 PATCH] A minor code clean-up: Use NULL_RTX instead of nullptr

[Bernhard Reutner-Fischer via Gcc-patches]

On Thu, 25 May 2023 18:58:04 +0200
Bernhard Reutner-Fischer  wrote:

> On Wed, 24 May 2023 18:54:06 +0100
> "Roger Sayle"  wrote:
> 
> > My understanding is that GCC's preferred null value for rtx is NULL_RTX
> > (and for tree is NULL_TREE), and by being typed allows strict type checking,
> > and use with function polymorphism and template instantiation.
> > C++'s nullptr is preferred over NULL and 0 for pointer types that don't
> > have a defined null of the correct type.
> > 
> > This minor clean-up uses NULL_RTX consistently in i386-expand.cc.  
> 
> Oh. Well, i can't resist cleanups :)

> (and handle nullptr too, and the same game for tree)

so like the attached. And
sed -e 's/RTX/TREE/g' -e 's/rtx/tree/g' \
  < ~/coccinelle/gcc-rtx-null.0.cocci \
  > ~/coccinelle/gcc-tree-null.0.cocci

I do not know if we want to shorten explicit NULL comparisons.
 foo == NULL => !foo and foo != NULL => foo
Left them alone in the form they were written.

See the attached result of the rtx hunks, someone would have to build
it and hack git-commit-mklog.py --changelog 'Use NULL_RTX.'
to print("{}.".format(random.choice(['Ditto', 'Same', 'Likewise']))) ;)

> 
> Just a thought..

cheers,


gcc-rtx-null.0.cocci
Description: Binary data
diff --git a/gcc/alias.cc b/gcc/alias.cc
index 7dc7e06de07..f1925ab3de2 100644
--- a/gcc/alias.cc
+++ b/gcc/alias.cc
@@ -1725,7 +1725,7 @@ get_reg_known_value (unsigned int regno)
   if (regno < vec_safe_length (reg_known_value))
 	return (*reg_known_value)[regno];
 }
-  return NULL;
+  return NULL_RTX;
 }
 
 /* Set it.  */
diff --git a/gcc/auto-inc-dec.cc b/gcc/auto-inc-dec.cc
index 1486e8c679a..568fae7b906 100644
--- a/gcc/auto-inc-dec.cc
+++ b/gcc/auto-inc-dec.cc
@@ -428,7 +428,7 @@ move_dead_notes (rtx_insn *to_insn, rtx_insn *from_insn, rtx pattern)
 {
   rtx note;
   rtx next_note;
-  rtx prev_note = NULL;
+  rtx prev_note = NULL_RTX;
 
   for (note = REG_NOTES (from_insn); note; note = next_note)
 {
diff --git a/gcc/bb-reorder.cc b/gcc/bb-reorder.cc
index 615d5426a34..e42e4593a6a 100644
--- a/gcc/bb-reorder.cc
+++ b/gcc/bb-reorder.cc
@@ -1477,7 +1477,7 @@ sjlj_fix_up_crossing_landing_pad (basic_block old_bb)
 	rtx_insn *insn = BB_END (e->src);
 	rtx note = find_reg_note (insn, REG_EH_REGION, NULL_RTX);
 
-	gcc_assert (note != NULL);
+	gcc_assert (note != NULL_RTX);
 	const unsigned old_index = INTVAL (XEXP (note, 0));
 
 	/* Generate the new landing-pad structure.  */
@@ -1525,7 +1525,7 @@ dw2_fix_up_crossing_landing_pad (eh_landing_pad old_lp, basic_block old_bb)
 	rtx_insn *insn = BB_END (e->src);
 	rtx note = find_reg_note (insn, REG_EH_REGION, NULL_RTX);
 
-	gcc_assert (note != NULL);
+	gcc_assert (note != NULL_RTX);
 	gcc_checking_assert (INTVAL (XEXP (note, 0)) == old_lp->index);
 	XEXP (note, 0) = GEN_INT (new_lp->index);
 
diff --git a/gcc/builtins.cc b/gcc/builtins.cc
index 8400adaf5b4..48df3d4d193 100644
--- a/gcc/builtins.cc
+++ b/gcc/builtins.cc
@@ -985,7 +985,7 @@ expand_builtin_setjmp_receiver (rtx receiver_label)
 	}
 }
 
-  if (receiver_label != NULL && targetm.have_builtin_setjmp_receiver ())
+  if (receiver_label != NULL_RTX && targetm.have_builtin_setjmp_receiver ())
 emit_insn (targetm.gen_builtin_setjmp_receiver (receiver_label));
   else if (targetm.have_nonlocal_goto_receiver ())
 emit_insn (targetm.gen_nonlocal_goto_receiver ());
@@ -4118,7 +4118,7 @@ expand_builtin_strncpy (tree exp, rtx target)
 static rtx
 gen_memset_value_from_prev (by_pieces_prev *prev, fixed_size_mode mode)
 {
-  rtx target = nullptr;
+  rtx target = NULL_RTX;
   if (prev != nullptr && prev->data != nullptr)
 {
   /* Use the previous data in the same mode.  */
@@ -4179,7 +4179,7 @@ gen_memset_value_from_prev (by_pieces_prev *prev, fixed_size_mode mode)
 		break;
 		  }
 	  }
-	  if (target == nullptr)
+	  if (target == NULL_RTX)
 	prev_rtx = copy_to_reg (prev_rtx);
 	}
 
@@ -4203,7 +4203,7 @@ builtin_memset_read_str (void *data, void *prev,
 
   rtx target = gen_memset_value_from_prev ((by_pieces_prev *) prev,
 	   mode);
-  if (target != nullptr)
+  if (target != NULL_RTX)
 return target;
   rtx src = gen_int_mode (*c, QImode);
 
@@ -4250,7 +4250,7 @@ builtin_memset_gen_str (void *data, void *prev,
 return (rtx) data;
 
   target = gen_memset_value_from_prev ((by_pieces_prev *) prev, mode);
-  if (target != nullptr)
+  if (target != NULL_RTX)
 return target;
 
   if (VECTOR_MODE_P (mode))
@@ -6278,11 +6278,11 @@ expand_builtin_atomic_compare_exchange (machine_mode mode, tree exp,
 is_weak = true;
 
   if (target == const0_rtx)
-target = NULL;
+target = NULL_RTX;
 
   /* Lest the rtl backend create a race condition with an imporoper store
  to memory, always create a new pseudo for OLDVAL.  */
-  oldval = NULL;
+  oldval = NULL_RTX;
 
   if (!expand_atomic_compare_and_swap (&target, &oldval, mem, expect, desired,
    is_weak, success, failure))
@@ -6387,8 +6387,8 @@ expand_ifn_atomic_compare_exchange (gcal

Re: [PATCH] PR52665 do not let .ident confuse assembler scan tests

[Bernhard Reutner-Fischer via Gcc-patches]

On Wed, 5 Sep 2018 17:32:04 +0200
Bernhard Reutner-Fischer  wrote:

> On Tue, 21 Jun 2016 at 00:19, Jeff Law  wrote:
> >
> > On 06/18/2016 01:31 PM, Bernhard Reutner-Fischer wrote:  

> > > gcc/testsuite/ChangeLog
> > >
> > > 2016-06-18  Bernhard Reutner-Fischer  
> > >
> > >   PR testsuite/52665
> > >   * lib/gcc-dg.exp (gcc-dg-test-1): Iterate over _required_options.
> > >   * lib/target-supports.exp (scan-assembler_required_options,
> > >   scan-assembler-not_required_options,
> > >   scan-assembler-times_required_options): Add -fno-ident.
> > >   * lib/scanasm.exp (scan-assembler-times): Fix error message.
> > >   * c-c++-common/ident-0a.c: New test.
> > >   * c-c++-common/ident-0b.c: New test.
> > >   * c-c++-common/ident-1a.c: New test.
> > >   * c-c++-common/ident-1b.c: New test.
> > >   * c-c++-common/ident-2a.c: New test.
> > >   * c-c++-common/ident-2b.c: New test.
> > >
> > > Ok for trunk?
> > >
> > > PS: proc force_conventional_output_for would be a bit misnomed by this,
> > > not sure if it should be renamed to maybe set_required_options_for or
> > > the like?  
> > OK.  
> 
> Now applied without the rename to trunk as r264128.
> 
> thanks,
> 
> >
> > Changing force_conventional_output to set_required_options_for is
> > pre-approved as well.

I've now applied the renaming as r14-1449-g994195b597ff20
thanks,

> >
> > jeff
> >  

Re: [PATCH v4] tree-ssa-sink: Improve code sinking pass

[Bernhard Reutner-Fischer via Gcc-patches]

Hi!

On Wed, 31 May 2023 12:31:35 +0530
Ajit Agarwal via Gcc-patches  wrote:

> Hello All:
> 
> This patch improves code sinking pass to sink statements before call to reduce
> register pressure.
> Review comments are incorporated.
> 
> For example :
> 
> void bar();
> int j;
> void foo(int a, int b, int c, int d, int e, int f)
> {
>   int l;
>   l = a + b + c + d +e + f;
>   if (a != 5)
> {
>   bar();
>   j = l;
> }
> }
> 
> Code Sinking does the following:
> 
> void bar();
> int j;
> void foo(int a, int b, int c, int d, int e, int f)
> {
>   int l;
>   
>   if (a != 5)
> {
>   l = a + b + c + d +e + f; 
>   bar();
>   j = l;
> }
> }
> 
> Bootstrapped regtested on powerpc64-linux-gnu.
> 
> Thanks & Regards
> Ajit
> 
> tree-ssa-sink: Improve code sinking pass
> 
> Code Sinking sinks the blocks after call.This increases register pressure
> for callee-saved registers. Improves code sinking before call in the use
> blocks or immediate dominator of use blocks.
> 
> 2023-05-24  Ajit Kumar Agarwal  
> 
> gcc/ChangeLog:
> 
>   * tree-ssa-sink.cc (statement_sink_location): Move statements before
>   calls.
>   (def_use_same_block): New function.
>   (select_best_block): Add heuristics to select the best blocks in the
>   immediate post dominator.
> 
> gcc/testsuite/ChangeLog:
> 
>   * gcc.dg/tree-ssa/ssa-sink-20.c: New testcase.
>   * gcc.dg/tree-ssa/ssa-sink-21.c: New testcase.
> ---
>  gcc/testsuite/gcc.dg/tree-ssa/ssa-sink-20.c | 15 +
>  gcc/testsuite/gcc.dg/tree-ssa/ssa-sink-21.c | 19 ++
>  gcc/tree-ssa-sink.cc| 74 +
>  3 files changed, 96 insertions(+), 12 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/ssa-sink-20.c
>  create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/ssa-sink-21.c
> 
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ssa-sink-20.c 
> b/gcc/testsuite/gcc.dg/tree-ssa/ssa-sink-20.c
> new file mode 100644
> index 000..49d5019ab93
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/ssa-sink-20.c
> @@ -0,0 +1,15 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fdump-tree-optimized -fdump-tree-sink-stats" } */

You don't test the optimized dump, do you?

> +void bar();
> +int j;
> +void foo(int a, int b, int c, int d, int e, int f)
> +{
> +  int l;
> +  l = a + b + c + d +e + f;
> +  if (a != 5)
> +{
> +  bar();
> +  j = l;
> +}
> +}
> +/* { dg-final { scan-tree-dump 
> {l_12\s+=\s+_4\s+\+\s+f_11\(D\);\n\s+bar\s+\(\)} sink1 } } */
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ssa-sink-21.c 
> b/gcc/testsuite/gcc.dg/tree-ssa/ssa-sink-21.c
> new file mode 100644
> index 000..84e7938c54f
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/ssa-sink-21.c
> @@ -0,0 +1,19 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fdump-tree-sink-stats" } */
> +void bar();
> +int j, x;
> +void foo(int a, int b, int c, int d, int e, int f)
> +{
> +  int l;
> +  l = a + b + c + d +e + f;
> +  if (a != 5)
> +{
> +  bar();
> +  if (b != 3)
> +x = 3;
> +  else
> +x = 5;
> +  j = l;
> +}
> +}
> +/* { dg-final { scan-tree-dump 
> {l_13\s+=\s+_4\s+\+\s+f_12\(D\);\n\s+bar\s+\(\)} sink1 } } */
> diff --git a/gcc/tree-ssa-sink.cc b/gcc/tree-ssa-sink.cc
> index b1ba7a2ad6c..ee8988bbb2c 100644
> --- a/gcc/tree-ssa-sink.cc
> +++ b/gcc/tree-ssa-sink.cc
> @@ -171,9 +171,28 @@ nearest_common_dominator_of_uses (def_operand_p def_p, 
> bool *debug_stmts)
>return commondom;
>  }
>  
> +/* Return TRUE if immediate uses of the defs in
> +   STMT occur in the same block as STMT, FALSE otherwise.  */
> +
> +bool
> +def_use_same_block (gimple *stmt)

Looks like this function should be static.

> +{
> +  def_operand_p def;
> +  ssa_op_iter iter;
> +
> +  FOR_EACH_SSA_DEF_OPERAND (def, stmt, iter, SSA_OP_DEF)
> +{
> +  gimple *def_stmt = SSA_NAME_DEF_STMT (DEF_FROM_PTR (def));
> +  if ((gimple_bb (def_stmt) == gimple_bb (stmt)))
> + return true;
> + }
> +  return false;
> +}
> +
>  /* Given EARLY_BB and LATE_BB, two blocks in a path through the dominator
> tree, return the best basic block between them (inclusive) to place
> -   statements.
> +   statements. The best basic block should be in immediate dominator of
> +   best basic block if the use stmt is after the call.

s/in/an/ ?

>  
> We want the most control dependent block in the shallowest loop nest.
>  
> @@ -190,7 +209,8 @@ nearest_common_dominator_of_uses (def_operand_p def_p, 
> bool *debug_stmts)
>  static basic_block
>  select_best_block (basic_block early_bb,
>  basic_block late_bb,
> -gimple *stmt)
> +gimple *stmt,
> +gimple *use)
>  {
>basic_block best_bb = late_bb;
>basic_block temp_bb = late_bb;
> @@ -230,14 +250,46 @@ select_best_block (basic_block early_bb,
>if (threshold > 100)
>   threshold = 100;
>  }
> -

superfluous whitespace change?

Re: [PATCH] jump: Change return type of predicate functions from int to bool

[Bernhard Reutner-Fischer via Gcc-patches]

On Wed, 31 May 2023 09:40:24 +0200
Uros Bizjak via Gcc-patches  wrote:

> On Wed, May 31, 2023 at 9:17 AM Richard Biener
>  wrote:

> > Do we have a diagnostic that would point out places we
> > assign the bool result to an integer variable?  Do we want
> > to change those places as well (did you intend to or restrict
> > the changes to functions only used in conditional context?)  
> 
> FWIW, I'm going through candidate files by hand, looking for predicate
> functions that return 0/1. The candidate files are the ones mentioned
> in rtl.h. In addition, I am doing some drive-by cleanups in candidate
> files.

I've scratched
https://inbox.sourceware.org/gcc-patches/20221112234543.95441-5-al...@gcc.gnu.org/
https://inbox.sourceware.org/gcc-patches/20221112234543.95441-6-al...@gcc.gnu.org/

to generate patches.
You had to manually adjust the declarations to match the patched
definitions, and i did not change the type of local variables feeding
into the return automatically.

But it helped find some low hanging fruit quickly.

HTH and cheers,

Re: PATCH v5 4/4] ree: Improve ree pass for rs6000 target using defined ABI interfaces.

[Bernhard Reutner-Fischer via Gcc-patches]

On Thu, 1 Jun 2023 10:53:02 +0530
Ajit Agarwal via Gcc-patches  wrote:

> Hello All:
> 
> This new version of patch 4 use improve ree pass for rs6000 target using 
> defined ABI interfaces.
> Bootstrapped and regtested on power64-linux-gnu.
> 
> Review comments incorporated.

Not a review:

/scratch/src/gcc-14.mine$ ./contrib/check_GNU_style.py /tmp/rs6k-ree-v5.patch 
=== ERROR type #1: there should be exactly one space between function name and 
parenthesis (2 error(s)) ===
gcc/ree.cc:1461:33:   if (state.defs_list.length() != 0)
gcc/ree.cc:1487:42:   if ((i+1) < reinsn_copy_list.length())

=== ERROR type #2: trailing operator (1 error(s)) ===
gcc/ree.cc:761:24:  machine_mode tgt_mode =

$ grep -E "^[-+].*[^[:space:]\.][[:space:]][[:space:]]" /tmp/rs6k-ree-v5.patch 
+   an return  registers.  */
+  if (GET_CODE (SET_SRC (set)) !=  ZERO_EXTEND)
+  if (GET_CODE (SET_SRC (set)) !=  ZERO_EXTEND)
+  if (code !=  ZERO_EXTEND)

And maybe you could add fewer empty lines in combine_reaching_defs()

thanks,

Re: [PATCH v5] tree-ssa-sink: Improve code sinking pass

[Bernhard Reutner-Fischer via Gcc-patches]

On 1 June 2023 09:20:08 CEST, Ajit Agarwal  wrote:
>Hello All:
>
>This patch improves code sinking pass to sink statements before call to reduce
>register pressure.
>Review comments are incorporated.

Hi Ajit!

I had two comments for v4 that you did not address in v5 or followed up.
thanks,

Re: [PATCH 04/14] c++: use _P() defines from tree.h

[Bernhard Reutner-Fischer via Gcc-patches]

On Thu, 1 Jun 2023 11:24:06 -0400
Patrick Palka  wrote:

> On Sat, May 13, 2023 at 7:26 PM Bernhard Reutner-Fischer via
> Gcc-patches  wrote:

> > diff --git a/gcc/cp/tree.cc b/gcc/cp/tree.cc
> > index 131b212ff73..19dfb3ed782 100644
> > --- a/gcc/cp/tree.cc
> > +++ b/gcc/cp/tree.cc
> > @@ -1173,7 +1173,7 @@ build_cplus_array_type (tree elt_type, tree 
> > index_type, int dependent)
> >  }
> >
> >/* Avoid spurious warnings with VLAs (c++/54583).  */
> > -  if (TYPE_SIZE (t) && EXPR_P (TYPE_SIZE (t)))
> > +  if (CAN_HAVE_LOCATION_P (TYPE_SIZE (t)))  
> 
> Hmm, this change seems undesirable...

mhm, yes that is misleading. I'll prepare a patch to revert this.
Let me have a look if there were other such CAN_HAVE_LOCATION_P changes
that we'd want to revert.

thanks,

Re: [PATCH 04/14] c++: use _P() defines from tree.h

[Bernhard Reutner-Fischer via Gcc-patches]

Hi David, Patrick,

On Thu, 1 Jun 2023 18:33:46 +0200
Bernhard Reutner-Fischer  wrote:

> On Thu, 1 Jun 2023 11:24:06 -0400
> Patrick Palka  wrote:
> 
> > On Sat, May 13, 2023 at 7:26 PM Bernhard Reutner-Fischer via
> > Gcc-patches  wrote:  
> 
> > > diff --git a/gcc/cp/tree.cc b/gcc/cp/tree.cc
> > > index 131b212ff73..19dfb3ed782 100644
> > > --- a/gcc/cp/tree.cc
> > > +++ b/gcc/cp/tree.cc
> > > @@ -1173,7 +1173,7 @@ build_cplus_array_type (tree elt_type, tree 
> > > index_type, int dependent)
> > >  }
> > >
> > >/* Avoid spurious warnings with VLAs (c++/54583).  */
> > > -  if (TYPE_SIZE (t) && EXPR_P (TYPE_SIZE (t)))
> > > +  if (CAN_HAVE_LOCATION_P (TYPE_SIZE (t)))
> > 
> > Hmm, this change seems undesirable...  
> 
> mhm, yes that is misleading. I'll prepare a patch to revert this.
> Let me have a look if there were other such CAN_HAVE_LOCATION_P changes
> that we'd want to revert.

Sorry for that!
I'd revert the hunk above and the one in gcc-rich-location.cc
(maybe_range_label_for_tree_type_mismatch::get_text), please see
attached. Bootstrap running, ok for trunk if it passes?

thanks,
>From 322bce380144b5199cca5775f7a3f0fb30a219ae Mon Sep 17 00:00:00 2001
From: Bernhard Reutner-Fischer 
Date: Thu, 1 Jun 2023 19:44:19 +0200
Subject: [PATCH] c++, analyzer: Expand CAN_HAVE_LOCATION_P macro.

r14-985-gca2007a9bb3074 used the collapsed macro definition
CAN_HAVE_LOCATION_P in gcc-rich-location.cc and r14-977-g8861c80733da5c
in c++'s build_cplus_array_type ().
However, although otherwise correct, the usage of CAN_HAVE_LOCATION_P
in these two spots is misleading, so this patch reverts aforementioned
two hunks.

gcc/cp/ChangeLog:

* tree.cc (build_cplus_array_type): Revert using the macro
CAN_HAVE_LOCATION_P.

gcc/ChangeLog:

* gcc-rich-location.cc 
(maybe_range_label_for_tree_type_mismatch::get_text):
Revert using the macro CAN_HAVE_LOCATION_P.
---
 gcc/cp/tree.cc   | 2 +-
 gcc/gcc-rich-location.cc | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/gcc/cp/tree.cc b/gcc/cp/tree.cc
index 19dfb3ed782..9363166152a 100644
--- a/gcc/cp/tree.cc
+++ b/gcc/cp/tree.cc
@@ -1173,7 +1173,7 @@ build_cplus_array_type (tree elt_type, tree index_type, 
int dependent)
 }
 
   /* Avoid spurious warnings with VLAs (c++/54583).  */
-  if (CAN_HAVE_LOCATION_P (TYPE_SIZE (t)))
+  if (TYPE_SIZE (t) && EXPR_P (TYPE_SIZE (t)))
 suppress_warning (TYPE_SIZE (t), OPT_Wunused);
 
   /* Push these needs up to the ARRAY_TYPE so that initialization takes
diff --git a/gcc/gcc-rich-location.cc b/gcc/gcc-rich-location.cc
index edecf07f81e..d02a5144cc6 100644
--- a/gcc/gcc-rich-location.cc
+++ b/gcc/gcc-rich-location.cc
@@ -200,7 +200,7 @@ maybe_range_label_for_tree_type_mismatch::get_text 
(unsigned range_idx) const
   tree expr_type = TREE_TYPE (m_expr);
 
   tree other_type = NULL_TREE;
-  if (CAN_HAVE_LOCATION_P (m_other_expr))
+  if (m_other_expr && EXPR_P (m_other_expr))
 other_type = TREE_TYPE (m_other_expr);
 
   range_label_for_type_mismatch inner (expr_type, other_type);
-- 
2.30.2

Re: [PATCH] libatomic: x86_64: Always try ifunc

[Bernhard Reutner-Fischer via Gcc-patches]

On 3 June 2023 13:25:32 CEST, Xi Ruoyao via Gcc-patches 
 wrote:

>There seems no good way to check if the CPU is Intel or AMD from
>the built-in macros (maybe we can check every known model like __skylake,
>__bdver2, ..., but it will be very error-prune and require an update
>whenever we add the support for a new x86 model).  The best thing we can
>do seems "always try ifunc" here.

IIRC there is __builtin_cpu_is (after initialisation) -- A couple of days ago, 
we wondered if it would be handy to lower that even in fortran without going 
through C, so i am pretty sure I don't make that up.. ;-)

Just a thought,

Re: [PATCH] libatomic: x86_64: Always try ifunc

[Bernhard Reutner-Fischer via Gcc-patches]

On 3 June 2023 15:46:02 CEST, Xi Ruoyao  wrote:

>Unfortunately __builtin_cpu_is performs CPU detection on runtime, not
>compile time.

Right, you were talking about configure, sorry.

Re: [PATCH] Fortran 2018 rounding modes changes

[Bernhard Reutner-Fischer via Gcc-patches]

On 31 August 2022 20:29:12 CEST, FX via Fortran  wrote:


+  case GFC_FPE_GFC_FPE_AWAY:

typo?

thanks,

Re: [PATCH] Fortran: add IEEE_QUIET_* and IEEE_SIGNALING_* comparisons

[Bernhard Reutner-Fischer via Gcc-patches]

On 2 September 2022 13:37:41 CEST, FX via Fortran  wrote:
>Hi,

Please do not call the non-standard abort, but use stop N.

IIRC I once had a trivial script.. 
https://www.mail-archive.com/search?l=gcc-patches@gcc.gnu.org&q=subject:%22%5C%5BPATCH%2C+OpenACC%5C%5D+Fortran+deviceptr%22&o=newest&f=1

---8<---
Like (modulo typos, untested):
$ cat abort_to_stop.awk ; echo EOF
# awk -f ./abort_to_stop.awk < foo.f90 > x && mv x foo.f90
BEGIN { IGNORECASE = 1; i = 1 } { while (sub(/call\s\s*abort/, "stop " i)) {let 
i++;}; print $0; }
EOF

HTH and thanks,

Re: [PATCH] Fortran: add IEEE_QUIET_* and IEEE_SIGNALING_* comparisons

[Bernhard Reutner-Fischer via Gcc-patches]

On 2 September 2022 17:54:00 CEST, FX  wrote:
>Hi Bernhard,
>
>> Please do not call the non-standard abort, but use stop N.
>
>Is there a specific reason? It’s a well-documented GNU extension, and it’s 
>useful because it can easily display a backtrace and give line info for the 
>failure, unlike STOP.
>I’ll replace if there is consensus, but apart from aesthetics I don’t see why.


IIRC there was discussion about abort on the ML some years ago where folks 
decided to switch to stop N.
I don't think I participated in that discussion, maybe somebody remembers the 
reasoning or is able to find the thread.

thanks,

Re: [PATCH] tree-optimization/109237 - last_stmt is possibly slow

[Bernhard Reutner-Fischer via Gcc-patches]

On 22 March 2023 13:29:52 CET, Richard Biener via Gcc-patches 
 wrote:

>The alternative would be to change last_stmt, explicitely introducing
>last_nondebug_stmt.  I remember we chickened out and made last_stmt
>conservative here but not anticipating the compile-time issues this
>creates.  I count 227 last_stmt and 12 last_and_only_stmt uses.

In https://inbox.sourceware.org/gcc-help/20211121010713.1452267f@nbbrfq/ i 
asked if maybe
---8<---
1) last_stmt
wouldn't it be more efficient if tree-cfg.c:: last_stmt() would
gimple_seq_last_nondebug_stmt (bb_seq (bb)) ? That would set stmt=NULL
only after the loop it seems..

Re: [PATCH v2] rtl-optimization: ppc backend generates unnecessary extension.

[Bernhard Reutner-Fischer via Gcc-patches]

On Thu, 30 Mar 2023 17:30:43 +0530
Ajit Agarwal via Gcc-patches  wrote:

> Hello All:

Just some nits.

And this seems to be an incremental diff ontop of your v2.
You might want check for coding-style errors by running:
 contrib/check_GNU_style.py /tmp/ree-v2bis.patch

> 
> This patch added enhancement to extimination elimination for rs6000 target.

I think REE means redundant extension elimination.

> gcc/ChangeLog:
> 
>   * ree.cc(is_feasible_elim_across_basic_blocks):

We often use the lispy _p suffix for predicates.
Maybe eliminate_across_bbs_p would be shorter.

>   Add checks to enable extension elimination..
>   * ree.cc(combine_reaching_defs): Add checks to enable
>   extension elimination.
> ---
>  gcc/ree.cc | 121 ++---
>  1 file changed, 116 insertions(+), 5 deletions(-)
> 
> diff --git a/gcc/ree.cc b/gcc/ree.cc
> index d05d37f9a23..7e4cce5cee4 100644
> --- a/gcc/ree.cc
> +++ b/gcc/ree.cc
> @@ -253,6 +253,56 @@ struct ext_cand
>  
>  static int max_insn_uid;
>  
> +/* Get all the reaching definitions of an instruction.  The definitions are
> +   desired for REG used in INSN.  Return the definition list or NULL if a
> +   definition is missing.  If DEST is non-NULL, additionally push the INSN
> +   of the definitions onto DEST.  */
> +
> +static struct df_link *
> +get_defs (rtx_insn *insn, rtx reg, vec *dest)
> +{
> +  df_ref use;
> +  struct df_link *ref_chain, *ref_link;
> +
> +  FOR_EACH_INSN_USE (use, insn)
> +{
> +  if (GET_CODE (DF_REF_REG (use)) == SUBREG)
> + return NULL;
> +  if (REGNO (DF_REF_REG (use)) == REGNO (reg))
> + break;
> +}
> +
> +  if (use == NULL) return NULL;

Missing newline before return.

> +
> +  gcc_assert (use != NULL);

Either the assert or the if but both is a bit much.

> +
> +  ref_chain = DF_REF_CHAIN (use);
> +
> +  for (ref_link = ref_chain; ref_link; ref_link = ref_link->next)
> +{
> +  /* Problem getting some definition for this instruction.  */
> +  if (ref_link->ref == NULL)
> + return NULL;
> +  if (DF_REF_INSN_INFO (ref_link->ref) == NULL)
> + return NULL;
> +  /* As global regs are assumed to be defined at each function call
> +  dataflow can report a call_insn as being a definition of REG.
> +  But we can't do anything with that in this pass so proceed only
> +  if the instruction really sets REG in a way that can be deduced
> +  from the RTL structure.  */
> +  if (global_regs[REGNO (reg)]
> +   && !set_of (reg, DF_REF_INSN (ref_link->ref)))
> + return NULL;
> +}
> +
> +  if (dest)
> +for (ref_link = ref_chain; ref_link; ref_link = ref_link->next)
> +  dest->safe_push (DF_REF_INSN (ref_link->ref));
> +
> +  return ref_chain;
> +}
> +
> +
>  /* Identify instruction AND with identical zero extension.  */
>  
>  static unsigned int
> @@ -393,6 +443,7 @@ combine_set_extension (ext_cand *cand, rtx_insn 
> *curr_insn, rtx *orig_set)
>machine_mode orig_mode = GET_MODE (SET_DEST (*orig_set));
>rtx new_set = NULL_RTX;
>rtx cand_pat = single_set (cand->insn);
> +

superfluous whitespace change.

>if (insn_is_zext_p(cand->insn)
> && CONST_INT_P (orig_src) && INTVAL (orig_src) != 0)
>  return false;
> @@ -401,6 +452,7 @@ combine_set_extension (ext_cand *cand, rtx_insn 
> *curr_insn, rtx *orig_set)
>   then we need to change the original load to reference the destination
>   of the extension.  Then we need to emit a copy from that destination
>   to the original destination of the load.  */
> + // print_rtl_single(stdout, cand_pat);

Please remove that debug comment.

>rtx new_reg;
>bool copy_needed
>  = (REGNO (SET_DEST (cand_pat)) != REGNO (XEXP (SET_SRC (cand_pat), 0)));
> @@ -547,6 +599,7 @@ transform_ifelse (ext_cand *cand, rtx_insn *def_insn)
>return false;
>  }
>  
> +#if 0
>  /* Get all the reaching definitions of an instruction.  The definitions are
> desired for REG used in INSN.  Return the definition list or NULL if a
> definition is missing.  If DEST is non-NULL, additionally push the INSN
> @@ -593,7 +646,7 @@ get_defs (rtx_insn *insn, rtx reg, vec *dest)
>  
>return ref_chain;
>  }
> -
> +#endif

Why did you move get_defs?

>  /* Get all the reaching uses of an instruction.  The uses are desired for REG
> set in INSN.  Return use list or NULL if a use is missing or irregular.  
> */
>  
> @@ -848,6 +901,36 @@ is_feasible_elim_across_basic_blocks (ext_cand *cand,
>   && REGNO (XEXP (PATTERN (BB_END (bb)), 0)) != REGNO (SET_DEST 
> (cand->expr)))
>   return false;
>  
> +   if (cand->code == ZERO_EXTEND
> + && GET_CODE ((PATTERN (def_insn))) == PARALLEL)
> + return false;

Excess braces above.

> +
> +   if (cand->code == ZERO_EXTEND
> + && GET_CODE ((PATTERN (def_insn))) == SET

Excess braces above.

> + && GET_CODE (SET_SRC (PATTERN (def_insn))) != XOR)
> + return false;
> +
> +   if (cand->code =

[PATCH 0/3] Fix mpfr and mpz memory leaks

[Bernhard Reutner-Fischer via Gcc-patches]

From: Bernhard Reutner-Fischer 

Hi!

These patches for the go, rust and fortran frontends fix the mpfr and
mpz memory leaks my coccinelle script found.

Bootstrapped and regtested without regressions on x86_64-unknown-linux,
including go,rust,fortran.

Ok for trunk for the fortran bits?

I'd hope that go and rust can apply the respective patches upstream.

Bernhard Reutner-Fischer (3):
  go: Fix memory leak in Integer_expression
  rust: Fix memory leak in compile_{integer,float}_literal
  Fortran: Fix mpz and mpfr memory leaks

 gcc/fortran/array.cc  | 3 +++
 gcc/fortran/expr.cc   | 8 
 gcc/fortran/simplify.cc   | 7 ++-
 gcc/go/gofrontend/expressions.cc  | 5 +
 gcc/rust/backend/rust-compile-expr.cc | 7 +++
 5 files changed, 25 insertions(+), 5 deletions(-)

-- 
2.30.2

[PATCH 3/3] Fortran: Fix mpz and mpfr memory leaks

[Bernhard Reutner-Fischer via Gcc-patches]

From: Bernhard Reutner-Fischer 

Cc: fort...@gcc.gnu.org

gcc/fortran/ChangeLog:

* array.cc (gfc_ref_dimen_size): Free mpz memory before ICEing.
* expr.cc (find_array_section): Fix mpz memory leak.
* simplify.cc (gfc_simplify_reshape): Fix mpz memory leaks in
error paths.
(gfc_simplify_set_exponent): Fix mpfr memory leak.
---
 gcc/fortran/array.cc| 3 +++
 gcc/fortran/expr.cc | 8 
 gcc/fortran/simplify.cc | 7 ++-
 3 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/gcc/fortran/array.cc b/gcc/fortran/array.cc
index be5eb8b6a0f..8b1e816a859 100644
--- a/gcc/fortran/array.cc
+++ b/gcc/fortran/array.cc
@@ -2541,6 +2541,9 @@ gfc_ref_dimen_size (gfc_array_ref *ar, int dimen, mpz_t 
*result, mpz_t *end)
   return t;
 
 default:
+  mpz_clear (lower);
+  mpz_clear (stride);
+  mpz_clear (upper);
   gfc_internal_error ("gfc_ref_dimen_size(): Bad dimen_type");
 }
 
diff --git a/gcc/fortran/expr.cc b/gcc/fortran/expr.cc
index 7fb33f81788..b4736804eda 100644
--- a/gcc/fortran/expr.cc
+++ b/gcc/fortran/expr.cc
@@ -1539,6 +1539,7 @@ find_array_section (gfc_expr *expr, gfc_ref *ref)
   mpz_init_set_ui (delta_mpz, one);
   mpz_init_set_ui (nelts, one);
   mpz_init (tmp_mpz);
+  mpz_init (ptr);
 
   /* Do the initialization now, so that we can cleanup without
  keeping track of where we were.  */
@@ -1682,7 +1683,6 @@ find_array_section (gfc_expr *expr, gfc_ref *ref)
   mpz_mul (delta_mpz, delta_mpz, tmp_mpz);
 }
 
-  mpz_init (ptr);
   cons = gfc_constructor_first (base);
 
   /* Now clock through the array reference, calculating the index in
@@ -1735,7 +1735,8 @@ find_array_section (gfc_expr *expr, gfc_ref *ref)
 "at %L requires an increase of the allowed %d "
 "upper limit.  See %<-fmax-array-constructor%> "
 "option", &expr->where, flag_max_array_constructor);
- return false;
+ t = false;
+ goto cleanup;
}
 
   cons = gfc_constructor_lookup (base, limit);
@@ -1750,8 +1751,6 @@ find_array_section (gfc_expr *expr, gfc_ref *ref)
   gfc_copy_expr (cons->expr), NULL);
 }
 
-  mpz_clear (ptr);
-
 cleanup:
 
   mpz_clear (delta_mpz);
@@ -1765,6 +1764,7 @@ cleanup:
   mpz_clear (ctr[d]);
   mpz_clear (stride[d]);
 }
+  mpz_clear (ptr);
   gfc_constructor_free (base);
   return t;
 }
diff --git a/gcc/fortran/simplify.cc b/gcc/fortran/simplify.cc
index ecf0e3558df..d1f06335e79 100644
--- a/gcc/fortran/simplify.cc
+++ b/gcc/fortran/simplify.cc
@@ -6866,6 +6866,7 @@ gfc_simplify_reshape (gfc_expr *source, gfc_expr 
*shape_exp,
  gfc_error ("The SHAPE array for the RESHAPE intrinsic at %L has a "
 "negative value %d for dimension %d",
 &shape_exp->where, shape[rank], rank+1);
+ mpz_clear (index);
  return &gfc_bad_expr;
}
 
@@ -6889,6 +6890,7 @@ gfc_simplify_reshape (gfc_expr *source, gfc_expr 
*shape_exp,
{
  gfc_error ("Shapes of ORDER at %L and SHAPE at %L are different",
 &order_exp->where, &shape_exp->where);
+ mpz_clear (index);
  return &gfc_bad_expr;
}
 
@@ -6902,6 +6904,7 @@ gfc_simplify_reshape (gfc_expr *source, gfc_expr 
*shape_exp,
{
  gfc_error ("Sizes of ORDER at %L and SHAPE at %L are different",
 &order_exp->where, &shape_exp->where);
+ mpz_clear (index);
  return &gfc_bad_expr;
}
 
@@ -6918,6 +6921,7 @@ gfc_simplify_reshape (gfc_expr *source, gfc_expr 
*shape_exp,
 "in the range [1, ..., %d] for the RESHAPE intrinsic "
 "near %L", order[i], &order_exp->where, rank,
 &shape_exp->where);
+ mpz_clear (index);
  return &gfc_bad_expr;
}
 
@@ -6926,6 +6930,7 @@ gfc_simplify_reshape (gfc_expr *source, gfc_expr 
*shape_exp,
{
  gfc_error ("ORDER at %L is not a permutation of the size of "
 "SHAPE at %L", &order_exp->where, &shape_exp->where);
+ mpz_clear (index);
  return &gfc_bad_expr;
}
  x[order[i]] = 1;
@@ -7408,7 +7413,7 @@ gfc_simplify_set_exponent (gfc_expr *x, gfc_expr *i)
   exp2 = (unsigned long) mpz_get_d (i->value.integer);
   mpfr_mul_2exp (result->value.real, frac, exp2, GFC_RND_MODE);
 
-  mpfr_clears (absv, log2, pow2, frac, NULL);
+  mpfr_clears (exp, absv, log2, pow2, frac, NULL);
 
   return range_check (result, "SET_EXPONENT");
 }
-- 
2.30.2

[PATCH 1/3] go: Fix memory leak in Integer_expression

[Bernhard Reutner-Fischer via Gcc-patches]

From: Bernhard Reutner-Fischer 

Cc: Ian Lance Taylor 

gcc/go/ChangeLog:

* gofrontend/expressions.cc (Integer_expression::do_import): Fix
memory leak.
---
 gcc/go/gofrontend/expressions.cc | 5 +
 1 file changed, 5 insertions(+)

diff --git a/gcc/go/gofrontend/expressions.cc b/gcc/go/gofrontend/expressions.cc
index 4ac55af7433..93f5d5dc52b 100644
--- a/gcc/go/gofrontend/expressions.cc
+++ b/gcc/go/gofrontend/expressions.cc
@@ -2728,6 +2728,7 @@ Integer_expression::do_import(Import_expression* imp, 
Location loc)
{
  go_error_at(imp->location(), "bad number in import data: %qs",
  real_str.c_str());
+ mpfr_clear (real);
  return Expression::make_error(loc);
}
}
@@ -2743,6 +2744,8 @@ Integer_expression::do_import(Import_expression* imp, 
Location loc)
{
  go_error_at(imp->location(), "bad number in import data: %qs",
  imag_str.c_str());
+ mpfr_clear (imag);
+ mpfr_clear (real);
  return Expression::make_error(loc);
}
   mpc_t cval;
@@ -2766,6 +2769,7 @@ Integer_expression::do_import(Import_expression* imp, 
Location loc)
{
  go_error_at(imp->location(), "bad number in import data: %qs",
  num.c_str());
+ mpz_clear (val);
  return Expression::make_error(loc);
}
   Expression* ret;
@@ -2783,6 +2787,7 @@ Integer_expression::do_import(Import_expression* imp, 
Location loc)
{
  go_error_at(imp->location(), "bad number in import data: %qs",
  num.c_str());
+ mpfr_clear (val);
  return Expression::make_error(loc);
}
   Expression* ret = Expression::make_float(&val, NULL, loc);
-- 
2.30.2

[PATCH 2/3] rust: Fix memory leak in compile_{integer,float}_literal

[Bernhard Reutner-Fischer via Gcc-patches]

From: Bernhard Reutner-Fischer 

Cc: Arthur Cohen 
Cc: Philip Herron 

gcc/rust/ChangeLog:

* backend/rust-compile-expr.cc (CompileExpr::compile_integer_literal):
(CompileExpr::compile_float_literal): Fix memory leak.
---
 gcc/rust/backend/rust-compile-expr.cc | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/gcc/rust/backend/rust-compile-expr.cc 
b/gcc/rust/backend/rust-compile-expr.cc
index 436fc924a13..82078b81953 100644
--- a/gcc/rust/backend/rust-compile-expr.cc
+++ b/gcc/rust/backend/rust-compile-expr.cc
@@ -2119,6 +2119,7 @@ CompileExpr::compile_integer_literal (const 
HIR::LiteralExpr &expr,
   if (mpz_init_set_str (ival, literal_value.as_string ().c_str (), 10) != 0)
 {
   rust_error_at (expr.get_locus (), "bad number in literal");
+  mpz_clear (ival);
   return error_mark_node;
 }
 
@@ -2133,6 +2134,9 @@ CompileExpr::compile_integer_literal (const 
HIR::LiteralExpr &expr,
   rust_error_at (expr.get_locus (),
 "integer overflows the respective type %<%s%>",
 tyty->get_name ().c_str ());
+  mpz_clear (type_min);
+  mpz_clear (type_max);
+  mpz_clear (ival);
   return error_mark_node;
 }
 
@@ -2158,6 +2162,7 @@ CompileExpr::compile_float_literal (const 
HIR::LiteralExpr &expr,
   != 0)
 {
   rust_error_at (expr.get_locus (), "bad number in literal");
+  mpfr_clear (fval);
   return error_mark_node;
 }
 
@@ -2179,9 +2184,11 @@ CompileExpr::compile_float_literal (const 
HIR::LiteralExpr &expr,
   rust_error_at (expr.get_locus (),
 "decimal overflows the respective type %<%s%>",
 tyty->get_name ().c_str ());
+  mpfr_clear (fval);
   return error_mark_node;
 }
 
+  mpfr_clear (fval);
   return real_value;
 }
 
-- 
2.30.2

Re: [PATCH 2/2] Fortran: Fix memory leak in gfc_add_include_path [PR68800]

[Bernhard Reutner-Fischer via Gcc-patches]

ping?

libcpp maintainers, is the helper in incpath.* ok?

fortraners,
Do you prefer a rogue, local forward declaration, or is the
introduction of that trivial wrapper ok? I don't think pulling in cpp.h
from f95-lang.cc is desirable, but i can do that if you all think
that's preferred.

cover-letter:
https://gcc.gnu.org/pipermail/gcc-patches/2021-November/583522.html

gcc/incpath.* bits (i guess that'd be for the libcpp maintainers):
https://gcc.gnu.org/pipermail/gcc-patches/2021-November/583520.html

fortran bits:
https://gcc.gnu.org/pipermail/gcc-patches/2021-November/583521.html

thanks,

On Sun, 7 Nov 2021 02:38:21 +0100
Bernhard Reutner-Fischer  wrote:

> On Sat, 6 Nov 2021 20:22:53 +0100
> Harald Anlauf  wrote:
> 
> > Hi Bernhard,
> > 
> > I cannot comment on the gcc/ parts, but
> >   
> 
> > > diff --git a/gcc/fortran/cpp.c b/gcc/fortran/cpp.c
> > > index e86386c8b17..04fe8fe460b 100644
> > > --- a/gcc/fortran/cpp.c
> > > +++ b/gcc/fortran/cpp.c
> > > @@ -728,12 +728,20 @@ gfc_cpp_done (void)
> > > cpp_clear_file_cache (cpp_in);
> > >   }
> > 
> > why do you introduce a wrapper for something outside of fortran
> > that is used only once,
> >   
> > > -/* PATH must be malloc-ed and NULL-terminated.  */
> > > +/* Free all cpp include dirs.  */
> > > +void
> > > +gfc_cpp_free_cpp_dirs (void)
> > > +{
> > > +  free_cpp_dirs ();
> > > +}  
> 
> > > diff --git a/gcc/fortran/cpp.h b/gcc/fortran/cpp.h
> > > index 44644a2a333..963b9a9c89e 100644
> > > --- a/gcc/fortran/cpp.h
> > > +++ b/gcc/fortran/cpp.h
> > > @@ -46,6 +46,7 @@ void gfc_cpp_post_options (bool);
> > >   bool gfc_cpp_preprocess (const char *source_file);
> > >
> > >   void gfc_cpp_done (void);
> > > +void gfc_cpp_free_cpp_dirs (void);
> > >
> > >   void gfc_cpp_add_include_path (char *path, bool user_supplied);
> > >   void gfc_cpp_add_include_path_after (char *path, bool user_supplied);
> > > diff --git a/gcc/fortran/f95-lang.c b/gcc/fortran/f95-lang.c
> > > index 58dcaf01d75..ec4c2cf01d9 100644
> > > --- a/gcc/fortran/f95-lang.c
> > > +++ b/gcc/fortran/f95-lang.c
> > > @@ -275,7 +275,7 @@ gfc_finish (void)
> > > gfc_cpp_done ();
> > > gfc_done_1 ();
> > > gfc_release_include_path ();
> > > -  return;
> > 
> > namely here?
> >   
> > > +  gfc_cpp_free_cpp_dirs ();
> > >   }
> > 
> > Why not call free_cpp_dirs () here directly, omit all unnecessary
> > stuff, and maybe only add a brief comment here?  
> 
> cpp.c includes incpath.h, f95-lang.c does not and should not.
> So the cleanest thing is to keep the cpp handling in cpp.[ch] and have
> the language frontend call into it's cpp bits.
> 
> It would be rather rogue to
> extern void free_cpp_dirs (void);
> in f95-lang.c and directly call it in gfc_finish, i'd say?
> 
> thanks,

Re: [PATCH 3/3] Fortran: Fix mpz and mpfr memory leaks

[Bernhard Reutner-Fischer via Gcc-patches]

On 3 April 2023 21:50:49 CEST, Harald Anlauf  wrote:
>Hi Bernhard,
>
>there is neither context nor a related PR with a testcase showing
>that this patch fixes issues seen there.

Yes, i forgot to mention the PR:

PR fortran/68800

I did not construct individual test cases but it should be obvious that we 
should not leak these.

>
>On 4/2/23 17:05, Bernhard Reutner-Fischer via Gcc-patches wrote:
>> From: Bernhard Reutner-Fischer 
>> 
>> Cc: fort...@gcc.gnu.org
>> 
>> gcc/fortran/ChangeLog:
>> 
>>  * array.cc (gfc_ref_dimen_size): Free mpz memory before ICEing.
>>  * expr.cc (find_array_section): Fix mpz memory leak.
>>  * simplify.cc (gfc_simplify_reshape): Fix mpz memory leaks in
>>  error paths.
>>  (gfc_simplify_set_exponent): Fix mpfr memory leak.
>> ---
>>   gcc/fortran/array.cc| 3 +++
>>   gcc/fortran/expr.cc | 8 
>>   gcc/fortran/simplify.cc | 7 ++-
>>   3 files changed, 13 insertions(+), 5 deletions(-)
>> 
>> diff --git a/gcc/fortran/array.cc b/gcc/fortran/array.cc
>> index be5eb8b6a0f..8b1e816a859 100644
>> --- a/gcc/fortran/array.cc
>> +++ b/gcc/fortran/array.cc
>> @@ -2541,6 +2541,9 @@ gfc_ref_dimen_size (gfc_array_ref *ar, int dimen, 
>> mpz_t *result, mpz_t *end)
>> return t;
>> 
>>   default:
>> +  mpz_clear (lower);
>> +  mpz_clear (stride);
>> +  mpz_clear (upper);
>> gfc_internal_error ("gfc_ref_dimen_size(): Bad dimen_type");
>>   }
>
>What is the point of clearing variables before issuing a gfc_internal_error?

To make it obvious that we are aware that we allocated these.

thanks,
>
>> diff --git a/gcc/fortran/expr.cc b/gcc/fortran/expr.cc
>> index 7fb33f81788..b4736804eda 100644
>> --- a/gcc/fortran/expr.cc
>> +++ b/gcc/fortran/expr.cc
>> @@ -1539,6 +1539,7 @@ find_array_section (gfc_expr *expr, gfc_ref *ref)
>> mpz_init_set_ui (delta_mpz, one);
>> mpz_init_set_ui (nelts, one);
>> mpz_init (tmp_mpz);
>> +  mpz_init (ptr);
>> 
>> /* Do the initialization now, so that we can cleanup without
>>keeping track of where we were.  */
>> @@ -1682,7 +1683,6 @@ find_array_section (gfc_expr *expr, gfc_ref *ref)
>> mpz_mul (delta_mpz, delta_mpz, tmp_mpz);
>>   }
>> 
>> -  mpz_init (ptr);
>> cons = gfc_constructor_first (base);
>> 
>> /* Now clock through the array reference, calculating the index in
>> @@ -1735,7 +1735,8 @@ find_array_section (gfc_expr *expr, gfc_ref *ref)
>>   "at %L requires an increase of the allowed %d "
>>   "upper limit.  See %<-fmax-array-constructor%> "
>>   "option", &expr->where, flag_max_array_constructor);
>> -  return false;
>> +  t = false;
>> +  goto cleanup;
>>  }
>> 
>> cons = gfc_constructor_lookup (base, limit);
>> @@ -1750,8 +1751,6 @@ find_array_section (gfc_expr *expr, gfc_ref *ref)
>> gfc_copy_expr (cons->expr), NULL);
>>   }
>> 
>> -  mpz_clear (ptr);
>> -
>>   cleanup:
>> 
>> mpz_clear (delta_mpz);
>> @@ -1765,6 +1764,7 @@ cleanup:
>> mpz_clear (ctr[d]);
>> mpz_clear (stride[d]);
>>   }
>> +  mpz_clear (ptr);
>> gfc_constructor_free (base);
>> return t;
>>   }
>> diff --git a/gcc/fortran/simplify.cc b/gcc/fortran/simplify.cc
>> index ecf0e3558df..d1f06335e79 100644
>> --- a/gcc/fortran/simplify.cc
>> +++ b/gcc/fortran/simplify.cc
>> @@ -6866,6 +6866,7 @@ gfc_simplify_reshape (gfc_expr *source, gfc_expr 
>> *shape_exp,
>>gfc_error ("The SHAPE array for the RESHAPE intrinsic at %L has a "
>>   "negative value %d for dimension %d",
>>   &shape_exp->where, shape[rank], rank+1);
>> +  mpz_clear (index);
>>return &gfc_bad_expr;
>>  }
>> 
>> @@ -6889,6 +6890,7 @@ gfc_simplify_reshape (gfc_expr *source, gfc_expr 
>> *shape_exp,
>>  {
>>gfc_error ("Shapes of ORDER at %L and SHAPE at %L are different",
>>   &order_exp->where, &shape_exp->where);
>> +  mpz_clear (index);
>>return &gfc_bad_expr;
>>  }
>> 
>> @@ -6902,6 +6904,7 @@ gfc_simplify_reshape (gfc_expr *source, gfc_expr 
>> *shape_exp,
>>  {
>>gfc_error ("Sizes of ORDER at %L and SH

Re: [RFC PATCH] range-op-float: Fix up op1_op2_relation of comparisons

[Bernhard Reutner-Fischer via Gcc-patches]

On 12 April 2023 16:21:24 CEST, Jakub Jelinek via Gcc-patches 
 wrote:

>--- gcc/range-op-float.cc.jj   2023-04-12 12:17:44.784962757 +0200
>+++ gcc/range-op-float.cc  2023-04-12 16:07:54.948759355 +0200
>@@ -835,10 +835,17 @@ public:
>   bool fold_range (irange &r, tree type,
>  const frange &op1, const frange &op2,
>  relation_trio = TRIO_VARYING) const final override;
>-  relation_kind op1_op2_relation (const irange &lhs, const frange &,
>-const frange &) const final override
>+  relation_kind op1_op2_relation (const irange &lhs, const frange &op1,
>+const frange &op2) const final override
>   {
>-return lt_op1_op2_relation (lhs);
>+relation_kind ret = lt_op1_op2_relation (lhs);
>+if (ret == VREL_GE
>+  && (op1.known_isnan ()
>+  || op1.maybe_isnan ()
>+  || op2.known_isnan ()
>+  || op2.maybe_isnan ()))
>+  ret = VREL_VARYING; // Inverse of VREL_LT is VREL_UNGE with NAN ops.
>+return ret;
>   }
>   bool op1_range (frange &r, tree type,
> const irange &lhs, const frange &op2,
>@@ -952,9 +959,17 @@ public:
>   bool fold_range (irange &r, tree type,
>  const frange &op1, const frange &op2,
>  relation_trio rel = TRIO_VARYING) const final override;
>-  relation_kind op1_op2_relation (const irange &lhs, const frange &,
>-const frange &) const final override
>+  relation_kind op1_op2_relation (const irange &lhs, const frange &op1,
>+const frange &op2) const final override
>   {
>+relation_kind ret = le_op1_op2_relation (lhs);
>+if (ret == VREL_GT
>+  && (op1.known_isnan ()
>+  || op1.maybe_isnan ()
>+  || op2.known_isnan ()
>+  || op2.maybe_isnan ()))
>+  ret = VREL_VARYING; // Inverse of VREL_LE is VREL_UNGT with NAN ops.
>+return ret;
> return le_op1_op2_relation (lhs);

I think you forgot to delete the above return.

thanks,

Re: [PATCH v2] ree: Improve ree pass for rs6000 target.

[Bernhard Reutner-Fischer via Gcc-patches]

On 6 April 2023 12:49:53 CEST, Ajit Agarwal via Gcc-patches 
 wrote:
>Hello All:
>
>Eliminate unnecessary redundant extension within basic and across basic blocks.

To borrow HP's "arm-chair development mode", unfortunately most of the comments 
i had for the previous version apply to this version too, fwiw.

PS: Pretty please avoid moving functions around, if possible, as it spoils the 
history needlessly, IMHO.

thanks, and looking forward to a stage 1 patch.