Support for WEAK attribute, part 2

[Rimvydas Jasinskas <rimvydasjas@gmail.com>]

On Tue, Feb 14, 2023 at 9:55 PM Harald Anlauf <anlauf@gmx.de> wrote:
> >>> There is one thing I cannot test, which is the handling of weak symbols
> >>> on other platforms.  A quick glance at the C testcases suggests that
> >>> someone with access to either an NVPTX or MingGW target might tell
> >>> whether that particular target should be excluded.
While working on part 2 patch for weak variables I noticed that MingGW
has quite different handling of weak symbols as weak externals between
32/64.  I think adding "! { dg-skip-if "" { x86-64-*-mingw*} }" is
needed to weak-1.f90 testcase, but I decided to wait for testsuite
failure reports.

> > And I'm curious, is '!GCC$ ATTRIBUTES weak' meant to be used only for
> > weak definitions (like in 'gfortran.dg/weak-1.f90'), or also for weak
> > declarations (which, for example, in the C world then evaluate to
> > zero-address unless actually defined)?  When I did a quick experiment,
> > that didn't seem to work?  (But may be my fault, of course.)
I really would avoid anything, even suggesting that it is possible to
do things like: "if (LOC(foo) > 0) call foo" directly in Fortran code.
I do not think this is possible with gfortran linked main programs.
Explicit rejection would need to be added if it is.  There is no need
to deal with segaulting programs like:
$ cat main.c
#pragma weak foo_
extern void foo_();
void main(void){ foo_(); }

> > And, orthogonally: is '!GCC$ ATTRIBUTES weak' meant to be used only for
> > subroutines (like in 'gfortran.dg/weak-1.f90') and also functions (I
> > suppose; test case?), or also for weak "data" in some way (which, for
> > example, in the C world then evaluates to a zero-address unless actually
> > defined)?
Weak zero-addressed "data" is more complicated between targets,  I do
not see a need for it.
Functions are handled the same as subroutines, I had testcase for it,
but decided to deal with fallout from weak-1.f90 first:
$ cat gcc/testsuite/gfortran.dg/weak-2.f90
! { dg-do compile }
! { dg-require-weak "" }
! { dg-final { scan-assembler "weak\[^ \t\]*\[ \t\]_?impl" } }
integer function impl()
implicit none
!GCC$ ATTRIBUTES weak :: impl
end function

> Syntax and use of the WEAK directive.
> !DIR$ WEAK procedure_name[, procedure_name] ...
> !DIR$ WEAK procedure_name= stub_name[, procedure_name1= stub_name1] ...
> stub_name
>      A stub procedure that exists in the code. The stub_name will be
> called if a strong reference does not exist for procedure_name. The
> stub_name procedure must have the same name and dummy argument list as
> procedure_name.
I think the same for procedure_name=stub_name can be achieved with
bind(c,'sym') already without caring about symbol name mangling:
$ cat gcc/testsuite/gfortran.dg/weak-6.f90
! { dg-do compile }
! { dg-require-weak "" }
! { dg-final { scan-assembler "weak\[^ \t\]*\[ \t\]_?bar__" } }
integer function impl() bind(c,name='bar__')
implicit none
!GCC$ ATTRIBUTES weak :: impl
end function

> I'm not sure whether we need to support weak symbols other than
> procedures in gfortran.  Maybe Rimvydas can comment on this.
In 2nd part patch I was thinking to add support for weak global
variables (like in modules):
--- 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);
 }

$ cat gcc/testsuite/gfortran.dg/weak-3.f90
! { dg-do compile }
! { dg-require-weak "" }
! { dg-skip-if "" { x86_64-*-mingw* } }
! { dg-skip-if "" { nvptx-*-* } }
! { dg-final { scan-assembler "weak\[^ \t\]*\[ \t\]_?__foo_MOD_abc" } }
module foo
implicit none
!GCC$ ATTRIBUTES weak :: abc
real :: abc(7)
end module

The catch is again scan-assembler issues with various symbol name
manglings.  Maybe just to add testcase without scan-assembler to check
if testcase is not rejected?

Currently already rejected are:
$ cat gcc/testsuite/gfortran.dg/weak-4.f90
! { dg-do compile }
! { dg-require-weak "" }
program foo  ! { dg-error "weak declaration of 'foo' must be public" "" }
implicit none
!GCC$ ATTRIBUTES weak :: foo
end program

$ cat gcc/testsuite/gfortran.dg/weak-9.f90
! { dg-do compile }
! { dg-require-weak "" }
subroutine foo
implicit none
real :: abc     ! { dg-error "weak declaration of 'abc' must be public" "" }
!GCC$ ATTRIBUTES weak :: abc
print *, abc
contains
 subroutine bar ! { dg-error "weak declaration of 'bar' must be public" "" }
!GCC$ ATTRIBUTES weak :: bar
 end subroutine
end subroutine

However error is not given if 'abc' is made a dummy argument or is not
used, any ideas why?

$ cat gcc/testsuite/gfortran.dg/weak-8.f90
! { dg-do compile }
! { dg-require-weak "" }
module foo
implicit none
real :: a,b
common /c/ a,b
!GCC$ ATTRIBUTES weak :: c
! { dg-error "has no IMPLICIT type" "common" {target *-*-*} .-1 }
end module

I suspect declaring common blocks weak should be explicitly rejected
somewhere in resolve.cc, since .comm + .weak (should?) be illegal:
$ cat cweak.s
.comm c_,8,16
.weak c_
$ as cweak.s
cweak.s: Assembler messages:
cweak.s: Error: symbol `c_' can not be both weak and common
However in weak-8.f90 error depends if "implicit none" is given.

Also what to do with declarations like:
$ cat gcc/testsuite/gfortran.dg/weak-7.f90
! { dg-do compile }
! { dg-require-weak "" }
module foo
implicit none
!GCC$ ATTRIBUTES weak :: foo
integer :: ijk
end module

Currently it is silently accepted and has no effect.  I would see it
useful if such a declaration would make all publicly visible variables
and procedures to be marked as weak (as a shorthand, instead of
declaring each variable/procedure as weak).  But I have not looked up
how to do this yet.
Similar issue is with other attributes too:
module aaa
!GCC$ ATTRIBUTES fastcall :: aaa
!GCC$ ATTRIBUTES deprecated :: aaa
!GCC$ ATTRIBUTES noinline :: aaa
integer :: ijk
end module

If such declarations to be explicitly rejected/diagnosed (say under
-Wattributes) some kind of table like in
gcc/c-family/c-attribs.cc:c_common_attribute_table[] would be needed,
where false/true indicate if attribute could be applied for a given
use case.
On a side note, could someone check if cdecl/stdcall/fastcall actually
affect code generation on windows targets?

In the end, the most problematic issue is useability of GCC directives
in user codes.  As more and more attributes or other directives get
supported it tends to create chaos in how users (try to) make use of
these attributes.  The NO_ARGS_CHECK is/was a good example.  The mpi
libraries generate bindings on the fly after checking if this
attribute is supported.  Other projects use cpp preprocessor "#if
__GNUC__ > 8", sometimes even in wrong ways like "#if __GFORTRAN__ >
10" or explicit rules like "cc -E -P -traditional -xc foo.F90 >
foo.f90 && $(FC) -c foo.f90" while not suspecting that system cc(1)
and say gfortran-10 could be of different versions on given host
environment (or even not gfortran), specially on systems where cc(1)
is 4.8.5.  The __GFORTRAN__ macro defined to '1' seems like a lost
opportunity to use it as a compiler version major or maybe even
currently active Fortran standard level.
Issue is that unlike C frontends, gfortran hard rejects compilation
units containing unknown attributes or slightly expanded new syntax:
$ cat cold.f90
subroutine foo
!GCC$ ATTRIBUTES cold :: foo
end subroutine
$ gfortran -c cold.f90
Error: Unknown attribute in !GCC$ ATTRIBUTES statement at (1)

I have a patch that adds -fallow-ignored-directive to demote such
errors to warnings, but it looks too ugly and does not solve the
useability issues by itself.
What about adding support for conditional directives based on the
compiler version?  Simple "!GCC$ if(13+) ATTRIBUTES weak :: foo" would
in theory allow to keep the same sourcecode with different compiler
versions and not require cpp preprocessor tricks once this feature
"age" enough.

Any thoughts?

Regards,
Rimvydas