Re: [PATCH v2 00/13] Add aarch64-w64-mingw32 target

2024-03-19 Thread Martin Storsjö

On Mon, 18 Mar 2024, Fangrui Song wrote:


LLVM has had an aarch64 mingw ABI support for a long time. Does this
patch series introduce a different ABI?
If yes, do you have a summary?


This patchset in itself does not reach ABI compatibility with the 
preexisting aarch64 mingw ecosystem - but this is also just the first 
patchset to lay out the groundwork for a new mingw target within GCC.


As far as I've understood, the divergence is not intended, and they are 
working on converging towards compatibility - but such bits are to be 
handled in later patchsets.


Off the top of my head, the major missing pieces wrt compatbility are the 
variadic calling convention, and using 64 bit doubles for "long double" 
(just like for Darwin).



Does the patch need any adaptation on the LLVM side, or should a
different target triple be picked?


No, I don't think a different target triple should be used - it is 
intended to be the same, but compatibility is a work in progress.



I have always been wondering what "32" in "x86_x64-w64-mingw32" means.


(Nit, I presume you meant "x86_64-w64-mingw32".)

As Andrew replied, w32 stands for win32, which isn't so much about the 
bitness as "the current windows API as of the last 30 years, as opposed to 
win16". So it's more of a name than something indicating any form of 
bitness.


Likewise, "w64" just indicates which vendor/fork is involved, so we also 
have i686-w64-mingw32 and armv7-w64-mingw32, for the 32 bit ABIs with an 
SDK from the same vendor.



https://github.com/llvm/llvm-project/pull/78908 even introduced the
first use of the triple "arm64ec-w64-mingw32" into llvm-project.


ARM64EC is a totally different thing though; that's an entirely separate 
ABI on almost every single level.


// Martin



Re: [PATCH v1 02/13] aarch64: The aarch64-w64-mingw32 target implements

2024-02-23 Thread Martin Storsjö

On Fri, 23 Feb 2024, Richard Sandiford wrote:

Are there two distinct ABIs for aarch64-*-mingw*?  Or are these 
distinctions ignored on aarch64 and just retained for compatibility?


On Windows on AArch64, the calling convention normally matches regular 
AAPCS64 - so the ms_abi attribute normally has no effect. However, for 
variadic functions, the calling convention differs, so the ms_abi 
attribute could be used to implement functions with the Windows vararg 
calling convention on Linux.


(As far as I know, the correct Windows vararg calling convention is not 
yet implemented in this patch series, but would be a later addition.)


Clang/LLVM does implement the Windows AArch64 vararg calling convention, 
and it used to be necessary for Wine on AArch64 before, but as Jacek 
mentioned, it's no longer needed by Wine.


ARM64EC is an entirely different thing though, both out of scope for this 
patchset, and also a much bigger thing than an MS_ABI attribute.


// Martin



Re: [PATCH] Add -gcodeview option

2022-10-25 Thread Martin Storsjö

On Tue, 25 Oct 2022, Mark Harmstone wrote:


On 24/10/22 12:08, Martin Storsjö wrote:
Hmm, what does this end up passing to the linker in the end - does it just 
pass "-pdb="? (What does the "*" parameter do here?) If that's the case - 
that sounds reasonable - assuming that if a user passes an extra 
-Wl,--pdb,myspecificname.pdb, that would take precedence (i.e. be passed 
after the compiler's default one). 


That's right. The "*" means "all languages".


Ok, great.

Btw, stylistically, should we strive towards using double dashes for the 
pdb option? I think that's the most canonical way for such getopt options 
(even though it doesn't make any practical difference). I've started 
trying to update various users to prefer that form (together with 
preferring -Wl,--pdb= over -Wl,--pdb,) and probably will send 
a few more.


// Martin


Re: [PATCH] Add -gcodeview option

2022-10-24 Thread Martin Storsjö

On Mon, 24 Oct 2022, Mark Harmstone wrote:


Both current lld and the next version of ld have an option -pdb, which
creates a PDB file which Microsoft's debuggers can use. This patch adds
a -gcodeview option, which passes this to the linker.

I do intend to expand this so it also creates the .debug$S and .debug$T
sections which would make this useful - I submitted patches for this a
while back, but they need to be rewritten to parse the DWARF DIEs rather
than using debug_hooks.

Clang also has -gcodeview, but AFAICS only uses it for .debug$S and
.debug$T, and doesn't use it for linker options (though IMO it probably
should).


That's true - in Clang, this option doesn't affect linking, it only 
affects code generation.


(FWIW, if I understand it correctly, Clang also does support generating 
both DWARF and CodeView at the same time - I think that would require 
passing something like "-g -gdwarf-4 -gcodeview" at the same time - but I 
don't have experience with playing with such setups.)


Another vague oddity in how this option is handled in Clang, is that if I 
only pass "-gcodeview" to the compiler, it doesn't actually generate any 
debug info (it just changes preference, in case I would request debug info 
separately), while one has to pass e.g. "-g -gcodeview" for it to do 
what's expected. I'm not sure if this is the same with dwarf, or if 
passing "-gdwarf-4" is enough for actually enabling generating dwarf debug 
info too. In any case, I don't think this aspect needs to be matched 
closely (unless dwarf does the same), as any existing users of PDB 
generation do use "-g -gcodeview", so as long as that case works, there 
shouldn't be any interop issues.



---
gcc/common.opt  | 4 
gcc/doc/invoke.texi | 7 +++
gcc/gcc.cc  | 4 
gcc/opts.cc | 3 +++
4 files changed, 18 insertions(+)

@@ -4608,6 +4608,10 @@ driver_handle_option (struct gcc_options *opts,
  do_save = false;
  break;

+case OPT_gcodeview:
+  add_infile ("-pdb=", "*");
+  break;


Hmm, what does this end up passing to the linker in the end - does it just 
pass "-pdb="? (What does the "*" parameter do here?) If that's the case - 
that sounds reasonable - assuming that if a user passes an extra 
-Wl,--pdb,myspecificname.pdb, that would take precedence (i.e. be passed 
after the compiler's default one).


// Martin



Re: [PATCH] [12/11/10] Fix invalid format warnings on Windows

2022-05-12 Thread Martin Storsjö

On Wed, 11 May 2022, Joseph Myers wrote:


I'd also like to check that "if mingw-w64 is configured to target UCRT" is
not something that is necessarily known when GCC is built or from the
command-line options passed to GCC.  Because ideally one might expect the
TARGET_OVERRIDES_FORMAT_ATTRIBUTES / TARGET_OVERRIDES_FORMAT_INIT
definitions to handle things appropriately conditionally, so that printf
attributes are handled as gnu_printf for the "if mingw-w64 is configured
to target UCRT" case.  Disregarding a built-in format attribute when one
is also specified explicitly in the header, even though the two are not
exactly equivalent attributes, as in this patch, seems more like the right
approach in the case where the attributes in installed header (at the time
GCC is run, not the time it is built) *are* the way in which GCC gets the
"configured to target UCRT" information - as opposed to it being something
available before the header is parsed.


Indeed, the "configured to target X" information isn't available at the 
point when GCC is built - that gets set afterwards. And even while it is 
usually mostly fixed soon afterwards, you can even (with some amount of 
gotchas) change what CRT to build for by passing the appropriate defines 
that picks a different default in the mingw-w64 headers.


Anyway - from the GCC point of view, it's not fixed, and whatever the 
parsed headers say, is the only thing that can be relied upon. So I think 
the approach of the patch is the right one, code style/issues 
notwithstanding.


// Martin



[PATCH] mh-mingw: Set __USE_MINGW_ACCESS in missed C++ flags variables

2021-04-09 Thread Martin Storsjö
This is similar to what was done in
eea4e2ff0a3f5e7f37df204c070cc5d9ef339e6e (where it was added to
STAGE*_CXXFLAGS), but this adds the flag to the CXXFLAGS and
BOOT_CXXFLAGS variables too (as it's already added to CFLAGS and
BOOT_CFLAGS).

2021-04-09  Martin Storsjö  

config/ChangeLog:
   * mh-mingw: Set __USE_MINGW_ACCESS in missed C++ flags
   variables
---
 config/mh-mingw | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/config/mh-mingw b/config/mh-mingw
index a795096f038..e91367a7112 100644
--- a/config/mh-mingw
+++ b/config/mh-mingw
@@ -1,7 +1,9 @@
 # Add -D__USE_MINGW_ACCESS to enable the built compiler to work on Windows
 # Vista (see PR33281 for details).
 BOOT_CFLAGS += -D__USE_MINGW_ACCESS -Wno-pedantic-ms-format
+BOOT_CXXFLAGS += -D__USE_MINGW_ACCESS -Wno-pedantic-ms-format
 CFLAGS += -D__USE_MINGW_ACCESS
+CXXFLAGS += -D__USE_MINGW_ACCESS
 STAGE1_CXXFLAGS += -D__USE_MINGW_ACCESS
 STAGE2_CXXFLAGS += -D__USE_MINGW_ACCESS
 STAGE3_CXXFLAGS += -D__USE_MINGW_ACCESS
-- 
2.25.1



Re: Patch for 96948

2020-11-06 Thread Martin Storsjö

On Fri, 6 Nov 2020, Jeff Law wrote:



On 9/8/20 9:34 AM, Martin Storsjö wrote:

Hi,

On Tue, 8 Sep 2020, Kirill Müller wrote:


Thanks for the heads up. The coincidence is funny -- a file that
hasn't been touched for years.


I think we both may originally be triggered from the same guy asking
around in different places about implementations of _Unwind_Backtrace
for windows, actually.


I do believe that we need the logic around the `first` flag for
consistency with the other unwind-*.c implementations.


Yes, if you store ms_context.Rip/Rsp before the RtlVirtualUnwind step
- but my patch stores them afterwards; after RtlVirtualUnwind, before
calling the callback.

The result should be the same, except if using the first flag
approach, I believe you're missing the last frame that is printed if
using my patch.


Presumably with your patch installed, the patch from Kirill is
unnecessary, right?


Indeed, as far as I know, this issue should be fixed now (but I'd 
appreciate if Kirill can retest things as well). Thanks for your time!


// Martin


Re: Fwd: libstdc++: Attempt to resolve PR83562

2020-11-06 Thread Martin Storsjö

On Fri, 6 Nov 2020, Liu Hao via Gcc-patches wrote:


在 2020/10/29 下午3:56, Liu Hao 写道:

I forward it here for comments.

Basing on the behavior of both GCC and Clang, `__cxa_thread_atexit` is used to 
register the
destructor of thread_local objects directly, suggesting the first parameter 
should have `__thiscall`
convention.

libstdc++ used the default `__cdecl` convention and caused crashes on 
1686-w64-mingw32 (see
PR83562). But to my surprise, libcxxabi uses `__cdecl` too [1], but I haven't 
heard any of relevant
reports so far.

Original patch is attached in case you can't find it in gcc-patches.



FWIW, this patch looks good and correct to me, from a mingw perspective.

// Martin


Re: Patch for 96948

2020-09-08 Thread Martin Storsjö

Hi,

On Tue, 8 Sep 2020, Kirill Müller wrote:


I haven't actually tested if the cfa value (ms_context.Rsp) is valid, I also
see no reason why it shouldn't be.

What does it take for your patch to be accepted? What's the minimum gcc
version where it will be available?


I'm not a maintainer nor a committer here, so it takes someone with such a 
role/privileges to review and commit it, and presumably it'd be part of 
the upcoming GCC 11 then, unless someone chooses to backport it to current 
release branches.


// Martin


Re: Patch for 96948

2020-09-08 Thread Martin Storsjö

Hi,

On Tue, 8 Sep 2020, Kirill Müller wrote:

Thanks for the heads up. The coincidence is funny -- a file 
that hasn't been touched for years.


I think we both may originally be triggered from the same guy asking 
around in different places about implementations of _Unwind_Backtrace for 
windows, actually.


I do believe that we need the logic around the `first` flag 
for consistency with the other unwind-*.c implementations.


Yes, if you store ms_context.Rip/Rsp before the RtlVirtualUnwind step - 
but my patch stores them afterwards; after RtlVirtualUnwind, before 
calling the callback.


The result should be the same, except if using the first flag approach, I 
believe you're missing the last frame that is printed if using my patch.


// Martin


[PATCH v2] libgcc: Expose the instruction pointer and stack pointer in SEH _Unwind_Backtrace

2020-09-08 Thread Martin Storsjö
Previously, the SEH version of _Unwind_Backtrace did unwind
the stack and call the provided callback function as intended,
but there was little the caller could do within the callback to
actually get any info about that particular level in the unwind.

Set the ra and cfa pointers, which are used by _Unwind_GetIP
and _Unwind_GetCFA, to allow using these functions from the
callacb to inspect the state at each stack frame.

2020-09-08  Martin Storsjö  

libgcc/Changelog:
* unwind-seh.c (_Unwind_Backtrace): Set the ra and cfa pointers
before calling the callback.
---
 libgcc/unwind-seh.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/libgcc/unwind-seh.c b/libgcc/unwind-seh.c
index 1a70180cfaa..275d782903a 100644
--- a/libgcc/unwind-seh.c
+++ b/libgcc/unwind-seh.c
@@ -466,6 +466,11 @@ _Unwind_Backtrace(_Unwind_Trace_Fn trace,
_context.disp->HandlerData,
_context.disp->EstablisherFrame, NULL);
 
+  /* Set values that the callback can inspect via _Unwind_GetIP
+   * and _Unwind_GetCFA. */
+  gcc_context.ra = ms_context.Rip;
+  gcc_context.cfa = ms_context.Rsp;
+
   /* Call trace function.  */
   if (trace (_context, trace_argument) != _URC_NO_REASON)
return _URC_FATAL_PHASE1_ERROR;
-- 
2.17.1



Re: Patch for 96948

2020-09-08 Thread Martin Storsjö

Hi,

On Mon, 7 Sep 2020, Kirill Müller via Gcc-patches wrote:

As requested, attaching a patch for 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96948. This solves a problem 
with _Unwind_Backtrace() on mingw64 + SEH.


What a coincidence - I actually sent a patch for the exact same thing 
last week, see 
https://gcc.gnu.org/pipermail/gcc-patches/2020-September/553082.html.


My version doesn't set gcc_context.cfa though, but is simpler by avoiding 
the whole "first" flag logic.


I can send an updated patch that also sets gcc_context.cfa in a smilar 
manner to the previous one.


// Martin


Re: [PATCH] gcc: Make strchr return value pointers const

2020-09-08 Thread Martin Storsjö

On Tue, 8 Sep 2020, Jakub Jelinek wrote:


On Tue, Sep 08, 2020 at 12:16:08PM +0100, Richard Sandiford wrote:

Are platform maintainers allowed to push general changes like these? If
so I can push soon.


Yeah, anyone with commit access can push an approved patch.


I've pushed this one yesterday already:
https://gcc.gnu.org/g:3fe3efe5c141a88a80c1ecc6aebc7f15d6426f62


Thanks!

// Martin



Re: [PATCH] libgcc: Expose the current instruction pointer in SEH _Unwind_Backtrace

2020-09-04 Thread Martin Storsjö

On Tue, 1 Sep 2020, Martin Storsjö wrote:


Previously, the SEH version of _Unwind_Backtrace did unwind
the stack and call the provided callback function as intended,
but there was little the caller could do within the callback to
actually get any info about that particular level in the unwind.

Set the ra pointer, which is used by _Unwind_GetIP, to allow
using this function to inspect the state within the callback,
before calling the callback function.

2020-09-01  Martin Storsjö  

libgcc/Changelog:
   * unwind-seh.c (_Unwind_Backtrace): Set the ra pointer before
   calling the callback.
---
libgcc/unwind-seh.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/libgcc/unwind-seh.c b/libgcc/unwind-seh.c
index 1a70180cfaa..72473135862 100644
--- a/libgcc/unwind-seh.c
+++ b/libgcc/unwind-seh.c
@@ -466,6 +466,8 @@ _Unwind_Backtrace(_Unwind_Trace_Fn trace,
_context.disp->HandlerData,
_context.disp->EstablisherFrame, NULL);

+  gcc_context.ra = ms_context.Rip;
+
  /* Call trace function.  */
  if (trace (_context, trace_argument) != _URC_NO_REASON)
return _URC_FATAL_PHASE1_ERROR;


Ping - any comments on this one?

// Martin


Re: [PATCH] gcc: Make strchr return value pointers const

2020-09-04 Thread Martin Storsjö

Hi,

On Fri, 4 Sep 2020, Jakub Jelinek wrote:


On Tue, Sep 01, 2020 at 04:01:42PM +0300, Martin Storsjö wrote:

This fixes compilation of codepaths for dos-like filesystems
with Clang. When built with clang, it treats C input files as C++
when the compiler driver is invoked in C++ mode, triggering errors
when the return value of strchr() on a pointer to const is assigned
to a pointer to non-const variable.


Not really specific to clang, e.g. glibc does that in its headers too
as the C++ standard mandates that (and I guess mingw should do that too).


This matches similar variables outside of the ifdefs for dos-like
path handling.

2020-09-01  Martin Storsjö  

gcc/Changelog:
* dwarf2out.c (file_name_acquire): Make a strchr return value
pointer to const.

libcpp/Changelog:
* files.c (remap_filename): Make a strchr return value pointer
to const.


LGTM.  And it is short enough not to need copyright assignment, so ok for
trunk.


Thanks! Can someone commit this for me?

// Martin


[PATCH] libgcc: Expose the current instruction pointer in SEH _Unwind_Backtrace

2020-09-01 Thread Martin Storsjö
Previously, the SEH version of _Unwind_Backtrace did unwind
the stack and call the provided callback function as intended,
but there was little the caller could do within the callback to
actually get any info about that particular level in the unwind.

Set the ra pointer, which is used by _Unwind_GetIP, to allow
using this function to inspect the state within the callback,
before calling the callback function.

2020-09-01  Martin Storsjö  

libgcc/Changelog:
* unwind-seh.c (_Unwind_Backtrace): Set the ra pointer before
calling the callback.
---
 libgcc/unwind-seh.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/libgcc/unwind-seh.c b/libgcc/unwind-seh.c
index 1a70180cfaa..72473135862 100644
--- a/libgcc/unwind-seh.c
+++ b/libgcc/unwind-seh.c
@@ -466,6 +466,8 @@ _Unwind_Backtrace(_Unwind_Trace_Fn trace,
_context.disp->HandlerData,
_context.disp->EstablisherFrame, NULL);
 
+  gcc_context.ra = ms_context.Rip;
+
   /* Call trace function.  */
   if (trace (_context, trace_argument) != _URC_NO_REASON)
return _URC_FATAL_PHASE1_ERROR;
-- 
2.17.1



[PATCH] gcc: Make strchr return value pointers const

2020-09-01 Thread Martin Storsjö
This fixes compilation of codepaths for dos-like filesystems
with Clang. When built with clang, it treats C input files as C++
when the compiler driver is invoked in C++ mode, triggering errors
when the return value of strchr() on a pointer to const is assigned
to a pointer to non-const variable.

This matches similar variables outside of the ifdefs for dos-like
path handling.

2020-09-01  Martin Storsjö  

gcc/Changelog:
* dwarf2out.c (file_name_acquire): Make a strchr return value
pointer to const.

libcpp/Changelog:
* files.c (remap_filename): Make a strchr return value pointer
to const.
---
 gcc/dwarf2out.c | 2 +-
 libcpp/files.c  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index b6ab49bb548..4096c0c0d69 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -12118,7 +12118,7 @@ file_name_acquire (dwarf_file_data **slot, 
file_name_acquire_data *fnad)
   f = strrchr (f, DIR_SEPARATOR);
 #if defined (DIR_SEPARATOR_2)
   {
-char *g = strrchr (fi->path, DIR_SEPARATOR_2);
+const char *g = strrchr (fi->path, DIR_SEPARATOR_2);
 
 if (g != NULL)
   {
diff --git a/libcpp/files.c b/libcpp/files.c
index 3d48c38fc0a..b890b8ebf1e 100644
--- a/libcpp/files.c
+++ b/libcpp/files.c
@@ -1693,7 +1693,7 @@ remap_filename (cpp_reader *pfile, _cpp_file *file)
   p = strchr (fname, '/');
 #ifdef HAVE_DOS_BASED_FILE_SYSTEM
   {
-   char *p2 = strchr (fname, '\\');
+   const char *p2 = strchr (fname, '\\');
if (!p || (p > p2))
  p = p2;
   }
-- 
2.17.1