Re: [PATCH v1 2/6] Extract ix86 dllimport implementation to mingw
On Thu, May 23, 2024 at 7:53 PM Evgeny Karpov wrote: > > > Thursday, May 23, 2024 10:35 AM > Uros Bizjak wrote: > > > Richard Sandiford wrote: > > > > > > > This looks good to me apart from a couple of very minor comments > > > > below, but please get approval from the x86 maintainers as well. In > > > > particular, they might prefer to handle ix86_legitimize_pe_coff_symbol > > > > in > > some other way. > > > > > > Jan and Uros, could you please review x86 refactoring for mingw part? > > > > Yes, perhaps legitimize_pe_coff_symbol should be handled similar to how > > machopic_legitimize_pic_address is handled.and just use "#if TARGET_PECOFF" > > at call sites when calling functions from the new winnt-dll.h. This would > > also > > allow us to remove the early check for !TARGET_PECOFF in > > legitimize_pe_coff_symbol. > > > > Uros. > > > The function legitimize_pe_coff_symbol is now part of mingw and will not be > used for linux targets. > This is why ix86_legitimize_pe_coff_symbol has been introduced, to be > available for all platforms. There is no need for a ix86_legitimize_pe_coff_symbol. This function is now defined in a header that is not included by default, so the call sites should use #if TARGET_PECOFF to isolate its use. Please see how "#if TARGET_MACHO" is used in config/i386/* files for the similar issue. I think that TARGET_PECOFF should follow this example. Uros.
Re: [PATCH v1 2/6] Extract ix86 dllimport implementation to mingw
Thursday, May 23, 2024 10:35 AM Uros Bizjak wrote: > Richard Sandiford wrote: > > > > > This looks good to me apart from a couple of very minor comments > > > below, but please get approval from the x86 maintainers as well. In > > > particular, they might prefer to handle ix86_legitimize_pe_coff_symbol in > some other way. > > > > Jan and Uros, could you please review x86 refactoring for mingw part? > > Yes, perhaps legitimize_pe_coff_symbol should be handled similar to how > machopic_legitimize_pic_address is handled.and just use "#if TARGET_PECOFF" > at call sites when calling functions from the new winnt-dll.h. This would also > allow us to remove the early check for !TARGET_PECOFF in > legitimize_pe_coff_symbol. > > Uros. The function legitimize_pe_coff_symbol is now part of mingw and will not be used for linux targets. This is why ix86_legitimize_pe_coff_symbol has been introduced, to be available for all platforms. Regards, Evgeny
Re: [PATCH v1 2/6] Extract ix86 dllimport implementation to mingw
On Thu, May 23, 2024 at 10:35 AM Uros Bizjak wrote: > > On Wed, May 22, 2024 at 4:32 PM Evgeny Karpov > wrote: > > > > Wednesday, May 22, 2024 1:06 PM > > Richard Sandiford wrote: > > > > > This looks good to me apart from a couple of very minor comments below, > > > but > > > please get approval from the x86 maintainers as well. In particular, > > > they might > > > prefer to handle ix86_legitimize_pe_coff_symbol in some other way. > > > > Thanks, Richard, for the review! > > The suggestions will be addressed in the next version. > > > > Jan and Uros, could you please review x86 refactoring for mingw part? > > Thanks. > > Yes, perhaps legitimize_pe_coff_symbol should be handled similar to > how machopic_legitimize_pic_address is handled.and just use "#if > TARGET_PECOFF" at call sites when calling functions from the new > winnt-dll.h. This would also allow us to remove the early check for > !TARGET_PECOFF in legitimize_pe_coff_symbol. Maybe you should look how TARGET_MACHO is handled in config/i386/* files. Uros.
Re: [PATCH v1 2/6] Extract ix86 dllimport implementation to mingw
On Wed, May 22, 2024 at 4:32 PM Evgeny Karpov wrote: > > Wednesday, May 22, 2024 1:06 PM > Richard Sandiford wrote: > > > This looks good to me apart from a couple of very minor comments below, but > > please get approval from the x86 maintainers as well. In particular, they > > might > > prefer to handle ix86_legitimize_pe_coff_symbol in some other way. > > Thanks, Richard, for the review! > The suggestions will be addressed in the next version. > > Jan and Uros, could you please review x86 refactoring for mingw part? Thanks. Yes, perhaps legitimize_pe_coff_symbol should be handled similar to how machopic_legitimize_pic_address is handled.and just use "#if TARGET_PECOFF" at call sites when calling functions from the new winnt-dll.h. This would also allow us to remove the early check for !TARGET_PECOFF in legitimize_pe_coff_symbol. Uros.
Re: [PATCH v1 2/6] Extract ix86 dllimport implementation to mingw
Wednesday, May 22, 2024 1:06 PM Richard Sandiford wrote: > This looks good to me apart from a couple of very minor comments below, but > please get approval from the x86 maintainers as well. In particular, they > might > prefer to handle ix86_legitimize_pe_coff_symbol in some other way. Thanks, Richard, for the review! The suggestions will be addressed in the next version. Jan and Uros, could you please review x86 refactoring for mingw part? Thanks. Regards, Evgeny
Re: [PATCH v1 2/6] Extract ix86 dllimport implementation to mingw
Evgeny Karpov writes: > This patch extracts the ix86 implementation for expanding a SYMBOL > into its corresponding dllimport, far-address, or refptr symbol. > It will be reused in the aarch64-w64-mingw32 target. > The implementation is copied as is from i386/i386.cc with > minor changes to follow to the code style. > > Also this patch replaces the original DLL import/export > implementation in ix86 with mingw. > > gcc/ChangeLog: > > * config.gcc: Add winnt-dll.o, which contains the DLL > import/export implementation. > * config/i386/cygming.h (SUB_TARGET_RECORD_STUB): Remove the > old implementation. Rename the required function to MinGW. > Rename it to a conditional function that will reuse the > MinGW implementation for COFF and nothing otherwise. > * config/i386/i386-expand.cc (ix86_expand_move): Likewise. > * config/i386/i386-expand.h (is_imported_p): Likewise. > (mingw_GOT_alias_set): Likewise. > (ix86_legitimize_pe_coff_symbol): Likewise. > * config/i386/i386-protos.h: Likewise. > * config/i386/i386.cc (is_imported_p): Likewise. > (ix86_legitimize_pe_coff_symbol): Likewise. > (ix86_GOT_alias_set): Likewise. > (legitimize_pic_address): Likewise. > (struct dllimport_hasher): > (GTY): Likewise. > (get_dllimport_decl): Likewise. > (legitimize_pe_coff_extern_decl): Likewise. > (legitimize_dllimport_symbol): Likewise. > (legitimize_pe_coff_symbol): Likewise. > (ix86_legitimize_address): Likewise. > * config/mingw/winnt.h (mingw_pe_record_stub): Likewise. > * config/mingw/winnt.cc (i386_pe_record_stub): Likewise. > (mingw_pe_record_stub): Likewise. > * config/mingw/t-cygming: Add the winnt-dll.o compilation. > * config/mingw/winnt-dll.cc: New file. This looks good to me apart from a couple of very minor comments below, but please get approval from the x86 maintainers as well. In particular, they might prefer to handle ix86_legitimize_pe_coff_symbol in some other way. > [...] > diff --git a/gcc/config/mingw/winnt-dll.cc b/gcc/config/mingw/winnt-dll.cc > new file mode 100644 > index 000..349ade6f5c0 > --- /dev/null > +++ b/gcc/config/mingw/winnt-dll.cc > @@ -0,0 +1,229 @@ > +/* Expand a SYMBOL into its corresponding dllimport, far-address, > +or refptr symbol. > +Copyright (C) 2024 Free Software Foundation, Inc. I suppose this should retain the range from the i386 file that the code is moving from: Copyright (C) 1988-2024 Free Software Foundation, Inc. > [...] > diff --git a/gcc/config/mingw/winnt-dll.h b/gcc/config/mingw/winnt-dll.h > new file mode 100644 > index 000..19c16e747a2 > --- /dev/null > +++ b/gcc/config/mingw/winnt-dll.h > @@ -0,0 +1,26 @@ > +/* Expand a SYMBOL into its corresponding dllimport, far-address, > +or refptr symbol. > +Copyright (C) 2024 Free Software Foundation, Inc. > + > +GCC is free software; you can redistribute it and/or modify it under > +the terms of the GNU General Public License as published by the Free > +Software Foundation; either version 3, or (at your option) any later > +version. > + > +GCC is distributed in the hope that it will be useful, but WITHOUT ANY > +WARRANTY; without even the implied warranty of MERCHANTABILITY or > +FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License > +for more details. > + > +You should have received a copy of the GNU General Public License > +along with GCC; see the file COPYING3. If not see > +http://www.gnu.org/licenses/. */ > + > +#ifndef GCC_MINGW_WINNT_DLL_H > +#define GCC_MINGW_WINNT_DLL_H > + > +extern bool is_imported_p (rtx x); > +extern alias_set_type mingw_GOT_alias_set (void); > +extern rtx legitimize_pe_coff_symbol (rtx addr, bool inreg); > + > +#endif > \ No newline at end of file Would be good to add the newlihe. Thanks, Richard
[PATCH v1 2/6] Extract ix86 dllimport implementation to mingw
This patch extracts the ix86 implementation for expanding a SYMBOL
into its corresponding dllimport, far-address, or refptr symbol.
It will be reused in the aarch64-w64-mingw32 target.
The implementation is copied as is from i386/i386.cc with
minor changes to follow to the code style.
Also this patch replaces the original DLL import/export
implementation in ix86 with mingw.
gcc/ChangeLog:
* config.gcc: Add winnt-dll.o, which contains the DLL
import/export implementation.
* config/i386/cygming.h (SUB_TARGET_RECORD_STUB): Remove the
old implementation. Rename the required function to MinGW.
Rename it to a conditional function that will reuse the
MinGW implementation for COFF and nothing otherwise.
* config/i386/i386-expand.cc (ix86_expand_move): Likewise.
* config/i386/i386-expand.h (is_imported_p): Likewise.
(mingw_GOT_alias_set): Likewise.
(ix86_legitimize_pe_coff_symbol): Likewise.
* config/i386/i386-protos.h: Likewise.
* config/i386/i386.cc (is_imported_p): Likewise.
(ix86_legitimize_pe_coff_symbol): Likewise.
(ix86_GOT_alias_set): Likewise.
(legitimize_pic_address): Likewise.
(struct dllimport_hasher):
(GTY): Likewise.
(get_dllimport_decl): Likewise.
(legitimize_pe_coff_extern_decl): Likewise.
(legitimize_dllimport_symbol): Likewise.
(legitimize_pe_coff_symbol): Likewise.
(ix86_legitimize_address): Likewise.
* config/mingw/winnt.h (mingw_pe_record_stub): Likewise.
* config/mingw/winnt.cc (i386_pe_record_stub): Likewise.
(mingw_pe_record_stub): Likewise.
* config/mingw/t-cygming: Add the winnt-dll.o compilation.
* config/mingw/winnt-dll.cc: New file.
---
gcc/config.gcc | 6 +-
gcc/config/i386/cygming.h | 2 +-
gcc/config/i386/i386-expand.cc | 2 +-
gcc/config/i386/i386-expand.h | 2 +-
gcc/config/i386/i386-protos.h | 2 +-
gcc/config/i386/i386.cc| 211 --
gcc/config/mingw/t-cygming | 6 +
gcc/config/mingw/winnt-dll.cc | 229 +
gcc/config/mingw/winnt-dll.h | 26
gcc/config/mingw/winnt.cc | 2 +-
gcc/config/mingw/winnt.h | 1 +
11 files changed, 292 insertions(+), 197 deletions(-)
create mode 100644 gcc/config/mingw/winnt-dll.cc
create mode 100644 gcc/config/mingw/winnt-dll.h
diff --git a/gcc/config.gcc b/gcc/config.gcc
index ef7f854735a..be2b20a155c 100644
--- a/gcc/config.gcc
+++ b/gcc/config.gcc
@@ -2181,7 +2181,7 @@ i[34567]86-*-cygwin*)
tmake_file="${tmake_file} mingw/t-cygming t-slibgcc"
target_gtfiles="$target_gtfiles \$(srcdir)/config/mingw/winnt.cc"
extra_options="${extra_options} mingw/cygming.opt i386/cygwin.opt"
- extra_objs="${extra_objs} winnt.o winnt-stubs.o"
+ extra_objs="${extra_objs} winnt.o winnt-stubs.o winnt-dll.o"
c_target_objs="${c_target_objs} msformat-c.o"
cxx_target_objs="${cxx_target_objs} winnt-cxx.o msformat-c.o"
d_target_objs="${d_target_objs} cygwin-d.o"
@@ -2199,7 +2199,7 @@ x86_64-*-cygwin*)
tmake_file="${tmake_file} mingw/t-cygming t-slibgcc"
target_gtfiles="$target_gtfiles \$(srcdir)/config/mingw/winnt.cc"
extra_options="${extra_options} mingw/cygming.opt i386/cygwin.opt"
- extra_objs="${extra_objs} winnt.o winnt-stubs.o"
+ extra_objs="${extra_objs} winnt.o winnt-stubs.o winnt-dll.o"
c_target_objs="${c_target_objs} msformat-c.o"
cxx_target_objs="${cxx_target_objs} winnt-cxx.o msformat-c.o"
d_target_objs="${d_target_objs} cygwin-d.o"
@@ -2283,7 +2283,7 @@ i[34567]86-*-mingw* | x86_64-*-mingw*)
*)
;;
esac
- extra_objs="${extra_objs} winnt.o winnt-stubs.o"
+ extra_objs="${extra_objs} winnt.o winnt-stubs.o winnt-dll.o"
c_target_objs="${c_target_objs} msformat-c.o"
cxx_target_objs="${cxx_target_objs} winnt-cxx.o msformat-c.o"
gas=yes
diff --git a/gcc/config/i386/cygming.h b/gcc/config/i386/cygming.h
index beedf7c398a..4110ceab824 100644
--- a/gcc/config/i386/cygming.h
+++ b/gcc/config/i386/cygming.h
@@ -459,7 +459,7 @@ do {\
#define TARGET_ASM_ASSEMBLE_VISIBILITY i386_pe_assemble_visibility
#undef SUB_TARGET_RECORD_STUB
-#define SUB_TARGET_RECORD_STUB i386_pe_record_stub
+#define SUB_TARGET_RECORD_STUB mingw_pe_record_stub
/* Static stack checking is supported by means of probes. */
#define STACK_CHECK_STATIC_BUILTIN 1
diff --git a/gcc/config/i386/i386-expand.cc b/gcc/config/i386/i386-expand.cc
index 8bb8f21e686..77bf4433aa8 100644
--- a/gcc/config/i386/i386-expand.cc
+++ b/gcc/config/i386/i386-expand.cc
@@ -412,7 +412,7 @@ ix86_expand_move (machine_mode mode, rtx operands[])
}
else
{
- tmp = legitimize_pe_coff_symbol (op1, addend != NULL_RTX)
