From: Rimvydas Jasinskas <rimvydasjas@gmail.com>
To: Harald Anlauf <anlauf@gmx.de>
Cc: Rimvydas Jasinskas via Fortran <fortran@gcc.gnu.org>,
gcc-patches@gnu.org
Subject: Re: Support for WEAK attribute, part 2
Date: Fri, 24 Feb 2023 07:16:51 +0200 [thread overview]
Message-ID: <CAFmAMQ09NyRUrYOuLdY25spnkmd-1E2bnPLprop8p36BTHb-Qg@mail.gmail.com> (raw)
In-Reply-To: <trinity-475912d3-f7e9-4ec9-b5c7-66d0cad9e63e-1677185631233@3c-app-gmx-bap67>
[-- Attachment #1: Type: text/plain, Size: 4323 bytes --]
On Thu, Feb 23, 2023 at 10:53 PM Harald Anlauf <anlauf@gmx.de> wrote:
> the patch is mostly fine, but there is a minor style issue:
>
> + if (sym->attr.ext_attr & (1 << EXT_ATTR_WEAK))
> + gfc_error ("Symbol %qs at %L has the WEAK attribute but is a %s",
> + sym->name, &sym->declared_at, sym->attr.dummy
> + ? "dummy argument" : "local variable");
> +
>
> It is my understanding that this is not translation-friendly.
> Please use separate error texts for either case instead.
Interesting, I was under the impression this was fixed with OO-inlines
around the *.c rename. In any case, adjusted in v2 to use:
+ if (sym->attr.ext_attr & (1 << EXT_ATTR_WEAK))
+ {
+ if (sym->attr.dummy)
+ gfc_error ("Symbol %qs at %L has the WEAK attribute but is a "
+ "dummy argument", sym->name, &sym->declared_at);
+ else
+ gfc_error ("Symbol %qs at %L has the WEAK attribute but is a "
+ "local variable", sym->name, &sym->declared_at);
+ }
> Do we need to really have that many separate files for all
> the tests? Note that each separate file contributes to the
> time developers wait on regtesting to complete. Some of the
> files essentially test only minor variations, like weak-2.f90
> and weak-3.f90.
These testcases are dg-compile and do not go through the "-O0 -O1 -O2
-O3 -Os" options like dg-run. Combining the testcases does not reduce
gfortran.sum a lot:
-PASS: gfortran.dg/weak-2.f90 -O scan-assembler weak[^ \t]*[ \t]_?impl
-PASS: gfortran.dg/weak-2.f90 -O (test for excess errors)
-PASS: gfortran.dg/weak-3.f90 -O scan-assembler weak[^ \t]*[ \t]_?bar__
-PASS: gfortran.dg/weak-3.f90 -O (test for excess errors)
-PASS: gfortran.dg/weak-4.f90 -O scan-assembler weak[^ \t]*[
\t]_?__foo_MOD_abc
-PASS: gfortran.dg/weak-4.f90 -O (test for excess errors)
-PASS: gfortran.dg/weak-5.f90 -O (test for excess errors)
-PASS: gfortran.dg/weak-6.f90 -O (test for errors, line 3)
-PASS: gfortran.dg/weak-6.f90 -O (test for excess errors)
-PASS: gfortran.dg/weak-7.f90 -O (test for errors, line 10)
-PASS: gfortran.dg/weak-7.f90 -O (test for errors, line 6)
-PASS: gfortran.dg/weak-7.f90 -O (test for excess errors)
-PASS: gfortran.dg/weak-8.f90 -O (test for errors, line 3)
-PASS: gfortran.dg/weak-8.f90 -O (test for errors, line 7)
-PASS: gfortran.dg/weak-8.f90 -O (test for excess errors)
+PASS: gfortran.dg/weak-2.f90 -O scan-assembler weak[^ \t]*[
\t]_?__foo_MOD_abc
+PASS: gfortran.dg/weak-2.f90 -O scan-assembler weak[^ \t]*[ \t]_?bar__
+PASS: gfortran.dg/weak-2.f90 -O scan-assembler weak[^ \t]*[ \t]_?impl1
+PASS: gfortran.dg/weak-2.f90 -O (test for excess errors)
+PASS: gfortran.dg/weak-3.f90 -O (test for errors, line 14)
+PASS: gfortran.dg/weak-3.f90 -O (test for errors, line 18)
+PASS: gfortran.dg/weak-3.f90 -O (test for errors, line 24)
+PASS: gfortran.dg/weak-3.f90 -O (test for errors, line 28)
+PASS: gfortran.dg/weak-3.f90 -O (test for errors, line 5)
+PASS: gfortran.dg/weak-3.f90 -O (test for excess errors)
Only benefit is a bit less gfortran/f951 binaries invocations at
expense of potentially introducing issues in what was intended to be
tested. There exists a partial(intentionally or not) sequential
file-scope namespace (like in C) how gfortran parses different units
in the same file. Swapping unit order in the file can affect not only
code generation but diagnostic counts reported. I tend to avoid
having more than one unit per source to avoid dealing with
"borrowing". However with part3 now implemented after debugging, I
guess, samples could be combined to "accepts" + "rejects" two
testcases, Done in v2.
> What is the purpose of testcase weak-5.f90? It's valid
> Fortran, the common block /c/ shows in the assembler and
> does not interfere with the module variable c.
Removed. Issue is not directly related to only the WEAK attributes.
Will be addressed in the future.
> Finally, please do not forget to CC patches to gcc-patches@
> so that others can see them.
Out of curiosity, what is the purpose of CC patches to gcc-patches
too? Attachments are even available in web mailing list too, like in:
https://gcc.gnu.org/pipermail/fortran/2023-February/058953.html
Regards,
Rimvydas
[-- Attachment #2: 0001-Fortran-Add-support-for-WEAK-attribute-for-variables.patch --]
[-- Type: text/x-patch, Size: 3923 bytes --]
From 5b83226c714b17780334b5bad9b17c2266af8232 Mon Sep 17 00:00:00 2001
From: Rimvydas Jasinskas <rimvydas.jas@gmail.com>
Date: Fri, 24 Feb 2023 04:41:00 +0000
Subject: Fortran: Add support for WEAK attribute for variables
Add the rest of the weak-*.f90 testcases.
gcc/fortran/ChangeLog:
* trans-decl.cc (gfc_finish_var_decl): Apply attribute.
(generate_local_decl): Add diagnostic for dummy and local variables.
gcc/testsuite/ChangeLog:
* gfortran.dg/weak-2.f90: New test.
* gfortran.dg/weak-3.f90: New test.
Signed-off-by: Rimvydas Jasinskas <rimvydas.jas@gmail.com>
---
gcc/fortran/trans-decl.cc | 14 +++++++++++++
gcc/testsuite/gfortran.dg/weak-2.f90 | 26 ++++++++++++++++++++++++
gcc/testsuite/gfortran.dg/weak-3.f90 | 30 ++++++++++++++++++++++++++++
3 files changed, 70 insertions(+)
create mode 100644 gcc/testsuite/gfortran.dg/weak-2.f90
create mode 100644 gcc/testsuite/gfortran.dg/weak-3.f90
diff --git a/gcc/fortran/trans-decl.cc b/gcc/fortran/trans-decl.cc
index ff64588b9a8..474920966ec 100644
--- a/gcc/fortran/trans-decl.cc
+++ b/gcc/fortran/trans-decl.cc
@@ -814,6 +814,10 @@ gfc_finish_var_decl (tree decl, gfc_symbol * sym)
&& (TREE_STATIC (decl) || DECL_EXTERNAL (decl)))
set_decl_tls_model (decl, decl_default_tls_model (decl));
+ /* Mark weak variables. */
+ if (sym->attr.ext_attr & (1 << EXT_ATTR_WEAK))
+ declare_weak (decl);
+
gfc_finish_decl_attrs (decl, &sym->attr);
}
@@ -5885,6 +5889,16 @@ generate_local_decl (gfc_symbol * sym)
if (!sym->attr.dummy && !sym->ns->proc_name->attr.entry_master)
generate_dependency_declarations (sym);
+ if (sym->attr.ext_attr & (1 << EXT_ATTR_WEAK))
+ {
+ if (sym->attr.dummy)
+ gfc_error ("Symbol %qs at %L has the WEAK attribute but is a "
+ "dummy argument", sym->name, &sym->declared_at);
+ else
+ gfc_error ("Symbol %qs at %L has the WEAK attribute but is a "
+ "local variable", sym->name, &sym->declared_at);
+ }
+
if (sym->attr.referenced)
gfc_get_symbol_decl (sym);
diff --git a/gcc/testsuite/gfortran.dg/weak-2.f90 b/gcc/testsuite/gfortran.dg/weak-2.f90
new file mode 100644
index 00000000000..3e0e877e903
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/weak-2.f90
@@ -0,0 +1,26 @@
+! { dg-do compile }
+! { dg-require-weak "" }
+! { dg-skip-if "" { x86_64-*-mingw* } }
+! { dg-skip-if "" { nvptx-*-* } }
+
+! 1.
+! { dg-final { scan-assembler "weak\[^ \t\]*\[ \t\]_?__foo_MOD_abc" } }
+module foo
+implicit none
+!GCC$ ATTRIBUTES weak :: abc
+real :: abc(7)
+end module
+
+! 2.
+! { dg-final { scan-assembler "weak\[^ \t\]*\[ \t\]_?impl1" } }
+integer function impl1()
+implicit none
+!GCC$ ATTRIBUTES weak :: impl1
+end function
+
+! 3.
+! { dg-final { scan-assembler "weak\[^ \t\]*\[ \t\]_?bar__" } }
+integer function impl2() bind(c,name='bar__')
+implicit none
+!GCC$ ATTRIBUTES weak :: impl2
+end function
diff --git a/gcc/testsuite/gfortran.dg/weak-3.f90 b/gcc/testsuite/gfortran.dg/weak-3.f90
new file mode 100644
index 00000000000..cfa845921ff
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/weak-3.f90
@@ -0,0 +1,30 @@
+! { dg-do compile }
+! { dg-require-weak "" }
+
+! 1.
+program foo1 ! { dg-error "weak declaration of 'foo1' must be public" "" }
+implicit none
+!GCC$ ATTRIBUTES weak :: foo1
+end program
+
+! 2.
+subroutine foo2
+implicit none
+contains
+ function dar() ! { dg-error "weak declaration of 'dar' must be public" "" }
+ integer :: dar
+!GCC$ ATTRIBUTES weak :: dar
+ end function
+ subroutine bar ! { dg-error "weak declaration of 'bar' must be public" "" }
+!GCC$ ATTRIBUTES weak :: bar
+ end subroutine
+end subroutine
+
+! 3.
+subroutine foo3(n) ! { dg-error "has the WEAK attribute but is a dummy argument" "" }
+implicit none
+integer :: n
+!GCC$ ATTRIBUTES weak :: n
+real :: abc ! { dg-error "has the WEAK attribute but is a local variable" "" }
+!GCC$ ATTRIBUTES weak :: abc
+end subroutine
--
2.39.2
next prev parent reply other threads:[~2023-02-24 5:17 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-02-10 5:42 Support for NOINLINE attribute Rimvydas Jasinskas
2023-02-10 8:24 ` Steve Kargl
2023-02-10 8:38 ` Rimvydas Jasinskas
2023-02-10 18:53 ` Steve Kargl
2023-02-10 21:07 ` Harald Anlauf
2023-02-10 21:16 ` Steve Kargl
2023-02-10 22:16 ` Rimvydas Jasinskas
2023-02-11 21:26 ` Harald Anlauf
2023-02-12 6:59 ` Rimvydas Jasinskas
2023-02-12 21:28 ` Harald Anlauf
2023-02-13 17:50 ` Harald Anlauf
2023-02-14 9:35 ` nvptx: Adjust 'scan-assembler' in 'gfortran.dg/weak-1.f90' (was: Support for NOINLINE attribute) Thomas Schwinge
2023-02-14 19:55 ` Harald Anlauf
2023-02-15 20:58 ` Support for WEAK attribute, part 2 Rimvydas Jasinskas
2023-02-16 21:50 ` Harald Anlauf
2023-02-23 13:55 ` Rimvydas Jasinskas
2023-02-23 20:53 ` Harald Anlauf
2023-02-24 5:16 ` Rimvydas Jasinskas [this message]
2023-02-24 22:03 ` Harald Anlauf
2023-03-28 21:06 ` Enable 'gfortran.dg/weak-2.f90' for nvptx target (was: Support for WEAK attribute, part 2) Thomas Schwinge
2023-02-18 20:35 ` Support for NOINLINE attribute Bernhard Reutner-Fischer
2023-02-24 7:19 ` Bernhard Reutner-Fischer
2023-02-24 12:02 ` Rimvydas Jasinskas
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAFmAMQ09NyRUrYOuLdY25spnkmd-1E2bnPLprop8p36BTHb-Qg@mail.gmail.com \
--to=rimvydasjas@gmail.com \
--cc=anlauf@gmx.de \
--cc=fortran@gcc.gnu.org \
--cc=gcc-patches@gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).