Re: [PATCH 01/12] [contrib] validate_failures.py: Avoid testsuite aliasing

[rep . dot . nop]

On 27 September 2023 16:47:27 CEST, Maxim Kuvyrkov  
wrote:
>Hi Bernhard,
>
>Thanks, I meant to fix this, but forgot.

np.

>The underlying problem here is that we want to detect which sub-testsuites had 
>failures.  Current regex doesn't match go's case because there is no "..." at 
>the end: "Running foo" vs "Running foo ..." .
>
>My preferred way of fixing this is to make go's testsuite print out "..." .  
>We have a similar patch for glibc [1].
>
>[1] https://sourceware.org/pipermail/libc-alpha/2023-June/148702.html

Which asks:
---8<---
>> WDYT?
> 
> I looked at the gcc-testresults mailing list, and there appear no
> === … failures === lines at all?  What was the motivation for adding it
> in the first place?

The only motivation is that it looks like a nice header for the following 
FAILs.  What's your preference for the line -- drop it entirely or print out:

=== glibc failures ===
no unexpected failures

?
---8<---

I'd drop the above entirely if there are no failures, it's pretty superfluous, 
isn't it.

And concerning gotools and the missing trailing ".exp ...", I guess it's fine 
to add that to streamline the gotools output to all the other existing sum 
output.

TIA,


>
>--
>Maxim Kuvyrkov
>https://www.linaro.org
>
>> On Sep 26, 2023, at 19:46, Bernhard Reutner-Fischer  
>> wrote:
>> 
>> Hi Maxim!
>> 
>> On Mon, 5 Jun 2023 18:06:25 +0400
>> Maxim Kuvyrkov via Gcc-patches  wrote:
>> 
 On Jun 3, 2023, at 19:17, Jeff Law  wrote:
 
 On 6/2/23 09:20, Maxim Kuvyrkov via Gcc-patches wrote:  
> This patch adds tracking of current testsuite "tool" and "exp"
> to the processing of .sum files.  This avoids aliasing between
> tests from different testsuites with same name+description.
> E.g., this is necessary for testsuite/c-c++-common, which is ran
> for both gcc and g++ "tools".
> This patch changes manifest format from ...
> 
> FAIL: gcc_test
> FAIL: g++_test
> 
> ... to ...
> 
> === gcc tests ===
> Running gcc/foo.exp ...
> FAIL: gcc_test
> === gcc Summary ==
> === g++ tests ===
> Running g++/bar.exp ...
> FAIL: g++_test
> === g++ Summary ==
> .
> The new format uses same formatting as DejaGnu's .sum files
> to specify which "tool" and "exp" the test belongs to.  
 I think the series is fine.  You're not likely to hear from Diego or Doug 
 I suspect, I don't think either are involved in GNU stuff anymore.
 
>>> 
>>> Thanks, Jeff.  I'll wait for a couple of days and will merge if there are 
>>> no new comments.
>> 
>> Maxim, may i ask you to have a look at the following problem, please?
>> 
>> ISTM that your exp code does not work as expected for go, maybe you
>> forgot to test the changes with go enabled?
>> 
>> Ever since your changes in summer i see the following:
>> 
>> gcc-14.mine$ 
>> /scratch/src/gcc-14.mine/contrib/testsuite-management/validate_failures.py 
>> --clean_build ../gcc-14.orig/
>> Getting actual results from build directory .
>> ./gcc/testsuite/go/go.sum
>> ./gcc/testsuite/gcc/gcc.sum
>> ./gcc/testsuite/objc/objc.sum
>> ./gcc/testsuite/jit/jit.sum
>> ./gcc/testsuite/gdc/gdc.sum
>> ./gcc/testsuite/gnat/gnat.sum
>> ./gcc/testsuite/ada/acats/acats.sum
>> ./gcc/testsuite/g++/g++.sum
>> ./gcc/testsuite/obj-c++/obj-c++.sum
>> ./gcc/testsuite/rust/rust.sum
>> ./gcc/testsuite/gfortran/gfortran.sum
>> ./x86_64-pc-linux-gnu/libgomp/testsuite/libgomp.sum
>> ./x86_64-pc-linux-gnu/libphobos/testsuite/libphobos.sum
>> ./x86_64-pc-linux-gnu/libstdc++-v3/testsuite/libstdc++.sum
>> ./x86_64-pc-linux-gnu/libffi/testsuite/libffi.sum
>> ./x86_64-pc-linux-gnu/libitm/testsuite/libitm.sum
>> ./x86_64-pc-linux-gnu/libgo/libgo.sum
>> ./x86_64-pc-linux-gnu/libatomic/testsuite/libatomic.sum
>> ./gotools/gotools.sum
>> .sum file seems to be broken: tool="gotools", exp="None", 
>> summary_line="FAIL: TestScript"
>> Traceback (most recent call last):
>>  File 
>> "/scratch/src/gcc-14.mine/contrib/testsuite-management/validate_failures.py",
>>  line 732, in 
>>retval = Main(sys.argv)
>>  File 
>> "/scratch/src/gcc-14.mine/contrib/testsuite-management/validate_failures.py",
>>  line 721, in Main
>>retval = CompareBuilds()
>>  File 
>> "/scratch/src/gcc-14.mine/contrib/testsuite-management/validate_failures.py",
>>  line 622, in CompareBuilds
>>actual = GetResults(sum_files)
>>  File 
>> "/scratch/src/gcc-14.mine/contrib/testsuite-management/validate_failures.py",
>>  line 466, in GetResults
>>build_results.update(ParseSummary(sum_fname))
>>  File 
>> "/scratch/src/gcc-14.mine/contrib/testsuite-management/validate_failures.py",
>>  line 405, in ParseSummary
>>result = result_set.MakeTestResult(line, ordinal)
>>  File 
>> "/scratch/src/gcc-14.mine/contrib/testsuite-management/validate_failures.py",
>>  line 239, in MakeTestResult
>>return TestResult(summary_line, ordinal,
>>  File 
>> "/scratch/src/gcc-14.mine/contrib/testsuite-management/validate_failures.py",
>>  line 151, in

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

[rep . dot . nop]

On 12 October 2023 14:35:36 CEST, Ajit Agarwal  wrote:
>This patch improves code sinking pass to sink statements before call to reduce
>register pressure.
>Review comments are incorporated.

Typo: "block block"

And spurious whitespace changes.

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

[rep . dot . nop]

On 21 October 2023 01:56:16 CEST, Vineet Gupta  wrote:
>On 10/19/23 23:50, Ajit Agarwal wrote:
>> Hello All:
>> 
>> This version 9 of the patch uses abi interfaces to remove zero and sign 
>> extension elimination.
>> Bootstrapped and regtested on powerpc-linux-gnu.
>> 
>> In this version (version 9) of the patch following review comments are 
>> incorporated.
>> 
>> a) Removal of hard code zero_extend and sign_extend  in abi interfaces.
>> b) Source and destination with different registers are considered.
>> c) Further enhancements.
>> d) Added sign extension elimination using abi interfaces.
>
>As has been trend in the past, I don't think all the review comments have been 
>addressed.

And apart from that, may I ask if this is just me, or does anybody else think 
that it might be worthwhile to actually read a patch before (re-)posting?

Seeing e.g. the proposed abi_extension_candidate_p as written in a first POC 
would deserve some manual CSE, if nothing more then for clarity and conciseness?

Just curious from a meta perspective..

And:

>> ree: Improve ree pass for rs6000 target using defined abi interfaces

mentioning powerpc like this, and then changing generic code could be 
interpreted as misleading, IMHO.

>> 
>> For rs6000 target we see redundant zero and sign extension and done
>> to improve ree pass to eliminate such redundant zero and sign extension
>> using defined ABI interfaces.

Mentioning powerpc in the body as one of the affected target(s) is of course 
fine.


>>   +/* Return TRUE if target mode is equal to source mode of zero_extend
>> +   or sign_extend otherwise false.  */

, false otherwise.

But I'm not a native speaker 


>> +/* Return TRUE if the candidate insn is zero extend and regno is
>> +   a return registers.  */
>> +
>> +static bool
>> +abi_extension_candidate_return_reg_p (/*rtx_insn *insn, */int regno)

Leftover debug comment.

>> +{
>> +  if (targetm.calls.function_value_regno_p (regno))
>> +return true;
>> +
>> +  return false;
>> +}
>> +

As said, I don't see why the below was not cleaned up before the V1 submission.
Iff it breaks when manually CSEing, I'm curious why?

>> +/* Return TRUE if reg source operand of zero_extend is argument registers
>> +   and not return registers and source and destination operand are same
>> +   and mode of source and destination operand are not same.  */
>> +
>> +static bool
>> +abi_extension_candidate_p (rtx_insn *insn)
>> +{
>> +  rtx set = single_set (insn);
>> +  machine_mode dst_mode = GET_MODE (SET_DEST (set));
>> +  rtx orig_src = XEXP (SET_SRC (set), 0);
>> +
>> +  if (!FUNCTION_ARG_REGNO_P (REGNO (orig_src))
>> +  || abi_extension_candidate_return_reg_p (/*insn,*/ REGNO (orig_src)))

On top, debug leftover.

>> +return false;
>> +
>> +  /* Mode of destination and source should be different.  */
>> +  if (dst_mode == GET_MODE (orig_src))
>> +return false;
>> +
>> +  machine_mode mode = GET_MODE (XEXP (SET_SRC (set), 0));
>> +  bool promote_p = abi_target_promote_function_mode (mode);
>> +
>> +  /* REGNO of source and destination should be same if not
>> +  promoted.  */
>> +  if (!promote_p && REGNO (SET_DEST (set)) != REGNO (orig_src))
>> +return false;
>> +
>> +  return true;
>> +}
>> +

As said, please also rephrase the above (and everything else if it obviously 
looks akin the above).

The rest, mentioned below,  should largely be covered by following the coding 
convention.

>> +/* Return TRUE if the candidate insn is zero extend and regno is
>> +   an argument registers.  */

Singular register.

>> +
>> +static bool
>> +abi_extension_candidate_argno_p (/*rtx_code code, */int regno)

Debug leftover.
I would probably have inlined this function manually, with a respective comment.
Did not look how often it is used, admittedly.

>> +{
>> +  if (FUNCTION_ARG_REGNO_P (regno))
>> +return true;
>> +
>> +  return false;
>> +}
[]
>> +
>>   /* This function goes through all reaching defs of the source

s/This function goes/Go/

>>  of the candidate for elimination (CAND) and tries to combine

(of, ?didn't look) candidate CAND for eliminating

>>  the extension with the definition instruction.  The changes

defining

Pre-existing, I know.
But you could fix those in a preparatory patch while you touch surrounding code.
This is not a requirement, of course, just good habit, IMHO.

>> @@ -770,6 +889,11 @@ combine_reaching_defs (ext_cand *cand, const_rtx 
>> set_pat, ext_state *state)
>>   state->defs_list.truncate (0);
>> state->copies_list.truncate (0);
>> +  rtx orig_src = XEXP (SET_SRC (cand->expr),0);
>> +
>> +  if (abi_extension_candidate_p (cand->insn)
>> +  && (!get_defs (cand->insn, orig_src, NULL)))

Excess braces.
Hopefully check_gnu_style would have complained.

>> +return abi_handle_regs (cand->insn);
>>   outcome = make_defs_and_copies_lists (cand->insn, set_pat, state);
>>   @@ -1036,6 +1160,15 @@ combine_reaching_defs (ext_cand *cand, const_rtx 
>> set_pat, ext_stat

Re: [PATCH] gcc.c-torture/execute/builtins/fputs.c: Define _GNU_SOURCE

[rep . dot . nop]

On 22 October 2023 21:45:12 CEST, Jeff Law  wrote:
>
>
>On 10/22/23 10:09, Andrew Pinski wrote:
>> On Sun, Oct 22, 2023 at 12:47 AM Florian Weimer  wrote:
>>> 
>>> Current glibc headers only declare fputs_unlocked for _GNU_SOURCE.
>>> Defining the macro avoids an implicit function declaration.
>> 
>> This does not help targets that don't use glibc though.
>> Note for builtins testsuite there is a lib-fputs.c file which will
>> define a fputs_unlock which is how it will link even if the libc does
>> not define a fputs_unlock.
>But isn't fputs_unlocked glibc specific to begin with?  ie, the test really 
>doesn't make sense AFAICT on non-glibc targets.

I think uClibc had it too, at least at one point in the past.

Re: [PATCH] Fortran/OpenMP: event handle in task detach cannot be a coarray [PR104131]

[rep . dot . nop]

On 24 October 2023 21:25:01 CEST, Harald Anlauf  wrote:
>Dear all,
>
>the attached simple patch adds a forgotten check that an event handle
>cannot be a coarray.  This case appears to have been overlooked in the
>original fix for this PR.
>
>I intend to commit as obvious within 24h unless there are comments.

diff --git a/gcc/fortran/openmp.cc b/gcc/fortran/openmp.cc
index 1cc65d7fa49..08081dacde4 100644
--- a/gcc/fortran/openmp.cc
+++ b/gcc/fortran/openmp.cc
@@ -8967,6 +8967,9 @@ resolve_omp_clauses (gfc_code *code, gfc_omp_clauses 
*omp_clauses,
   else if (omp_clauses->detach->symtree->n.sym->attr.dimension > 0)
gfc_error ("The event handle at %L must not be an array element",
   &omp_clauses->detach->where);
+  else if (omp_clauses->detach->symtree->n.sym->attr.codimension)
+   gfc_error ("The event handle at %L must not be a coarray",

ISTM that we usually do not mention "element" when talking about undue 
(co)array access.

Maybe we want to streamline this specific error message?

LGTM otherwise.
Thanks for your dedication!


+  &omp_clauses->detach->where);
   else if (omp_clauses->detach->symtree->n.sym->ts.type == BT_DERIVED
   || omp_clauses->detach->symtree->n.sym->ts.type == BT_CLASS)
gfc_error ("The event handle at %L must not be part of "

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

[rep . dot . nop]

On 24 October 2023 09:36:22 CEST, Ajit Agarwal  wrote:
>Hello Bernhard:
>
>On 23/10/23 7:40 pm, Bernhard Reutner-Fischer wrote:
>> On Mon, 23 Oct 2023 12:16:18 +0530
>> Ajit Agarwal  wrote:
>> 
>>> Hello All:
>>>
>>> Addressed below review comments in the version 11 of the patch.
>>> Please review and please let me know if its ok for trunk.
>> 
>> s/satisified/satisfied/
>> 
>
>I will fix this.

thanks!

>
 As said, I don't see why the below was not cleaned up before the V1 
 submission.
 Iff it breaks when manually CSEing, I'm curious why?
>> 
>> The function below looks identical in v12 of the patch.
>> Why didn't you use common subexpressions?
>> ba
>
>Using CSE here breaks aarch64 regressions hence I have reverted it back 
>not to use CSE,

Just for my own education, can you please paste your patch perusing common 
subexpressions and an assembly diff of the failing versus working aarch64 
testcase, along how you configured that failing (cross-?)compiler and the 
command-line of a typical testcase that broke when manually CSEing the function 
below?

I might have not completely understood the subtile intricacies of RTL 
re-entrancy, it seems?

thanks

   
>> +/* Return TRUE if reg source operand of zero_extend is argument 
>> registers
>> +   and not return registers and source and destination operand are same
>> +   and mode of source and destination operand are not same.  */
>> +
>> +static bool
>> +abi_extension_candidate_p (rtx_insn *insn)
>> +{
>> +  rtx set = single_set (insn);
>> +  machine_mode dst_mode = GET_MODE (SET_DEST (set));
>> +  rtx orig_src = XEXP (SET_SRC (set), 0);
>> +
>> +  if (!FUNCTION_ARG_REGNO_P (REGNO (orig_src))
>> +  || abi_extension_candidate_return_reg_p (/*insn,*/ REGNO 
>> (orig_src)))  
>> +return false;
>> +
>> +  /* Mode of destination and source should be different.  */
>> +  if (dst_mode == GET_MODE (orig_src))
>> +return false;
>> +
>> +  machine_mode mode = GET_MODE (XEXP (SET_SRC (set), 0));
>> +  bool promote_p = abi_target_promote_function_mode (mode);
>> +
>> +  /* REGNO of source and destination should be same if not
>> +  promoted.  */
>> +  if (!promote_p && REGNO (SET_DEST (set)) != REGNO (orig_src))
>> +return false;
>> +
>> +  return true;
>> +}
>> +  
>> 
>> 

 As said, please also rephrase the above (and everything else if it 
 obviously looks akin the above).
>> 
>> thanks

Re: [PATCH] Format gotools.sum closer to what DejaGnu does

[rep . dot . nop]

Hi Maxim!

Many thanks for the patch! Quick question below..

On 2 November 2023 13:48:55 CET, Maxim Kuvyrkov  
wrote:
>... to restore compatability with validate_failures.py .
>The testsuite script validate_failures.py expects
>"Running  ..." to extract  values,
>and gotools.sum provided "Running ".
>
>Note that libgo.sum, which also uses Makefile logic to generate
>DejaGnu-like output, already has "..." suffix.
>
>gotools/ChangeLog:
>
>   * Makefile.am: Update "Running  ..." output
>   * Makefile.in: Regenerate.
>---
> gotools/Makefile.am | 4 ++--
> gotools/Makefile.in | 5 +++--
> 2 files changed, 5 insertions(+), 4 deletions(-)
>
>diff --git a/gotools/Makefile.am b/gotools/Makefile.am
>index 7b5302990f8..d2376b9c25b 100644
>--- a/gotools/Makefile.am
>+++ b/gotools/Makefile.am
>@@ -332,8 +332,8 @@ check: check-head check-go-tool check-runtime 
>check-cgo-test check-carchive-test
>   @cp gotools.sum gotools.log
>   @for file in cmd_go-testlog runtime-testlog cgo-testlog 
> carchive-testlog cmd_vet-testlog embed-testlog; do \
> testname=`echo $${file} | sed -e 's/-testlog//' -e 's|_|/|'`; \
>-echo "Running $${testname}" >> gotools.sum; \
>-echo "Running $${testname}" >> gotools.log; \
>+echo "Running $${testname} ..." >> gotools.sum; \
>+echo "Running $${testname} ..." >> gotools.log; \
> sed -e 's/^--- \(.*\) ([^)]*)$$/\1/' < $${file} >> gotools.log; \
> grep '^--- ' $${file} | sed -e 's/^--- \(.*\) ([^)]*)$$/\1/' -e 
> 's/SKIP/UNTESTED/' | sort -k 2 >> gotools.sum; \
>   done
>diff --git a/gotools/Makefile.in b/gotools/Makefile.in
>index 2783b91ef4b..9cc238e748d 100644
>--- a/gotools/Makefile.in
>+++ b/gotools/Makefile.in
>@@ -317,6 +317,7 @@ pdfdir = @pdfdir@
> prefix = @prefix@
> program_transform_name = @program_transform_name@
> psdir = @psdir@
>+runstatedir = @runstatedir@

Are you sure you used the correct version of automake?

thanks

> sbindir = @sbindir@
> sharedstatedir = @sharedstatedir@
> srcdir = @srcdir@
>@@ -1003,8 +1004,8 @@ mostlyclean-local:
> @NATIVE_TRUE@ @cp gotools.sum gotools.log
> @NATIVE_TRUE@ @for file in cmd_go-testlog runtime-testlog cgo-testlog 
> carchive-testlog cmd_vet-testlog embed-testlog; do \
> @NATIVE_TRUE@   testname=`echo $${file} | sed -e 's/-testlog//' -e 's|_|/|'`; 
> \
>-@NATIVE_TRUE@   echo "Running $${testname}" >> gotools.sum; \
>-@NATIVE_TRUE@   echo "Running $${testname}" >> gotools.log; \
>+@NATIVE_TRUE@   echo "Running $${testname} ..." >> gotools.sum; \
>+@NATIVE_TRUE@   echo "Running $${testname} ..." >> gotools.log; \
> @NATIVE_TRUE@   sed -e 's/^--- \(.*\) ([^)]*)$$/\1/' < $${file} >> 
> gotools.log; \
> @NATIVE_TRUE@   grep '^--- ' $${file} | sed -e 's/^--- \(.*\) ([^)]*)$$/\1/' 
> -e 's/SKIP/UNTESTED/' | sort -k 2 >> gotools.sum; \
> @NATIVE_TRUE@ done

Re: [PATCH] Format gotools.sum closer to what DejaGnu does

[rep . dot . nop]

On 2 November 2023 18:06:54 CET, Maxim Kuvyrkov  
wrote:
>> On Nov 2, 2023, at 21:02, rep.dot@gmail.com wrote:
>> 
>> Hi Maxim!
>> 
>> Many thanks for the patch! Quick question below..
>> 
>> On 2 November 2023 13:48:55 CET, Maxim Kuvyrkov  
>> wrote:
>>> ... to restore compatability with validate_failures.py .
>>> The testsuite script validate_failures.py expects
>>> "Running  ..." to extract  values,
>>> and gotools.sum provided "Running ".
>>> 
>>> Note that libgo.sum, which also uses Makefile logic to generate
>>> DejaGnu-like output, already has "..." suffix.
>>> 
>>> gotools/ChangeLog:
>>> 
>>> * Makefile.am: Update "Running  ..." output
>>> * Makefile.in: Regenerate.
>>> ---
>>> gotools/Makefile.am | 4 ++--
>>> gotools/Makefile.in | 5 +++--
>>> 2 files changed, 5 insertions(+), 4 deletions(-)
>>> 
>>> diff --git a/gotools/Makefile.am b/gotools/Makefile.am
>>> index 7b5302990f8..d2376b9c25b 100644
>>> --- a/gotools/Makefile.am
>>> +++ b/gotools/Makefile.am
>>> @@ -332,8 +332,8 @@ check: check-head check-go-tool check-runtime 
>>> check-cgo-test check-carchive-test
>>> @cp gotools.sum gotools.log
>>> @for file in cmd_go-testlog runtime-testlog cgo-testlog carchive-testlog 
>>> cmd_vet-testlog embed-testlog; do \
>>>   testname=`echo $${file} | sed -e 's/-testlog//' -e 's|_|/|'`; \
>>> -   echo "Running $${testname}" >> gotools.sum; \
>>> -   echo "Running $${testname}" >> gotools.log; \
>>> +   echo "Running $${testname} ..." >> gotools.sum; \
>>> +   echo "Running $${testname} ..." >> gotools.log; \
>>>   sed -e 's/^--- \(.*\) ([^)]*)$$/\1/' < $${file} >> gotools.log; \
>>>   grep '^--- ' $${file} | sed -e 's/^--- \(.*\) ([^)]*)$$/\1/' -e 
>>> 's/SKIP/UNTESTED/' | sort -k 2 >> gotools.sum; \
>>> done
>>> diff --git a/gotools/Makefile.in b/gotools/Makefile.in
>>> index 2783b91ef4b..9cc238e748d 100644
>>> --- a/gotools/Makefile.in
>>> +++ b/gotools/Makefile.in
>>> @@ -317,6 +317,7 @@ pdfdir = @pdfdir@
>>> prefix = @prefix@
>>> program_transform_name = @program_transform_name@
>>> psdir = @psdir@
>>> +runstatedir = @runstatedir@
>> 
>> Are you sure you used the correct version of automake?
>
>I used automake 1.15.1 (from Ubuntu 20.04 automake-1.15 package), and I 
>double-checked after getting the runstatedir update.

I think that runstatedir is a Debian (and derivatives) addition, would probably 
suffice to just drop that line manually..

The patch itself looks like it would be ok, probably even obvious, but I can 
not approve it.

I'm a bit surprised that you don't need to have "exp" != None for 
validate-failures to work after your exp addition, but I take it you checked 
that aspect :-)

thanks, again!

>
>I would appreciate someone checking on their side to make sure I don't have 
>something weird going on in my setup.
>
>--
>Maxim Kuvyrkov
>https://www.linaro.org
>

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

[rep . dot . nop]

On 2 November 2023 11:25:47 CET, Jonathan Wakely  wrote:
>On Thu, 2 Nov 2023 at 10:23, Andreas Schwab wrote:
>>
>> On Nov 02 2023, Jonathan Wakely wrote:
>>
>> > Git tags are cheap, but I can imagine a concern about hundreds of new
>> > tags "littering" the output of 'git tag -l'. I don't _think_ you can
>> > put tags under an alternative ref that isn't fetched by default (as we
>> > do with refs/users and refs/vendor). I think tags have to go under
>> > refs/tags. But grep -v could be used to filter out snapshot tags
>> > easily.
>>
>> There is no inherent limitation on publishing tags outside of refs/tags,
>> to make them invisible by git tag.  There are already existing examples
>> of tags residing under various refs/users and refs/vendors namespaces.
>
>
>Ah, good to know, thanks.
>
>So then there's no reason that snapshots would have to clutter up the
>list of default tags for anybody who isn't interested in them.
>

Thanks Andreas. Exactly. So, just to emphasise the obvious:
Let's please use refs/snapshot ?

Re: [PATCH v2] Format gotools.sum closer to what DejaGnu does

[rep . dot . nop]

On 3 November 2023 14:38:06 CET, Jeff Law  wrote:
>
>
>On 11/3/23 06:54, Maxim Kuvyrkov wrote:

>> gotools/ChangeLog:
>> 
>>  * Makefile.am: Update "Running  ..." output
>>  * Makefile.in: Regenerate.
>OK.  Thanks for running down the differences in the autogenerated  bits.

Many thanks for reinstating regression checking even for go stuff and hence the 
extra 2 or 3 kilometres to fix unnoticed regressions imposed by different 
dejagnu check files/suites and the wart of using pristine autotools 
infrastructure.

cheers

Re: [patch, libgfortran] Part 2: PR105456 Child I/O does not propage iostat

[rep . dot . nop]

On 5 March 2024 04:15:12 CET, Jerry D  wrote:

>
>Attached is the revised patch using the already available string_len_trim 
>function.
>
>This hunk is only executed if a user has not passed an iostat or iomsg 
>variable in the parent I/O statement and an error is triggered which 
>terminates execution of the program. In this case, the iomsg string is 
>provided in the usual error message in a "processor defined" way.
>
>(F2023):
>
>12.6.4.8.3 Executing defined input/output data transfers
>---
>11 If the iostat argument of the defined input/output procedure has a nonzero 
>value when that procedure returns, and the processor therefore terminates 
>execution of the program as described in 12.11, the processor shall make the 
>value of the iomsg argument available in a processor-dependent manner.
>---
>
>OK for trunk?

LGTM.
thanks!

Re: [PATCH v2 08/13] aarch64: Add Cygwin and MinGW environments for AArch64

[rep . dot . nop]

On 19 March 2024 14:40:57 CET, Christophe Lyon  
wrote:
>On Mon, 18 Mar 2024 at 22:35, Evgeny Karpov  
>wrote:
>>
>> Monday, March 18, 2024 2:27 PM
>> Christophe Lyon wrote:
>>
>> > > +/* Disable SEH and declare the required SEH-related macros that are
>> > > +still needed for compilation.  */ #undef TARGET_SEH #define
>> > > +TARGET_SEH 0
>> > > +
>> > > +#define SSE_REGNO_P(N) 0
>> > > +#define GENERAL_REGNO_P(N) 0
>> > I think you forgot to add a comment to explain the above two lines.
>> > (it was requested during v1 review)
>> >
>> > Thanks,
>> >
>> > Christophe
>>
>> Hi Christophe,
>>
>> Thank you for the review!
>> The comment regarding SEH and SEH-related macros has been added two lines 
>> above.
>> It may not be obvious, but these macros are needed to emit SEH data in 
>> mingw/winnt.cc.
>> This group is separated by an empty line; however, it still relates to 
>> SEH-related macros.
>>
>Thanks for the clarification, I thought that comment only applied to
>the two lines about TARGET_SEH.

So, for avoidance of doubt, please drop the vertical space before SSE_REGNO_P 
to be gentle to the casual/inattentive reader?

Or add /* SEH-related */ after the vertical space, to make it clear?

thanks

Re: [PATCH] cpp: new built-in __EXP_COUNTER__

[rep . dot . nop]

On 19 March 2024 18:27:13 CET, Kaz Kylheku  wrote:
>On 2024-03-18 00:30, Jonathan Wakely wrote:
>> I don't have an opinion on the implementation, or the proposal itself,
>> except that the implementation seems susprisingly simple, which is
>> nice.
>
>Hi Jonathan,
>
>Here is an updated patch.
>
>It rebased cleanly over more than newer 16000 commits, suggesting
>that the area in the cpp code is "still waters", which is good.
>
>I made the documentation change not to recommend using #if, but
>#ifdef.
>
>I got rid of the ChangeLog changes, and also tried to pay more
>attention to the log message format, where the ChangeLog pieces
>are specified.
>
>In the first test case, I had to adjust the expected warning text
>for two lines.
>

Please forgive the bike shedding, but __EXP_COUNTER__ would lead me into 
thinking about exponents or thereabouts.
__MACRO_EXPANSION_COUNTER__ is more what your patch is about, IMHO? Maybe you 
could come up with a more descriptive name, please?

And, while I can see what could possibly be done with that, I'm not really 
convinced that it would be a wise idea to (unilaterally) support that idea. 
Don't you think that this would encourage producing more spaghetti code?

Just curious about real world motivating examples I guess.
cheers

Re: [PATCH v3] tree-profile: Disable indirect call profiling for IFUNC resolvers

[rep . dot . nop]

On 3 April 2024 15:49:13 CEST, "H.J. Lu"  wrote:


>> OK witht that change.
>> Honza
>
>I am checking in this patch with the updated comments:
>
>  /* Disable indirect call profiling for an IFUNC resolver and its
> callees since it requires TLS which hasn't been set up yet when
> the dynamic linker is resolving IFUNC symbols.  See
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114115
>   */
>
>Thanks.
>

+  /* Skip if it has been visited.  */
+  unsigned int uid = e->caller->get_uid ();
+  if (bitmap_bit_p (ifunc_ref_map, uid))
+   continue;
+  bitmap_set_bit (ifunc_ref_map, uid);

I think you could have written this as
if (!bitmap_set_bit (ifunc_ref_map, uid))
  continue;

FWIW. thanks

Re: [PATCH v3] tree-profile: Disable indirect call profiling for IFUNC resolvers

[rep . dot . nop]

On 5 April 2024 03:03:05 CEST, "H.J. Lu"  wrote:
>On Thu, Apr 4, 2024 at 5:34 PM  wrote:
>>
>> On 3 April 2024 15:49:13 CEST, "H.J. Lu"  wrote:
>

>> +  /* Skip if it has been visited.  */
>> +  unsigned int uid = e->caller->get_uid ();
>> +  if (bitmap_bit_p (ifunc_ref_map, uid))
>> +   continue;
>> +  bitmap_set_bit (ifunc_ref_map, uid);
>>
>> I think you could have written this as
>> if (!bitmap_set_bit (ifunc_ref_map, uid))
>>   continue;
>>
>
>Feel free to submit a patch.

OK, could be that  
https://inbox.sourceware.org/gcc-patches/20211101220212.3d308d1f@nbbrfq/
was not applied yet, the bitmap_clear_bit is the same.
I'll try to remember these for next stage 1.

cheers

Re: [PATCH] gcc/configure: Re-introduce INSTALL_INFO

[rep . dot . nop]

On 1 February 2024 18:15:34 CET, Christophe Lyon  
wrote:
>BUILD_INFO is currently a byproduct of checking makeinfo
>presence/version.  INSTALL_INFO used to be defined similarly, but was
>removed in 2000 (!) by commit 17db658241d18cf6db59d31bc2d6eac96e9257df
>(svn r38141).
>
>In order to save build time, our CI overrides BUILD_INFO="", which
>works when invoking 'make all' but not for 'make install' in case some
>info files need an update.

Instead of resurrecting INSTALL_INFO maybe you could something along the lines 
of

https://gcc.gnu.org/bugzilla/attachment.cgi?id=15038&action=edit

not sure which approach would be considered cleaner..

HTH

Re: [PATCH] x86-64: Find a scratch register for large model profiling

[rep . dot . nop]

On 2 February 2024 00:02:54 CET, "H.J. Lu"  wrote:
>On Thu, Feb 1, 2024 at 10:32 AM Jakub Jelinek  wrote:
>>
>> On Thu, Feb 01, 2024 at 10:15:30AM -0800, H.J. Lu wrote:
>> > --- a/gcc/config/i386/i386.cc
>> > +++ b/gcc/config/i386/i386.cc
>> > @@ -22749,6 +22749,31 @@ current_fentry_section (const char **name)
>> >return true;
>> >  }
>> >
>> > +/* Return an unused caller-saved register at entry for profile.  */
>> > +
>> > +static int
>> > +x86_64_select_profile_regnum (bool r11_ok ATTRIBUTE_UNUSED)
>> > +{
>> > +  int i;
>>
>> Why not just return R10_REG here if flag_entry != 0 (i.e. keep existing
>> behavior unless emitting profiler after prologue)?
>
>Fixed in v2.

Nit: r10_ok is now superfluous, but lets wait for Jakub.
thanks

Re: [PATCH] gcc/configure: Re-introduce INSTALL_INFO

[rep . dot . nop]

On 5 February 2024 12:30:23 CET, Christophe Lyon  
wrote:
>On Fri, 2 Feb 2024 at 11:40, Christophe Lyon  
>wrote:
>>
>> On Fri, 2 Feb 2024 at 11:10,  wrote:
>> >
>> > On 1 February 2024 18:15:34 CET, Christophe Lyon 
>> >  wrote:
>> > >BUILD_INFO is currently a byproduct of checking makeinfo
>> > >presence/version.  INSTALL_INFO used to be defined similarly, but was
>> > >removed in 2000 (!) by commit 17db658241d18cf6db59d31bc2d6eac96e9257df
>> > >(svn r38141).
>> > >
>> > >In order to save build time, our CI overrides BUILD_INFO="", which
>> > >works when invoking 'make all' but not for 'make install' in case some
>> > >info files need an update.
>> >
>> > Instead of resurrecting INSTALL_INFO maybe you could something along the 
>> > lines of
>> >
>> > https://gcc.gnu.org/bugzilla/attachment.cgi?id=15038&action=edit
>>
>> Ha indeed something along these lines would work too.
>> Thanks for the archaeology :-)
>>
>> >
>> > not sure which approach would be considered cleaner..
>> Not sure either.
>>
>> What do maintainers prefer?
>>
>
>Actually that leads to a small patch:
>https://gcc.gnu.org/pipermail/gcc-patches/2024-February/644957.html

Thats even better.
thanks

>
>> >
>> > HTH

Re: [PATCH 5/5] bpf: renamed coreout.* files to btfext-out.*.

[rep . dot . nop]

On 21 February 2024 18:16:30 CET, David Faust  wrote:
>
>
>On 2/20/24 02:24, Cupertino Miranda wrote:
>> gcc/ChangeLog:
>>  * config.gcc (target_gtfiles): changed coreout to btfext-out.
>>  (extra_objs): changed coreout to btfext-out.
>
>I think these entries should start with a capital letter ("Changed...").

Present tense in ChangeLog,  as mandated by the standard: /ed/s/ed//g

thanks,

>
>>  * config/bpf/coreout.cc: Renamed to btfext-out.cc
>>  * config/bpf/btfext-out.cc: Added
>>  * config/bpf/coreout.h: Renamed to btfext-out.h
>>  * config/bpf/btfext-out.h: Added
>>  * config/bpf/core-builtins.cc: Changed include
>>  * config/bpf/core-builtins.h: Changed include
>>  * config/bpf/t-bpf: Renamed file.
>

Re: [PATCH v1 05/13] Reuse MinGW from i386 for AArch64

[rep . dot . nop]

On 21 February 2024 19:34:43 CET, Evgeny Karpov  
wrote:
>

Please use git send-email. Your mail ends up as empty as here, otherwise.

The ChangeLog has to be expressed in present tense, as mandated by the 
standard; s/Moved/Move/g etc.

In any sane world ( and in gcc ) to fold, respectively a folder, is something 
else compared to a directory ( which you probably mean when moving a file from 
one directory to another directory as you seem to do ).

Most of the free world has left COFF behind since several decades, so I won't 
comment on that. YMMV.

HTH

Re: [PATCH, v2] Fix fortran/PR114024

[rep . dot . nop]

On 23 February 2024 22:15:17 CET, Harald Anlauf  wrote:
>Hi Steve, all,
>
>here's an updated patch with an enhanced testcase that also
>checks MOLD= besides SOURCE=.
>
>Regtested on x86_64-pc-linux-gnu.  Is it OK for mainline?

LGTM
cheers

>
>Cheers,
>Harald

Re: [commit] MIPS: Add ATTRIBUTE_UNUSED to mips_start_function_definition

[rep . dot . nop]

On 11 January 2024 10:59:21 CET, YunQiang Su  wrote:
>Fix build warning:
>  mips.cc: warning: unused parameter 'decl'.
>
>gcc
>   * config/mips/mips.cc (mips_start_function_definition):
>   Add ATTRIBUTE_UNUSED.
>---
> gcc/config/mips/mips.cc | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
>diff --git a/gcc/config/mips/mips.cc b/gcc/config/mips/mips.cc
>index 60b336e43d0..e752019b5e2 100644
>--- a/gcc/config/mips/mips.cc
>+++ b/gcc/config/mips/mips.cc
>@@ -7330,7 +7330,8 @@ mips_start_unique_function (const char *name)
>function contains MIPS16 code.  */
> 
> static void
>-mips_start_function_definition (const char *name, bool mips16_p, tree decl)
>+mips_start_function_definition (const char *name, bool mips16_p,
>+  tree decl ATTRIBUTE_UNUSED)

Nowadays in C++ you can just remove the identifier name:

+mips_start_function_definition (const char *name, bool mips16_p, tree)

> {
>   if (mips16_p)
> fprintf (asm_out_file, "\t.set\tmips16\n");

Re: [PATCH] RISC-V: Tweak the wording for the sorry message

[rep . dot . nop]

On 19 January 2024 03:41:57 CET, Kito Cheng  wrote:
>Thanks, pushed to trunk :)

Thanks, but don't you have to update the tests too, at least
gcc/testsuite/gcc.target/riscv/rvv/base/big_endian-2.c ?

thanks

>
>On Fri, Jan 19, 2024 at 10:36 AM juzhe.zh...@rivai.ai
> wrote:
>>
>> OK
>>
>> 
>> juzhe.zh...@rivai.ai
>>
>>
>> From: Kito Cheng
>> Date: 2024-01-19 10:34
>> To: rep.dot.nop; jeffreyalaw; rdapp.gcc; juzhe.zhong; gcc-patches
>> CC: Kito Cheng
>> Subject: [PATCH] RISC-V: Tweak the wording for the sorry message
>> Use "does not" rather than "cannot", because it's implementation issue.
>>
>> gcc/ChangeLog:
>>
>> * config/riscv/riscv.cc (riscv_override_options_internal): Tweak
>> sorry message.
>> ---
>> gcc/config/riscv/riscv.cc | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
>> index f1d5129397f..dd6e68a08c2 100644
>> --- a/gcc/config/riscv/riscv.cc
>> +++ b/gcc/config/riscv/riscv.cc
>> @@ -8798,13 +8798,13 @@ riscv_override_options_internal (struct gcc_options 
>> *opts)
>>   We can only allow TARGET_MIN_VLEN * 8 (LMUL) < 65535.  */
>>if (TARGET_MIN_VLEN_OPTS (opts) > 4096)
>> -sorry ("Current RISC-V GCC cannot support VLEN greater than 4096bit for 
>> "
>> +sorry ("Current RISC-V GCC does not support VLEN greater than 4096bit 
>> for "
>>"'V' Extension");
>>/* FIXME: We don't support RVV in big-endian for now, we may enable RVV 
>> with
>>   big-endian after finishing full coverage testing.  */
>>if (TARGET_VECTOR && TARGET_BIG_ENDIAN)
>> -sorry ("Current RISC-V GCC cannot support RVV in big-endian mode");
>> +sorry ("Current RISC-V GCC does not support RVV in big-endian mode");
>>/* Convert -march to a chunks count.  */
>>riscv_vector_chunks = riscv_convert_vector_bits (opts);
>> --
>> 2.34.1
>>
>>

Re: [committed] Fix comment typos

[rep . dot . nop]

Hi

Just another commentary typo..

On 17 January 2024 11:23:01 CET, Jakub Jelinek  wrote:

>--- gcc/gengtype.cc.jj 2024-01-03 11:51:23.314845233 +0100
>+++ gcc/gengtype.cc2024-01-16 18:56:57.383009291 +0100
>@@ -4718,8 +4718,8 @@ write_roots (pair_p variables, bool emit
> }
> 
> /* Prints not-as-ugly version of a typename of T to OF.  Trades the uniquness
>-   guaranteee for somewhat increased readability.  If name conflicts do 
>happen,

s/uniquness/uniqueness/g

thanks

Re: Fix merging of value predictors

[rep . dot . nop]

On 17 January 2024 14:20:49 CET, Jan Hubicka  wrote:

>--- a/gcc/predict.def
>+++ b/gcc/predict.def
>@@ -94,6 +94,16 @@ DEF_PREDICTOR (PRED_LOOP_ITERATIONS_GUESSED, "guessed loop 
>iterations",
> DEF_PREDICTOR (PRED_LOOP_ITERATIONS_MAX, "guessed loop iterations",
>  PROB_UNINITIALIZED, PRED_FLAG_FIRST_MATCH)
> 
>+/* Prediction which is an outcome of combining multiple value predictions.  */
>+DEF_PREDICTOR (PRED_COMBINED_VALUE_PREDICTIONS,
>+ "combined value predictions", PROB_UNINITIALIZED, 0)
>+
>+/* Prediction which is an outcome of combining multiple value predictions
>+   on PHI statement (this is less accurate since we do not know reverse
>+   edge probabilities at that time).  */
>+DEF_PREDICTOR (PRED_COMBINED_VALUE_PREDICTIONS_PHI,
>+ "combined value predictions", PROB_UNINITIALIZED, 0)
>+

Do you want to add "phi" somewhere to the latter (to distinguish them in the 
dumps)?

thanks

Re: [PATCH] openmp: Change to using a hashtab to lookup offload target addresses for indirect function calls

[rep . dot . nop]

On 22 January 2024 21:33:17 CET, Kwok Cheung Yeung  
wrote:
>Hi
>
>There was a bug in the declare-target-indirect-2.c libgomp testcase (testing 
>indirect calls in offloaded target regions, spread over multiple 
>teams/threads) that due to an errant fallthrough in a switch statement 
>resulted in only one indirect function ever getting called:
>
>switch (i % 3)
>  {
>case 0: fn_ptr[i] = &foo;  // Missing break
>case 1: fn_ptr[i] = &bar;  // Missing break
>case 2: fn_ptr[i] = &baz;
>  }
>
>However, when the missing break statements are added, the testcase fails with 
>an invalid memory access. Upon investigation, this is due to the use of a 
>splay-tree as the lookup structure for indirect addresses, as the splay-tree 
>moves frequently accessed elements closer to the root node and so needs 
>locking when used from multiple threads. However, this would end up partially 
>serialising all the threads and kill performance. I have switched the lookup 
>structure from a splay tree to a hashtab instead to avoid locking during 
>lookup.
>
>I have also tidied up the initialisation of the lookup table by calling it 
>only from the first thread of the first team, instead of redundantly calling 
>it from every thread and only having the first one reached do the 
>initialisation. This removes the need for locking during initialisation.
>
>Tested with offloading to NVPTX and GCN with a x86_64 host. Okay for master?


Can you please akso update the comments to talk about hashtab instead of splay?

TIA

>
>Thanks
>
>Kwok

Re: [PATCH v2 5/5] Add documentation for musttail attribute

[rep . dot . nop]

On 24 January 2024 20:30:45 CET, Andi Kleen  wrote:
>---
> gcc/doc/extend.texi | 16 
> 1 file changed, 16 insertions(+)
>
>diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
>index 0bc586d120e7..c68d32bed8de 100644
>--- a/gcc/doc/extend.texi
>+++ b/gcc/doc/extend.texi
>@@ -9867,6 +9867,22 @@ foo (int x, int y)
> @code{y} is not actually incremented and the compiler can but does not
> have to optimize it to just @code{return 42 + 42;}.
> 
>+@cindex @code{musttail} statement attribute
>+@item musttail
>+
>+The @code{gnu::musttail} or @code{clang::hottail} attribute

AFAICS this patchset does not handle hottail ?

thanks,

>+can be applied to a return statement that returns the value
>+of a call to indicate that the call must be a tail call
>+that does not allocate extra stack space.
>+
>+@smallexample
>+[[gnu::musttail]] return foo();
>+@end smallexample
>+
>+If the compiler cannot generate a tail call it will generate
>+an error. Tail calls generally require enabling optimization.
>+On some targets they may not be supported.
>+
> @end table
> 
> @node Attribute Syntax

Re: [PATCH] Fortran: use name of array component in runtime error message [PR30802]

[rep . dot . nop]

On 28 January 2024 22:43:37 CET, Steve Kargl 
 wrote:
>On Sun, Jan 28, 2024 at 08:56:24PM +0100, Harald Anlauf wrote:
>> 
>> Am 28.01.24 um 12:39 schrieb Mikael Morin:
>> > Le 24/01/2024 à 22:39, Harald Anlauf a écrit :
>> > > Dear all,
>> > > 
>> > > this patch is actually only a followup fix to generate the proper name
>> > > of an array reference in derived-type components for the runtime error
>> > > message generated for the bounds-checking code.  Without the proper
>> > > part ref, not only a user may get confused: I was, too...
>> > > 
>> > > The testcase is compile-only, as it is only important to check the
>> > > strings used in the error messages.
>> > > 
>> > > Regtested on x86_64-pc-linux-gnu.  OK for mainline?
>> > > 
>> > 
>> > the change proper looks good, and is an improvement.  But I'm a little
>> > concerned by the production of references like in the test x1%vv%z which
>> > could be confusing and is strictly speaking invalid fortran (multiple
>> > non-scalar components).  Did you consider generating x1%vv(?,?)%zz or
>> > x1%vv(...)%z or similar?
>> 
>> yes, that seems very reasonable, given that this is what NAG does.
>> 
>> We also have spurious %_data in some error messages that I'll try
>> to get rid off.
>> 
>
>I haven't looked at the patch, but sometimes (if not always) things
>like _data are marked with attr.artificial.  You might see if this
>will help with suppressing spurious messages.
>

Reminds me of
https://inbox.sourceware.org/fortran/2024231748.376086cd@nbbrfq/

Maybe thats missing, i did not apply that yet, did i?

HTH

Re: [PATCH] Fortran: Mark internal symbols as artificial [PR88009,PR68800]

[rep . dot . nop]

On 29 January 2024 22:06:04 CET, Harald Anlauf  wrote:
>Am 29.01.24 um 21:45 schrieb Bernhard Reutner-Fischer:
>> On Wed, 17 Nov 2021 21:32:14 +0100
>> Harald Anlauf  wrote:
>> 
>>> Do you have testcases/reproducers demonstrating that the patch actually
>>> fixes the issues you're describing?
>> 
>> I believe that marking artificial symbols as such is obvious and i did
>> use the existing tests to verify that the changes do not regress but
>> behave as intended. I did check that the memory leak in
>> gfc_find_derived_vtab is fixed with the patch.
>> 
>> Ok for stage 1 if the rebased regression test passes?
>> 
>> thanks
>> 
>>> 
>>> Am 17.11.21 um 09:12 schrieb Bernhard Reutner-Fischer via Gcc-patches:
 On Tue, 16 Nov 2021 21:46:32 +0100
 Harald Anlauf via Fortran  wrote:
 
> Hi Bernhard,
> 
> I'm trying to understand your patch.  What does it really try to solve?
 
 Compiler generated symbols should be marked artificial.
 The fix for PR88009 ( f8add009ce300f24b75e9c2e2cc5dd944a020c28 ,
 r9-5194 ) added artificial just to the _final component and left out all 
 the rest.
 Note that the majority of compiler generated symbols in class.c
 already had artificial set properly.
 The proposed patch amends the other generated symbols to be marked
 artificial, too.
 
 The other parts fix memory leaks.
 
> 
> PR88009 is closed and seems to have nothing to do with this.
 
 Well it marked only _final as artificial and forgot to adjust the
 others as well.
 We can remove the reference to PR88009 if you prefer?
 
 thanks!
> 
> Harald
> 
> Am 14.11.21 um 23:17 schrieb Bernhard Reutner-Fischer via Fortran:
>> Hi!
>> 
>> Amend fix for PR88009 to mark all these class components as artificial.
>> 
>> gcc/fortran/ChangeLog:
>> 
>>* class.c (gfc_build_class_symbol, 
>> generate_finalization_wrapper,
>>(gfc_find_derived_vtab, find_intrinsic_vtab): Use stringpool 
>> for
>>names. Mark internal symbols as artificial.
>>* decl.c (gfc_match_decl_type_spec, gfc_match_end): Fix
>>indentation.
>>(gfc_match_derived_decl): Fix indentation. Check extension 
>> level
>>before incrementing refs counter.
>>* parse.c (parse_derived): Fix style.
>>* resolve.c (resolve_global_procedure): Likewise.
>>* symbol.c (gfc_check_conflict): Do not ignore artificial 
>> symbols.
>>(gfc_add_flavor): Reorder condition, cheapest first.
>>(gfc_new_symbol, gfc_get_sym_tree,
>>generate_isocbinding_symbol): Fix style.
>>* trans-expr.c (gfc_trans_subcomponent_assign): Remove
>>restriction on !artificial.
>>* match.c (gfc_match_equivalence): Special-case CLASS_DATA for
>>warnings.
>> 
>> ---
>> gfc_match_equivalence(), too, should not bail-out early on the first
>> error but should diagnose all errors. I.e. not goto cleanup but set
>> err=true and continue in order to diagnose all constraints of a
>> statement. Maybe Sandra or somebody else will eventually find time to
>> tweak that.
>> 
>> I think it also plugs a very minor leak of name in gfc_find_derived_vtab
>> so i also tagged it [PR68800]. At least that was the initial
>> motiviation to look at that spot.
>> We were doing
>> -  name = xasprintf ("__vtab_%s", tname);
>> ...
>>  gfc_set_sym_referenced (vtab);
>> - name = xasprintf ("__vtype_%s", tname);
>> 
>> Bootstrapped and regtested without regressions on x86_64-unknown-linux.
>> Ok for trunk?
>> 
> 
> 
 
 
>>> 
>> 
>> 
>
>Can you please post the patch here so that we can review it?
>

I'm very sorry that I missed to provide an explicit reference, the initial 
patch was submitted here:
https://inbox.sourceware.org/fortran/2024231748.376086cd@nbbrfq/
But I will follow-up with a rebased, tested patch ASAP during a weekend or 
vacation. 

But just to give context, that's what I was talking about..
thanks

PS: IMHO a strcmp(..,"_data") is inappropriate for you should check 
attr.artificial and the path exploder to give hints should ideally deal with 
this transparently -- IMHO

Re: [RFA] New pass for sign/zero extension elimination

[rep . dot . nop]

On 22 November 2023 23:23:41 CET, Jeff Law  wrote:
>
>
>On 11/20/23 11:56, Dimitar Dimitrov wrote:
>> On Sun, Nov 19, 2023 at 05:47:56PM -0700, Jeff Law wrote:
>> ...

>>> +  enum rtx_code xcode = GET_CODE (x);
>>> +  if (xcode == SET)
>>> +   {
>>> + const_rtx dst = SET_DEST (x);
>>> + rtx src = SET_SRC (x);
>>> + const_rtx y;
>>> + unsigned HOST_WIDE_INT bit = 0;
>>> +
>>> + /* The code of the RHS of a SET.  */
>>> + enum rtx_code code = GET_CODE (src);
>>> +
>>> + /* ?!? How much of this should mirror SET handling, potentially
>>> +being shared?   */
>>> + if (SUBREG_BYTE (dst).is_constant () && SUBREG_P (dst))
>> 
>> Shouldn't SUBREG_P be checked first like:
>>if (SUBREG_P (dst) && SUBREG_BYTE (dst).is_constant ())
>Yes, absolutely. It'll be fixed in the next update.
>
>This also highlighted that I never added pru-elf to the configurations in my 
>tester.  I remember thinking that it needed to be added, but obviously that 
>mental TODO got lost.  I've just fixed that.


And please drop the superfluous enum from rtx_code while at it?

TIA

Re: hurd: Add multilib paths for gnu-x86_64

[rep . dot . nop]

On 27 November 2023 15:48:33 CET, Thomas Schwinge  
wrote:
>Hi!
>
>On 2023-10-28T21:19:59+0200, Samuel Thibault  wrote:
>> We need the multilib paths in gcc to find e.g. glibc crt files on
>> Debian.
>
>ACK.
>
>> This is essentially based on t-linux64 version.
>
>Yes, but isn't the overall setup diverged from GNU/Linux?
>
>Currently, x86_64 GNU/Hurd first gets 'i386/t-linux64', whose definitons
>are only later:
>
>> --- a/gcc/config.gcc
>> +++ b/gcc/config.gcc
>> @@ -5828,6 +5828,9 @@ case ${target} in
>>   visium-*-*)
>>   target_cpu_default2="TARGET_CPU_$with_cpu"
>>   ;;
>> + x86_64-*-gnu*)
>> + tmake_file="$tmake_file i386/t-gnu64"
>> + ;;
>>  esac
>
>... then here (effectively) overwritten by 'i386/t-gnu64'.  Instead, I
>suppose, we should handle 'i386/t-linux64' and 'i386/t-gnu64' alike, and
>resolve relevant configuration differences.
>
>As fas a I can tell, 'i386/t-linux64' is also used for multilib-enabled
>('test x$enable_targets = xall') x86 GNU/Linux, and that's not
>(correspondingly) done for x86 GNU/Hurd?
>
>However, such things can certainly be resolved incrementally, later on.
>I understand that your change does work for you as-is, so I've now pushed
>that to master branch in commit 5707e9db9c398d311defc80c5b7822c9a07ead60
>"hurd: Add multilib paths for gnu-x86_64", see attached.

+# To support i386, x86-64 and x32 libraries, the directory structrue

I guess one could legitimately understand this as a "structure setting 
standards " ;) but let's spell it structure nevertheless?

cheers

Re: [Patch] Fortran: invoke.texi - link to OpenCoarrays.org + mention libcaf_single

[rep . dot . nop]

On 20 May 2024 02:31:27 CEST, Sandra Loosemore  wrote:
>On 5/19/24 02:01, Tobias Burnus wrote:
>> I noticed that gfortran's coarray support did not link to the 
>> http://www.opencoarrays.org/ >
>> [snip]
>> 
>> diff --git a/gcc/fortran/invoke.texi b/gcc/fortran/invoke.texi
>> index 40e8e4a7cdd..78a2910b8d8 100644
>> --- a/gcc/fortran/invoke.texi
>> +++ b/gcc/fortran/invoke.texi
>> @@ -1753,7 +1753,10 @@ Single-image mode, i.e. @code{num_images()} is always 
>> one.
>>   @item @samp{lib}
>>  Library-based coarray parallelization; a suitable GNU Fortran coarray
>> -library needs to be linked.
>> +library needs to be linked such as @url{http://opencoarrays.org}.
>
>This would read better as
>
>library such as @url{http://opencoarrays.org} needs to be linked.

Maybe use https?

thanks

>
>> +Alternatively, GCC's @code{libcaf_single} library can be linked,
>> +albeit it only supports a single image.
>> +
>>  @end table
>
>OK with that tweak.
>
>-Sandra
>
>
>

Re: [PATCH] fortran: Support optional dummy as BACK argument of MINLOC/MAXLOC.

[rep . dot . nop]

On 22 July 2024 20:53:18 CEST, Mikael Morin  wrote:
>From: Mikael Morin 
>
>Hello,
>
>this fixes a null pointer dereference with absent optional dummy passed
>as BACK argument of MINLOC/MAXLOC.
>
>Tested for regression on x86_64-linux.
>OK for master?
>
>-- >8 --
>
>Protect the evaluation of BACK with a check that the reference is non-null
>in case the expression is an optional dummy, in the inline code generated
>for MINLOC and MAXLOC.
>
>This change contains a revert of the non-testsuite part of commit
>r15-1994-ga55d24b3cf7f4d07492bb8e6fcee557175b47ea3, which factored the
>evaluation of BACK out of the loop using the scalarizer.  It was a bad idea,
>because delegating the argument evaluation to the scalarizer makes it
>cumbersome to add a null pointer check next to the evaluation.
>
>Instead, evaluate BACK at the beginning, before scalarization, add a check
>that the argument is present if necessary, and evaluate the resulting
>expression to a variable, before using the variable in the inline code.
>
>gcc/fortran/ChangeLog:
>
>   * trans-intrinsic.cc (maybe_absent_optional_variable): New function.
>   (gfc_conv_intrinsic_minmaxloc): Remove BACK from scalarization and
>   evaluate it before.  Add a check that BACK is not null if the
>   expression is an optional dummy.  Save the resulting expression to a
>   variable.  Use the variable in the generated inline code.
>
>gcc/testsuite/ChangeLog:
>
>   * gfortran.dg/maxloc_6.f90: New test.
>   * gfortran.dg/minloc_7.f90: New test.
>---
> gcc/fortran/trans-intrinsic.cc |  81 +-
> gcc/testsuite/gfortran.dg/maxloc_6.f90 | 366 +
> gcc/testsuite/gfortran.dg/minloc_7.f90 | 366 +
> 3 files changed, 799 insertions(+), 14 deletions(-)
> create mode 100644 gcc/testsuite/gfortran.dg/maxloc_6.f90
> create mode 100644 gcc/testsuite/gfortran.dg/minloc_7.f90
>
>diff --git a/gcc/fortran/trans-intrinsic.cc b/gcc/fortran/trans-intrinsic.cc
>index 180d0d7a88c..9f3c3ce47bc 100644
>--- a/gcc/fortran/trans-intrinsic.cc
>+++ b/gcc/fortran/trans-intrinsic.cc
>@@ -5209,6 +5209,50 @@ gfc_conv_intrinsic_dot_product (gfc_se * se, gfc_expr * 
>expr)
> }
> 
> 
>+/* Tells whether the expression E is a reference to an optional variable whose
>+   presence is not known at compile time.  Those are variable references 
>without
>+   subreference; if there is a subreference, we can assume the variable is
>+   present.  We have to special case full arrays, which we represent with a 
>fake
>+   "full" reference, and class descriptors for which a reference to data is 
>not
>+   really a subreference.  */
>+
>+bool
>+maybe_absent_optional_variable (gfc_expr *e)
>+{
>+  if (!(e && e->expr_type == EXPR_VARIABLE))
>+return false;

[]

>+}
>+
>+
> /* Remove unneeded kind= argument from actual argument list when the
>result conversion is dealt with in a different place.  */
> 
>@@ -5321,11 +5365,11 @@ gfc_conv_intrinsic_minmaxloc (gfc_se * se, gfc_expr * 
>expr, enum tree_code op)
>   tree nonempty;
>   tree lab1, lab2;
>   tree b_if, b_else;
>+  tree back;
>   gfc_loopinfo loop;
>   gfc_actual_arglist *actual;
>   gfc_ss *arrayss;
>   gfc_ss *maskss;
>-  gfc_ss *backss;
>   gfc_se arrayse;
>   gfc_se maskse;
>   gfc_expr *arrayexpr;
>@@ -5391,10 +5435,27 @@ gfc_conv_intrinsic_minmaxloc (gfc_se * se, gfc_expr * 
>expr, enum tree_code op)
> && maskexpr->symtree->n.sym->attr.dummy
> && maskexpr->symtree->n.sym->attr.optional;
>   backexpr = actual->next->next->expr;
>-  if (backexpr)
>-backss = gfc_get_scalar_ss (gfc_ss_terminator, backexpr);
>+
>+  gfc_init_se (&backse, NULL);
>+  if (backexpr == nullptr)
>+back = logical_false_node;
>+  else if (maybe_absent_optional_variable (backexpr))
>+{

/* if this fires, some wonky race is going on.. */

>+  gcc_assert (backexpr->expr_type == EXPR_VARIABLE);

drop it, downgrade to checking, or is it worth?

>+
>+  gfc_conv_expr (&backse, backexpr);
>+  tree present = gfc_conv_expr_present (backexpr->symtree->n.sym, false);
>+  back = fold_build2_loc (input_location, TRUTH_ANDIF_EXPR,
>+logical_type_node, present, backse.expr);
>+}
>   else
>-backss = nullptr;
>+{
>+  gfc_conv_expr (&backse, backexpr);
>+  back = backse.expr;
>+}
>+  gfc_add_block_to_block (&se->pre, &backse.pre);
>+  back = gfc_evaluate_now_loc (input_location, back, &se->pre);
>+  gfc_add_block_to_block (&se->pre, &backse.post);
> 
>   nonempty = NULL

[]
thanks

Re: [PATCH] fortran: Support optional dummy as BACK argument of MINLOC/MAXLOC.

[rep . dot . nop]

On 27 July 2024 21:11:19 CEST, Mikael Morin  wrote:
>Le 27/07/2024 à 19:23, rep.dot@gmail.com a écrit :
>> On 22 July 2024 20:53:18 CEST, Mikael Morin  wrote:
>>> From: Mikael Morin 
>>> 
>>> Hello,
>>> 
>>> this fixes a null pointer dereference with absent optional dummy passed
>>> as BACK argument of MINLOC/MAXLOC.
>>> 
>>> Tested for regression on x86_64-linux.
>>> OK for master?
>>> 
>>> -- >8 --
>>> 
>>> Protect the evaluation of BACK with a check that the reference is non-null
>>> in case the expression is an optional dummy, in the inline code generated
>>> for MINLOC and MAXLOC.
>>> 
>>> This change contains a revert of the non-testsuite part of commit
>>> r15-1994-ga55d24b3cf7f4d07492bb8e6fcee557175b47ea3, which factored the
>>> evaluation of BACK out of the loop using the scalarizer.  It was a bad idea,
>>> because delegating the argument evaluation to the scalarizer makes it
>>> cumbersome to add a null pointer check next to the evaluation.
>>> 
>>> Instead, evaluate BACK at the beginning, before scalarization, add a check
>>> that the argument is present if necessary, and evaluate the resulting
>>> expression to a variable, before using the variable in the inline code.
>>> 
>>> gcc/fortran/ChangeLog:
>>> 
>>> * trans-intrinsic.cc (maybe_absent_optional_variable): New function.
>>> (gfc_conv_intrinsic_minmaxloc): Remove BACK from scalarization and
>>> evaluate it before.  Add a check that BACK is not null if the
>>> expression is an optional dummy.  Save the resulting expression to a
>>> variable.  Use the variable in the generated inline code.
>>> 
>>> gcc/testsuite/ChangeLog:
>>> 
>>> * gfortran.dg/maxloc_6.f90: New test.
>>> * gfortran.dg/minloc_7.f90: New test.
>>> ---
>>> gcc/fortran/trans-intrinsic.cc |  81 +-
>>> gcc/testsuite/gfortran.dg/maxloc_6.f90 | 366 +
>>> gcc/testsuite/gfortran.dg/minloc_7.f90 | 366 +
>>> 3 files changed, 799 insertions(+), 14 deletions(-)
>>> create mode 100644 gcc/testsuite/gfortran.dg/maxloc_6.f90
>>> create mode 100644 gcc/testsuite/gfortran.dg/minloc_7.f90
>>> 
>>> diff --git a/gcc/fortran/trans-intrinsic.cc b/gcc/fortran/trans-intrinsic.cc
>>> index 180d0d7a88c..9f3c3ce47bc 100644
>>> --- a/gcc/fortran/trans-intrinsic.cc
>>> +++ b/gcc/fortran/trans-intrinsic.cc
>>> @@ -5209,6 +5209,50 @@ gfc_conv_intrinsic_dot_product (gfc_se * se, 
>>> gfc_expr * expr)
>>> }
>>> 
>>> 
>>> +/* Tells whether the expression E is a reference to an optional variable 
>>> whose
>>> +   presence is not known at compile time.  Those are variable references 
>>> without
>>> +   subreference; if there is a subreference, we can assume the variable is
>>> +   present.  We have to special case full arrays, which we represent with 
>>> a fake
>>> +   "full" reference, and class descriptors for which a reference to data 
>>> is not
>>> +   really a subreference.  */
>>> +
>>> +bool
>>> +maybe_absent_optional_variable (gfc_expr *e)
>>> +{
>>> +  if (!(e && e->expr_type == EXPR_VARIABLE))
>>> +return false;
>> 
>> []
>> 
>>> +}
>>> +
>>> +
>>> /* Remove unneeded kind= argument from actual argument list when the
>>> result conversion is dealt with in a different place.  */
>>> 
>>> @@ -5321,11 +5365,11 @@ gfc_conv_intrinsic_minmaxloc (gfc_se * se, gfc_expr 
>>> * expr, enum tree_code op)
>>>tree nonempty;
>>>tree lab1, lab2;
>>>tree b_if, b_else;
>>> +  tree back;
>>>gfc_loopinfo loop;
>>>gfc_actual_arglist *actual;
>>>gfc_ss *arrayss;
>>>gfc_ss *maskss;
>>> -  gfc_ss *backss;
>>>gfc_se arrayse;
>>>gfc_se maskse;
>>>gfc_expr *arrayexpr;
>>> @@ -5391,10 +5435,27 @@ gfc_conv_intrinsic_minmaxloc (gfc_se * se, gfc_expr 
>>> * expr, enum tree_code op)
>>>  && maskexpr->symtree->n.sym->attr.dummy
>>>  && maskexpr->symtree->n.sym->attr.optional;
>>>backexpr = actual->next->next->expr;
>>> -  if (backexpr)
>>> -backss = gfc_get_scalar_ss (gfc_ss_terminator, backexpr);
>>> +
>>> +  gfc_init_se (&backse, NULL);
>>> +  if (backexpr == nullptr)
>>> +back = logical_false_node;
>>> +  else if (maybe_absent_optional_variable (backexpr))
>>> +{
>> 
>> /* if this fires, some wonky race is going on.. */
>> 
>>> +  gcc_assert (backexpr->expr_type == EXPR_VARIABLE);
>> 
>> drop it, downgrade to checking, or is it worth?
>> 
>Whether it is worth it, I don't know; it's protecting the access to 
>backexpr->symtree a few lines down, idependently of the implementation of 
>maybe_absent_optional_variable.

nod

>
>I expect the compiler to optimize it away,

ye[l]*p 

> so I can surely make it a checking-only assert.

let's keep the assert unless someone opts for any downgrade within 72h ?

maybe add the comment if an assert persists, though. LGTM otherwise AFAIC

Re: [PATCH] Value range: Add range op for __builtin_isfinite

[rep . dot . nop]

On 12 April 2024 07:30:10 CEST, HAO CHEN GUI  wrote:


>
>
>patch.diff
>diff --git a/gcc/gimple-range-op.cc b/gcc/gimple-range-op.cc
>index 9de130b4022..99c511728d3 100644
>--- a/gcc/gimple-range-op.cc
>+++ b/gcc/gimple-range-op.cc
>@@ -1192,6 +1192,56 @@ public:
>   }
> } op_cfn_isinf;
>
>+//Implement range operator for CFN_BUILT_IN_ISFINITE
>+class cnf_isfinite : public range_operator
>+{


s/cnf/cfn/g
I guess.
thanks

Re: [Patch] Fortran: Dead-function removal in error.cc (shrinking by 40%)

[rep . dot . nop]

On 11 October 2024 16:42:44 CEST, David Malcolm  wrote:
>On Fri, 2024-10-11 at 15:34 +0100, Paul Richard Thomas wrote:
>> Hi Tobias,
>> 
>> Good catch! It looks 'obvious' to me too :-)


yeah, thanks for the explicit cleanup


>> > * I want to move support range-based locations, which is also a
>> > good
>> > opportunity to fix some misplaced '1' (e.g. which point at white
>> > space
>> > instead of the actual declaration or ...).


yep.
We have this nasty bug that locus and comments (maybe with or without 
continuation, don't remember the format or all) do havoc to locus (see BZ)
This is solved when we use locus that support ranges (and somebody going 
throught the FE) &


>> > 
>> > * David wants to improve json/sarif output, including stderr +
>> > sarif/json
>> > output at the same time, but that has issues with
>> > delayed/suppressed/buffered
>> > diagnostic in gfortran (because of the try & error parsing* in
>> > Fortran)
>> > → https://gcc.gnu.org/PR116613 for the former and
>> > https://gcc.gnu.org/105916
>> > for the buffering issue.
>
>Thanks for the patch; I was planning to take a look at the SARIF
>buffering issue later today/Monday from the gcc/diagnostic.cc/h side
>(perhaps introducing an idea of "pending diagnostics" there), so any
>simplifications on the fortran side are most welcome!  My knowledge of
>Fortran is almost zero, sorry.
>
>Dave
>

Re: [Patch] Fortran: Use OpenACC's acc_on_device builtin, fix OpenMP' __builtin_is_initial_device

[rep . dot . nop]

On 14 October 2024 10:23:56 CEST, Thomas Schwinge  
wrote:
>Hi Tobias!
>
>On 2024-10-13T10:21:01+0200, Tobias Burnus  wrote:
>> Now pushed as r15-4298-g3269a722b7a036.
>
>> Tobias Burnus wrote:
>>> Anyone feeling like reviewing this patch?
>
>Yes.  But please allow for more than 1 1/2 work days.

>>  * types.def (BT_BOOL): Fix type by using bool_type_node.

let's try this hunk - should work'ish

Re: [PATCH 2/4] RISC-V: Implement TARGET_SCHED_PRESSURE_PREFER_NARROW [PR/114729]

[rep . dot . nop]

On 20 October 2024 21:40:16 CEST, Vineet Gupta  wrote:

>diff --git a/gcc/testsuite/gcc.target/riscv/sched1-spills/spill1.cpp 
>b/gcc/testsuite/gcc.target/riscv/sched1-spills/spill1.cpp
>new file mode 100644
>index ..47a9d1a01ab4
>--- /dev/null
>+++ b/gcc/testsuite/gcc.target/riscv/sched1-spills/spill1.cpp
>

>+/* { dg-final { scan-assembler-times "%sfp" 0 } } */

scan-assembler-not, please

Re: [PATCH v2] gfortran testsuite: Remove unit-files in files having open-statements, PR116701

[rep . dot . nop]

On 25 September 2024 13:51:07 CEST, Andre Vehreschild  wrote:
>Hi Hans-Peter,
>
>preface: I am not a testsuite nor an m4 expert.
>
>So I may be wrong in arguing that your changes look reasonable. I like the
>"automatic" clean-up process very much. So by me, ok for mainline. But you may
>want to wait for one other ok from some one who has more experience in
>the gfortran testsuite (yes, still wondering who that might be :-( ).

LGTM
Ok for trunk.

thanks,

>
>Thanks for the patch,
>   Andre
>
>On Wed, 25 Sep 2024 04:06:14 +0200
>Hans-Peter Nilsson  wrote:
>
>> Changes since v1:
>> - Rename gfortran-dg-rmunits to fortran-delete-unit-files.
>> - Move it to lib/fortran-modules.exp.
>> - Tweak commit message accordingly and mention cause of placement of
>>   the proc.
>> - Tweak proc comment to mention why keeping removals unique despite
>>   comment.
>>
>> Here's a general approach to handle PR116701.  I considered
>> adding manual deletions as quoted below and mentioned in the
>> PR, but seeing the handling of "integer 8" in
>> fortran-torture-execute I decided to follow that example:
>> better scan the source for open-statements than relying on
>> manual annotations and people remembering to add them for
>> new test-cases.
>>
>> I hope the inclusion of gfortran-dg.exp in
>> fortran-torture.exp is not controversial, but there's no
>> fortran-specific testsuite file common to dg and
>> classic-torture and also this placement is still in the
>> "Utility routines" section of gfortran-dg.exp.  (BTW, the C
>> torture-tests changed to the dg framework some time ago - no
>> more .x-files there and dg-directives actually work - there
>> are some in gfortran.fortran-torture that are apparently
>> ignored!)
>>
>> There's one further cleanup possible, removing the manual
>> removal in open_errors_2.f90 (which should have used
>> "target", not "build")
>>
>> Works for cris-elf (no regressions).  Version v1 was also
>> similarly regtested on native x86_64-linux-gnu.  Manual
>> checks have verified the unit-removal.
>>
>> Ok to commit?
>>
>> -- >8 --
>> PR testsuite/116701 shows that left-behind files from
>> unnamed gfortran open statements (named unit.N, where N =
>> unit number) can interfere with the result of a subsequent
>> run.  While that's unlikely to happen for a "real" fortran
>> target or a test with a deleting close-statement, test-cases
>> should not rely on previous test-cases passing and not
>> execute along different execution paths depending on earlier
>> runs, even if the difference is benevolent.
>>
>> Most but not all fortran test-cases go through
>> gfortran-dg-runtest (gfortran.dg) or fortran-torture-execute
>> (gfortran.fortran-torture).  However, the exceptions, with
>> more complex framework and call-chains, either don't run or
>> don't have open-statements, so a more complex solution
>> doesn't seem worthwhile.  If test-cases with open-statements
>> are added later to those parts of the test-suite, calls to
>> fortran-delete-unit-files at the right spot may be added or
>> worst case, "manual" cleanup-calls added, like:
>> ! { dg-final { remote_file target delete "fort.10" } }
>> Put the new proc in fortran-modules.exp since that's where other
>> common fortran-testsuite dejagnu-library functions are located.
>>
>>  PR testsuite/116701
>>  * lib/fortran-modules.exp (fortran-delete-unit-files): New proc.
>>  * lib/gfortran-dg.exp (gfortran-dg-runtest): Call
>>  fortran-delete-unit-files after executing test.
>>  * lib/fortran-torture.exp (fortran-torture-execute): Ditto.
>> ---
>>  gcc/testsuite/lib/fortran-modules.exp | 21 +
>>  gcc/testsuite/lib/fortran-torture.exp |  2 ++
>>  gcc/testsuite/lib/gfortran-dg.exp |  1 +
>>  3 files changed, 24 insertions(+)
>>
>> diff --git a/gcc/testsuite/lib/fortran-modules.exp
>> b/gcc/testsuite/lib/fortran-modules.exp index 158b16bada91..a7196f13ed22
>> 100644 --- a/gcc/testsuite/lib/fortran-modules.exp
>> +++ b/gcc/testsuite/lib/fortran-modules.exp
>> @@ -172,3 +172,24 @@ proc igrep { args } {
>>  }
>>  return $grep_out
>>  }
>> +
>> +# If the code has any "open" statements for numbered units, make sure
>> +# no corresponding output file remains.  Redundant remove operations
>> +# are ok, but duplicate removals look sloppy, so track for uniqueness.
>> +proc fortran-delete-unit-files { src } {
>> +set openpat {open *\( *(?:unit *= *)?([0-9]+)}
>> +set openmatches [igrep $src $openpat]
>> +if {![string match "" $openmatches]} {
>> +# verbose -log "Found \"$openmatches\""
>> +set deleted_units {}
>> +foreach openmatch $openmatches {
>> +regexp -nocase -- "$openpat" $openmatch match unit
>> +if {[lsearch $deleted_units $unit] < 0} {
>> +set rmfile "fort.$unit"
>> +verbose -log "Deleting $rmfile"
>> +remote_file target delete "fort.$unit"
>> +lappend deleted_units $unit
>> +}
>> +}
>> +}
>> +}
>> diff --git a

Re: [PATCH] gfortran testsuite: Remove unit-files in files having open-statements, PR116701

[rep . dot . nop]

>Your interpretation of my typo is correct.  Along with Andre I like auto 
>cleanup. On new test cases we try to have them self delete whether they pass 
>or fail.
>

so why don't we issue the cleanup then, regardless?

>So your changes are ok with me.
>
>> No.
>> 
>>> 

Re: [PATCH] gfortran testsuite: Remove unit-files in files having open-statements, PR116701

[rep . dot . nop]

On 25 September 2024 22:08:15 CEST, rep.dot@gmail.com wrote:
>
>>Your interpretation of my typo is correct.  Along with Andre I like auto 
>>cleanup. On new test cases we try to have them self delete whether they pass 
>>or fail.
>>
>
>so why don't we issue the cleanup then, regardless?



>
>>So your changes are ok with me.

Re: [PATCH] arm: Force flag_pic for FDPIC

[rep . dot . nop]

On 27 September 2024 16:05:01 CEST, "Richard Earnshaw (lists)" 
 wrote:
>On 26/09/2024 19:21, Ramana Radhakrishnan wrote:
>> On Mon, Mar 4, 2024 at 1:43 PM Fangrui Song  wrote:
>>>
>>> From: Fangrui Song 
>>>
>>> -fno-pic -mfdpic generated code is like regular -fno-pic, not suitable
>>> for FDPIC (absolute addressing for symbol references and no function
>>> descriptor).  The sh port simply upgrades -fno-pic to -fpie by setting
>>> flag_pic.  Let's follow suit.
>>>
>>> Link: 
>>> https://inbox.sourceware.org/gcc-patches/20150913165303.gc17...@brightrain.aerifal.cx/
>>>
>>> gcc/ChangeLog:
>>>
>>> * config/arm/arm.cc (arm_option_override): Set flag_pic if
>>>   TARGET_FDPIC.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>> * gcc.target/arm/fdpic-pie.c: New test.
>>> ---
>>>  gcc/config/arm/arm.cc|  6 +
>>>  gcc/testsuite/gcc.target/arm/fdpic-pie.c | 30 
>>>  2 files changed, 36 insertions(+)
>>>  create mode 100644 gcc/testsuite/gcc.target/arm/fdpic-pie.c
>>>
>>> diff --git a/gcc/config/arm/arm.cc b/gcc/config/arm/arm.cc
>>> index 1cd69268ee9..f2fd3cce48c 100644
>>> --- a/gcc/config/arm/arm.cc
>>> +++ b/gcc/config/arm/arm.cc
>>> @@ -3682,6 +3682,12 @@ arm_option_override (void)
>>>arm_pic_register = FDPIC_REGNUM;
>>>if (TARGET_THUMB1)
>>> sorry ("FDPIC mode is not supported in Thumb-1 mode");
>>> +
>>> +  /* FDPIC code is a special form of PIC, and the vast majority of code
>>> +generation constraints that apply to PIC also apply to FDPIC, so we
>>> + set flag_pic to avoid the need to check TARGET_FDPIC everywhere
>>> + flag_pic is checked. */
>>> +  flag_pic = 2;
>>>  }
>> 
>> Been a while since I looked at this stuff but should this not be
>> flag_pie being set rather than flag_pic here if the expectation is to
>> turn -fno-PIC -mfdpic into fPIE ?
>
>-fPIE implies -fPIC, but has the added implication that any definition of an 
>object we see is the one true definition and cannot be pre-empted during 
>loading (in a shared library, a definition of X may be pre-empted by another 
>definition of X in either the application itself or another shared library 
>that was loaded first).
>
>Part of the confusion comes from the manual, though:
>
>Select the FDPIC ABI, which uses 64-bit function descriptors to
>represent pointers to functions.  When the compiler is configured for
>@code{arm-*-uclinuxfdpiceabi} targets, this option is on by default
>and implies @option{-fPIE} if none of the PIC/PIE-related options is
>provided.  On other targets, it only enables the FDPIC-specific code
>generation features, and the user should explicitly provide the
>PIC/PIE-related options as needed.
>
>Which conflates things relating to the option flag and the compiler 
>configuration.  I think that needs clearing up as well.  Something like
>
>Select the FDPIC ABI, which uses 64-bit function descriptors to
>represent pointers to functions.  @option{-mfdpic} implies @option{-fPIC}.
>
>When the compiler is configured for @code{arm-*-uclinuxfdpiceabi} targets, 
>this option is on by default and the compiler defaults to @option{-fPIE}, 
>unless @option{-fPIC} is explicitly specified.
>
>might cover it, but I'm not sure I've fully untangled the web of option 
>permutations here.  Perhaps someone could tabulate the expected options 
>somewhere for clarity.

yep, I think that's about it. fore please  TIA

>
>The other option would be to error if flag_pic is not set, when -mfdpic is 
>set, which would force the user to be explicit as to which pic options they 
>want (technically the explicit combination -mno-pic -mfdpic has no meaning).

nod

>
>R.
>
>> 
>> 
>>>
>>>if (arm_pic_register_string != NULL)
>>> diff --git a/gcc/testsuite/gcc.target/arm/fdpic-pie.c 
>>> b/gcc/testsuite/gcc.target/arm/fdpic-pie.c
>>> new file mode 100644
>>> index 000..909db8bce74
>>> --- /dev/null
>>> +++ b/gcc/testsuite/gcc.target/arm/fdpic-pie.c
>>> @@ -0,0 +1,30 @@
>>> +// { dg-do compile }
>>> +// { dg-options "-O2 -fno-pic -mfdpic" }
>>> +// { dg-skip-if "-mpure-code and -fPIC incompatible" { *-*-* } { 
>>> "-mpure-code" } }
>>> +
>>> +__attribute__((visibility("hidden"))) void hidden_fun(void);
>>> +void fun(void);
>>> +__attribute__((visibility("hidden"))) extern int hidden_var;
>>> +extern int var;
>>> +__attribute__((visibility("hidden"))) const int ro_hidden_var = 42;
>>> +
>>> +// { dg-final { scan-assembler "hidden_fun\\(GOTOFFFUNCDESC\\)" } }
>>> +void *addr_hidden_fun(void) { return hidden_fun; }
>>> +
>>> +// { dg-final { scan-assembler "fun\\(GOTFUNCDESC\\)" } }
>>> +void *addr_fun(void) { return fun; }
>>> +
>>> +// { dg-final { scan-assembler "hidden_var\\(GOT\\)" } }
>>> +void *addr_hidden_var(void) { return &hidden_var; }
>>> +
>>> +// { dg-final { scan-assembler "var\\(GOT\\)" } }
>>> +void *addr_var(void) { return &var; }
>>> +
>>> +// { dg-final { scan-assembler ".LANCHOR0\\(GOT\\)" } }
>>> +const int *addr_

Re: [PATCH] arm: Force flag_pic for FDPIC

[rep . dot . nop]

On 26 September 2024 20:51:55 CEST, Fangrui Song  wrote:
>On Thu, Sep 26, 2024 at 11:21 AM Ramana Radhakrishnan
> wrote:
>>
>> On Mon, Mar 4, 2024 at 1:43 PM Fangrui Song  wrote:
>> >
>> > From: Fangrui Song 
>> >
>> > -fno-pic -mfdpic generated code is like regular -fno-pic, not suitable
>> > for FDPIC (absolute addressing for symbol references and no function
>> > descriptor).  The sh port simply upgrades -fno-pic to -fpie by setting
>> > flag_pic.  Let's follow suit.
>> >
>> > Link: 
>> > https://inbox.sourceware.org/gcc-patches/20150913165303.gc17...@brightrain.aerifal.cx/
>> >
>> > gcc/ChangeLog:
>> >
>> > * config/arm/arm.cc (arm_option_override): Set flag_pic if
>> >   TARGET_FDPIC.
>> >
>> > gcc/testsuite/ChangeLog:
>> >
>> > * gcc.target/arm/fdpic-pie.c: New test.
>> > ---
>> >  gcc/config/arm/arm.cc|  6 +
>> >  gcc/testsuite/gcc.target/arm/fdpic-pie.c | 30 
>> >  2 files changed, 36 insertions(+)
>> >  create mode 100644 gcc/testsuite/gcc.target/arm/fdpic-pie.c
>> >
>> > diff --git a/gcc/config/arm/arm.cc b/gcc/config/arm/arm.cc
>> > index 1cd69268ee9..f2fd3cce48c 100644
>> > --- a/gcc/config/arm/arm.cc
>> > +++ b/gcc/config/arm/arm.cc
>> > @@ -3682,6 +3682,12 @@ arm_option_override (void)
>> >arm_pic_register = FDPIC_REGNUM;
>> >if (TARGET_THUMB1)
>> > sorry ("FDPIC mode is not supported in Thumb-1 mode");
>> > +
>> > +  /* FDPIC code is a special form of PIC, and the vast majority of 
>> > code
>> > +generation constraints that apply to PIC also apply to FDPIC, so 
>> > we
>> > + set flag_pic to avoid the need to check TARGET_FDPIC everywhere
>> > + flag_pic is checked. */
>> > +  flag_pic = 2;
>> >  }
>>
>> Been a while since I looked at this stuff but should this not be
>> flag_pie being set rather than flag_pic here if the expectation is to
>> turn -fno-PIC -mfdpic into fPIE ?
>
>I think flag_pic determines the PIC/PIE vs no-PIC difference
>while flag_shlib determines the PIC vs PIE difference (whether
>non-local default linkage symbol might bind externally).
>
>Setting flag_pic here is correct.

indeed.
pic 2 lgtm
thanks

>
>>
>> >
>> >if (arm_pic_register_string != NULL)
>> > diff --git a/gcc/testsuite/gcc.target/arm/fdpic-pie.c 
>> > b/gcc/testsuite/gcc.target/arm/fdpic-pie.c
>> > new file mode 100644
>> > index 000..909db8bce74
>> > --- /dev/null
>> > +++ b/gcc/testsuite/gcc.target/arm/fdpic-pie.c
>> > @@ -0,0 +1,30 @@
>> > +// { dg-do compile }
>> > +// { dg-options "-O2 -fno-pic -mfdpic" }
>> > +// { dg-skip-if "-mpure-code and -fPIC incompatible" { *-*-* } { 
>> > "-mpure-code" } }
>> > +
>> > +__attribute__((visibility("hidden"))) void hidden_fun(void);
>> > +void fun(void);
>> > +__attribute__((visibility("hidden"))) extern int hidden_var;
>> > +extern int var;
>> > +__attribute__((visibility("hidden"))) const int ro_hidden_var = 42;
>> > +
>> > +// { dg-final { scan-assembler "hidden_fun\\(GOTOFFFUNCDESC\\)" } }
>> > +void *addr_hidden_fun(void) { return hidden_fun; }
>> > +
>> > +// { dg-final { scan-assembler "fun\\(GOTFUNCDESC\\)" } }
>> > +void *addr_fun(void) { return fun; }
>> > +
>> > +// { dg-final { scan-assembler "hidden_var\\(GOT\\)" } }
>> > +void *addr_hidden_var(void) { return &hidden_var; }
>> > +
>> > +// { dg-final { scan-assembler "var\\(GOT\\)" } }
>> > +void *addr_var(void) { return &var; }
>> > +
>> > +// { dg-final { scan-assembler ".LANCHOR0\\(GOT\\)" } }
>> > +const int *addr_ro_hidden_var(void) { return &ro_hidden_var; }
>> > +
>> > +// { dg-final { scan-assembler "hidden_var\\(GOT\\)" } }
>> > +int read_hidden_var(void) { return hidden_var; }
>> > +
>> > +// { dg-final { scan-assembler "var\\(GOT\\)" } }
>> > +int read_var(void) { return var; }
>> > --
>> > 2.44.0.rc1.240.g4c46232300-goog
>> >
>
>
>

Re: [PATCH] Fortran: fix front-end GMP memleaks

[rep . dot . nop]

On 22 December 2024 21:50:41 CET, Harald Anlauf  wrote:
>Dear all,
>
>please find attached fixes for GMP memleaks in the gfortran frontend
>found when running f951 under valgrind.  One of them surfaced when
>looking at a testcases that invoved pointer rank remapping.  After a
>successful gfc_array_size we need to clear the size returned as a
>gmp variable.  Further scanning revealed two places in RANDOM_SEED
>when checking the GET= and PUT= array argument sizes.
>
>Regtested on x86_64-pc-linux-gnu.
>
>I intend to commit to mainline as obvious within 24h unless there
>are objections.

LGTM
thanks for the patch!

>
>Thanks,
>Harald
>

Re: [PATCH 2/4] cfgloopmanip: Add infrastructure for scaling of multi-exit loops [PR117790]

[rep . dot . nop]

On 6 January 2025 12:34:45 CET, Alex Coplan  wrote:

nit:
s/caling/calling/

thanks