[PATCH] D134637: clang-tblgen build: avoid duplicate inclusion of libLLVMSupport

mstorsjo added a comment. Nice, +1 this looks reasonable to me. I've run into similar issues occasionally with mingw builds with dylib enabled too, where (depending on how the linker ends up pulling in objects) you can end up with duplicate definitions etc. I would expect that this also fixes t

[PATCH] D134637: clang-tblgen build: avoid duplicate inclusion of libLLVMSupport

mstorsjo added a reviewer: beanz. mstorsjo added a subscriber: beanz. mstorsjo added a comment. Adding @beanz who might have some more cmake knowledge on whether this is the best/least bad way of doing things. Comment at: clang/lib/Support/CMakeLists.txt:26 +DISABLE_LLVM_L

[PATCH] D135027: [Clang][MinGW][cygwin] Fix __declspec with -fdeclspec enabled

This revision was automatically updated to reflect the committed changes. Closed by commit rGd90633a74bef: [Clang][MinGW][cygwin] Fix __declspec with -fdeclspec enabled (authored by alvinhochun, committed by mstorsjo). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://rev

[PATCH] D133457: Add Clang driver flags equivalent to cl's /MD, /MT, /MDd, /MTd.

mstorsjo added a comment. @akhuang I noticed that https://github.com/llvm/llvm-project/blob/main/clang/lib/Driver/ToolChains/MSVC.cpp#L199-L200 has got an explicit check for `Args.hasArg(options::OPT__SLASH_MD, options::OPT__SLASH_MDd)` - I think that this would need to be amended to handle th

[PATCH] D135110: [NFC] [HLSL] Move common metadata to LLVMFrontend

mstorsjo added a comment. I don't quite know why, but it seems like this new library breaks tests of llvm-config; if I start out with an empty build directory and run `ninja check-llvm` (or `ninja llvm-test-depends`), then this library doesn't get built, and llvm-config prints `llvm-config: err

[PATCH] D141206: [clang] [MinGW] Avoid adding /include and /lib when cross compiling

mstorsjo created this revision. mstorsjo added reviewers: mati865, alvinhochun. Herald added a subscriber: pengfei. Herald added a project: All. mstorsjo requested review of this revision. Herald added a subscriber: MaskRay. Herald added a project: clang. The MinGW compiler driver first tries to d

[PATCH] D141206: [clang] [MinGW] Avoid adding /include and /lib when cross compiling

mstorsjo added a comment. Also, this patch is lacking tests for now - mostly bringing it up for discussion first - I can add tests if others agree that it seems like a reasonable path forward. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141206/n

[PATCH] D141206: [clang] [MinGW] Avoid adding /include and /lib when cross compiling

mstorsjo added a comment. In D141206#4034159 , @mati865 wrote: > I had thought we do that already so this change looks reasonable for me. > Just one thought, do we support multilib toolchains? I think in that case > Clang would have to add `/include` des

[PATCH] D141206: [clang] [MinGW] Avoid adding /include and /lib when cross compiling

mstorsjo added a comment. In D141206#4034451 , @mstorsjo wrote: > In D141206#4034159 , @mati865 wrote: > >> I had thought we do that already so this change looks reasonable for me. >> Just one thought, do we suppo

[PATCH] D141206: [clang] [MinGW] Avoid adding /include and /lib when cross compiling

mstorsjo added a comment. In D141206#4037160 , @alvinhochun wrote: > The idea sounds reasonable. I don't know mingw-w64 toolchains well enough, > but I'll try: > > How does it interact with the following conditions (lines 469-475)? They look > like the

[PATCH] D141206: [clang] [MinGW] Avoid adding /include and /lib when cross compiling

mstorsjo updated this revision to Diff 488182. mstorsjo added a comment. Updated with some testcases. This does test that the include directory is omitted when cross compiling, but those kinds of tests, which set up a simulated toolchain environment with symlinks, don't run on actual Windows - s

[PATCH] D137073: [clang] Fix inline builtin functions of an __asm__ renamed function with symbol prefixes

This revision was automatically updated to reflect the committed changes. Closed by commit rG9b3834ef67e3: [clang] Fix inline builtin functions of an __asm__ renamed function with symbol… (authored by mstorsjo). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.ll

[PATCH] D137024: [compiler-rt] Switch from llvm-config to find_package(LLVM)

mstorsjo added a comment. In D137024#3901342 , @mgorny wrote: > Thanks! Let's hope it works this time. FYI, this broke builds of mine (https://github.com/mstorsjo/llvm-mingw/actions/runs/3382275510/jobs/5617040099), but I just need to change/add a work

[PATCH] D137024: [compiler-rt] Switch from llvm-config to find_package(LLVM)

mstorsjo added a comment. In D137024#3905181 , @mgorny wrote: >> These workarounds are fine for me, but I wonder if it would be useful with a >> more direct cmake option to tell it not look for these files at all. > > CMake has something like that built-

[PATCH] D137107: Allow MS extension: support of constexpr with __declspec(dllimport).

mstorsjo added a comment. In D137107#3905443 , @zahiraam wrote: > and the same assembly (load __imp_?val) (is that why you made the comment > above?). And the test case runs. Yes, pretty much - the actual value of the address of `&val` isn't known at c

[PATCH] D137107: Allow MS extension: support of constexpr with __declspec(dllimport).

mstorsjo added a comment. In D137107#3906439 , @zahiraam wrote: > Here is an even smaller test case. > lib.cpp: > __declspec(dllexport) int val=12; > > t1.cpp: > int main () { > > extern int _declspec(dllimport) val; > int& val_ref = val; > return v

[PATCH] D137669: clang/cmake: Require pre-built test dependencies for stand-alone builds

mstorsjo added subscribers: smeenai, mstorsjo. mstorsjo added a comment. This does, somewhat, coincide with what I'm trying to do in D131052 . There, I don't point out the binaries for `FileCheck` and similar, but point out preexisting native `llvm-tblgen`, `cl

[PATCH] D137669: clang/cmake: Require pre-built test dependencies for stand-alone builds

mstorsjo added a comment. Also FYI, OpenMP has a bit of prior art in this area - see e.g. https://github.com/llvm/llvm-project/blob/main/openmp/cmake/OpenMPTesting.cmake#L27-L36. There it prints a warning at cmake time, disabling tests, saying why, and giving hints about how to fix it. Reposi

[PATCH] D137669: clang/cmake: Require pre-built test dependencies for stand-alone builds

mstorsjo added a comment. In D137669#3920309 , @tstellar wrote: > In D137669#3915899 , @mstorsjo > wrote: > >> This does, somewhat, coincide with what I'm trying to do in D131052 >>

[PATCH] D137470: [Offloading] Initial support for registering offloading entries on COFF targets

mstorsjo added a comment. This looks reasonable to me, overall. I didn't quite try to follow all the wall-of-text changes in the tests though, but overall fine. Just a couple comments and one case where I didn't find where code went in the refactoring. Comment at: clang/test/

[PATCH] D137470: [Offloading] Initial support for registering offloading entries on COFF targets

mstorsjo added a comment. In D137470#3928731 , @jhuber6 wrote: > Another significant portion of getting this workflow to work for Windows / > COFF is parsing the linker arguments. I should be able to look at `lld-link` > and add necessarily aliases to w

[PATCH] D137024: [compiler-rt] Switch from llvm-config to find_package(LLVM)

mstorsjo added a comment. In D137024#3931488 , @mgorny wrote: > Well, I'm certainly not opposed to making all the paths configurable. > However, I'm not sure if having CMake file accessible one way or another > wouldn't eventually be a necessity. For on

[PATCH] D137698: [include-cleaner] Add self-contained file support for PragmaIncludes.

mstorsjo added inline comments. Comment at: clang-tools-extra/include-cleaner/lib/CMakeLists.txt:12 LINK_LIBS clangAST Note that this file changed a bit further in 68294afa0836bb62be921e2143d147cdfdc8ba70, so you may want to rebase again. (I tested this

[PATCH] D142297: [Clang][OpenMP] Find the type `omp_allocator_handle_t` from identifier table

mstorsjo added a comment. This change broke the `parallel/bug54082.c` testcase in the OpenMP runtime test set, when running on Windows (both mingw and MSVC configurations, and happening both in i386 and x86_64 builds), see e.g. https://github.com/mstorsjo/llvm-mingw/actions/runs/4011290068/jobs

[PATCH] D141206: [clang] [MinGW] Avoid adding /include and /lib when cross compiling

mstorsjo added inline comments. Comment at: clang/test/Driver/mingw-sysroot.cpp:25-38 // RUN: env "PATH=%T/testroot-gcc/bin:%PATH%" %clang -target x86_64-w64-mingw32 -rtlib=platform -stdlib=libstdc++ --sysroot="" -c -### %s 2>&1 | FileCheck -check-prefix=CHECK_TESTROOT_GCC %s

[PATCH] D141206: [clang] [MinGW] Avoid adding /include and /lib when cross compiling

mstorsjo added inline comments. Comment at: clang/test/Driver/mingw-sysroot.cpp:25-38 // RUN: env "PATH=%T/testroot-gcc/bin:%PATH%" %clang -target x86_64-w64-mingw32 -rtlib=platform -stdlib=libstdc++ --sysroot="" -c -### %s 2>&1 | FileCheck -check-prefix=CHECK_TESTROOT_GCC %s

[PATCH] D141206: [clang] [MinGW] Avoid adding /include and /lib when cross compiling

mstorsjo updated this revision to Diff 488752. mstorsjo added a comment. Switch the negative test to `--implicit-check-not`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141206/new/ https://reviews.llvm.org/D141206 Files: clang/lib/Driver/ToolC

[PATCH] D131052: [CMake] Allow setting the location of host tools with LLVM_NATIVE_TOOL_DIR

mstorsjo updated this revision to Diff 488982. mstorsjo added a comment. Herald added subscribers: Moerafaat, zero9178, bzcheeseman, sdasgup3, wenzhicui, wrengr, cota, teijeong, rdzhabarov, tatianashp, msifontes, jurahul, Kayjukh, grosul1, Joonsoo, stephenneuendorffer, liufengdb, aartbik, mgester

[PATCH] D131052: [CMake] Allow setting the location of host tools with LLVM_NATIVE_TOOL_DIR

mstorsjo added inline comments. Comment at: llvm/cmake/modules/AddLLVM.cmake:2401 + +macro(setup_host_tool tool_name setting_name exe_var_name target_var_name) + cmake_parse_arguments(ARG "" "SCOPE" "" ${ARGN}) beanz wrote: > Please make this a `function` instea

[PATCH] D131052: [CMake] Allow setting the location of host tools with LLVM_NATIVE_TOOL_DIR

mstorsjo added a comment. In D131052#4051948 , @beanz wrote: > The convention that `find_program` uses is to cache the variables, which > causes them to be defined at global scope. Right, I guess that could work too. It would be less of a pure refactori

[PATCH] D131052: [CMake] Allow setting the location of host tools with LLVM_NATIVE_TOOL_DIR

mstorsjo updated this revision to Diff 489098. mstorsjo added a comment. Switched the macro to a function and changed the variables to cache variables, to allow setting them in the grandparent scope without being a macro. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https:

[PATCH] D133574: [C2x] reject type definitions in offsetof

mstorsjo added a comment. +1 on this being disruptive. Also, not only GCC, but also MSVC seem to handle the existing constructs in the wild. I ran into this at https://github.com/FFmpeg/FFmpeg/blob/n5.1.2/libavutil/video_enc_params.c#L30-L34 (where it probably would be trivial to fix, but leavi

[PATCH] D141206: [clang] [MinGW] Avoid adding /include and /lib when cross compiling

This revision was automatically updated to reflect the committed changes. Closed by commit rGfd15cb935d7a: [clang] [MinGW] Avoid adding /include and /lib when cross compiling (authored by mstorsjo). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.ll

[PATCH] D133574: [C2x] reject type definitions in offsetof

mstorsjo added a subscriber: aeubanks. mstorsjo added a comment. For clarity in the review - the relanded commit ended up reverted by @aeubanks in 39da55e8f548a11f7dadefa73ea73d809a5f1729 . The relanded commit triggers failed

[PATCH] D131052: [CMake] Allow setting the location of host tools with LLVM_NATIVE_TOOL_DIR

mstorsjo added a comment. In D131052#4052563 , @mstorsjo wrote: > Switched the macro to a function and changed the variables to cache > variables, to allow setting them in the grandparent scope without being a > macro. @beanz - Does this seems ok to yo

[PATCH] D131052: [CMake] Allow setting the location of host tools with LLVM_NATIVE_TOOL_DIR

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGd3da9067d143: [CMake] Allow setting the location of host tools with LLVM_NATIVE_TOOL_DIR (authored by mstorsjo). Repository: rG LLVM Github Monore

[PATCH] D136651: [Clang] Give Clang the ability to use a shared stat cache

mstorsjo added a comment. In D136651#4064474 , @glandium wrote: > In D136651#4064260 , @glandium > wrote: > >> This broke our mac builds with errors like: >> >> ld64.lld: error: undefined symbol: CFRunLoopRun >

[PATCH] D136651: [Clang] Give Clang the ability to use a shared stat cache

mstorsjo added a comment. In D136651#4066949 , @friss wrote: > Thanks for the report, unfortunately I have no way of testing this setup. Can you try a build with `-DLLVM_LINK_LLVM_DYLIB=ON`? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACT

[PATCH] D142346: [docs] Add release notes for news in 16.x done by me, or otherwise relating to MinGW targets

mstorsjo created this revision. mstorsjo added reviewers: efriedma, aaron.ballman, alvinhochun, mati865. Herald added a reviewer: alexander-shaposhnikov. Herald added a reviewer: MaskRay. Herald added a project: All. mstorsjo requested review of this revision. Herald added a reviewer: jdoerfert. He

[PATCH] D142346: [docs] Add release notes for news in 16.x done by me, or otherwise relating to MinGW targets

mstorsjo added inline comments. Comment at: lld/docs/ReleaseNotes.rst:78 + if ``-static`` has been specified. This fixes conformance to what + GNU ld dpes. (`D135651 `_) + MaskRay wrote: > `s/dpes/does/` Thanks! I'll fix this b

[PATCH] D142346: [docs] Add release notes for news in 16.x done by me, or otherwise relating to MinGW targets

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGc3737a652230: [docs] Add release notes for news in 16.x done by me, or otherwise relating to… (authored by mstorsjo). Changed prior to commit: htt

[PATCH] D141968: [NFC] Consolidate llvm::CodeGenOpt::Level handling

mstorsjo added inline comments. Comment at: llvm/include/llvm/Support/CodeGen.h:67 + inline std::optional getLevel(IDType ID) { +if (ID < 0 || ID > 3) + return std::nullopt; scott.linder wrote: > barannikov88 wrote: > > As I can see, clients do not chec

[PATCH] D126291: [flang][Driver] Update link job on windows

mstorsjo added a comment. In D126291#3573137 , @mmuetzel wrote: > Not sure if this is the right place to add this. But maybe something like > this could be used to add 'windows-msvc' and 'windows-gnu' features that > could be used to run tests condition

[PATCH] D126291: [flang][Driver] Update link job on windows

mstorsjo added a comment. In D126291#3573742 , @mmuetzel wrote: > In D126291#3573607 , @mstorsjo > wrote: > >> FWIW, something very similar was added just a couple days ago in an lldb >> specific lit.cfg.py: htt

[PATCH] D126291: [flang][Driver] Update link job on windows

mstorsjo added a comment. In D126291#3573937 , @mmuetzel wrote: > Ah ok. So instead of having `! REQUIRES:` lines, those tests should add > `-target x86_64-windows-msvc` to the flang invocation? Exactly > On platforms that didn't build the necessary li

[PATCH] D126291: [flang][Driver] Update link job on windows

mstorsjo added a comment. In D126291#3577133 , @rovka wrote: > I had the same idea about switching the tests to using target triples instead > of having separate files for it, but initially I had some issues getting that > to work properly. When specify

[PATCH] D127528: [Clang] Let the linker choose shared or static libunwind unless specified

mstorsjo added a reviewer: mstorsjo. mstorsjo added a comment. LGTM in general, but I'd leave it to the others to formally approve. Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:1477 if (Args.hasArg(options::OPT_shared_libgcc)) return LibGccType::SharedLibGcc;

[PATCH] D115674: [CMake][compiler-rt] Provide a dedicated option for LLVM unwinder

mstorsjo accepted this revision. mstorsjo added a comment. The changes left in this patch seem fine to me now! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115674/new/ https://reviews.llvm.org/D115674 _

[PATCH] D126676: [clang] Disallow differences in defines used for creating and using PCH

mstorsjo added a comment. In D126676#3587243 , @rnk wrote: > I don't have a great answer here, but yes, dllexport macro norms sort of run > directly counter to the normal ways that people use PCH. It seems like > projects will need to have a library mod

[PATCH] D126291: [flang][Driver] Update link job on windows

mstorsjo added a comment. Looks good to me in general, one general question though. Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:773 + // TODO: Make this work unconditionally once Flang is mature enough. + if (!Args.hasArg(options::OPT_flang_experimental_exec)) +

[PATCH] D126291: [flang][Driver] Update link job on windows

mstorsjo added inline comments. Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:773 + // TODO: Make this work unconditionally once Flang is mature enough. + if (!Args.hasArg(options::OPT_flang_experimental_exec)) +return; rovka wrote: > mstorsjo wrot

[PATCH] D128036: [CMake] Option to select C++ library for runtimes that use it

mstorsjo added inline comments. Comment at: compiler-rt/CMakeLists.txt:534 +if (COMPILER_RT_CXX_LIBRARY STREQUAL "libcxx") + # We are using the in-tree libc++ so avoid including the default one. + append_list_if(COMPILER_RT_HAS_NOSTDINCXX_FLAG -nostdinc++ COMPILER_RT_COMMON_CF

[PATCH] D128036: [CMake] Option to select C++ library for runtimes that use it

mstorsjo accepted this revision. mstorsjo added a comment. This revision is now accepted and ready to land. LGTM Comment at: compiler-rt/CMakeLists.txt:534 +if (COMPILER_RT_CXX_LIBRARY STREQUAL "libcxx") + # We are using the in-tree libc++ so avoid including the default one. +

[PATCH] D126397: [pseudo] Fix pseudo-gen usage when cross-compiling

mstorsjo added a comment. Regarding building native tools while cross compiling - for other tools so far, we’ve also had the option to pass an option like `-DLLVM_TABLEGEN=path/to/llvm-tblgen`. If all the needed native tools are provided that way, the build doesn’t need to set up the nested nat

[PATCH] D126676: [clang] Disallow differences in defines used for creating and using PCH

mstorsjo created this revision. mstorsjo added reviewers: aaron.ballman, rnk. Herald added a project: All. mstorsjo requested review of this revision. Herald added a project: clang. Some amount of differences between defines when creating and using a PCH were intentionally allowed in c379c072405f3

[PATCH] D126717: [pseudo] Add PSEUDO_GEN cmake cache variable to avoid nested CMake invocation

mstorsjo accepted this revision. mstorsjo added a comment. This revision is now accepted and ready to land. Looks reasonable to me. The variable name feels a bit un-namespaced though, but that's what the tool is called currently. Should we rename the tool and the variable to give it a bit more o

[PATCH] D126764: [clang] [ARM] Add __builtin_sponentry like on aarch64

mstorsjo created this revision. mstorsjo added reviewers: efriedma, zzheng. Herald added a subscriber: kristof.beyls. Herald added a project: All. mstorsjo requested review of this revision. Herald added a project: clang. This is used for calling the SEH aware setjmp on MinGW. Repository: rG L

[PATCH] D126764: [clang] [ARM] Add __builtin_sponentry like on aarch64

This revision was automatically updated to reflect the committed changes. Closed by commit rGf730749e8584: [clang] [ARM] Add __builtin_sponentry like on aarch64 (authored by mstorsjo). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126764/new/ https:

[PATCH] D126862: [clang] [MSVC] Enable unwind tables for ARM

mstorsjo created this revision. mstorsjo added reviewers: efriedma, zzheng. Herald added a subscriber: kristof.beyls. Herald added a project: All. mstorsjo requested review of this revision. Herald added a subscriber: MaskRay. Herald added a project: clang. The backend now can generate working unw

[PATCH] D126865: [clang] [Headers] Check __SEH__, when checking if ARM EHABI is implied

mstorsjo created this revision. mstorsjo added reviewers: MaskRay, efriedma, zzheng. Herald added subscribers: StephenFan, kristof.beyls. Herald added a project: All. mstorsjo requested review of this revision. Herald added a project: clang. ARM EHABI isn't signalled by any specific compiler built

[PATCH] D126871: [clang] [MinGW] Use SEH for unwind info on ARM by default

mstorsjo created this revision. mstorsjo added reviewers: efriedma, zzheng. Herald added a subscriber: kristof.beyls. Herald added a project: All. mstorsjo requested review of this revision. Herald added a subscriber: MaskRay. Herald added a project: clang. This goes hand in hand with D126870

[PATCH] D126862: [clang] [MSVC] Enable unwind tables for ARM

mstorsjo updated this revision to Diff 433836. mstorsjo added a comment. Added testing, including testing for the existing architectures. There was a good match for testing for this in windows-exceptions.cpp - unfortunately it ends up with 2 more RUN lines, but it fits in so well there so I'd r

[PATCH] D126862: [clang] [MSVC] Enable unwind tables for ARM

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGe8402d5de82a: [clang] [MSVC] Enable unwind tables for ARM (authored by mstorsjo). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D126291: [flang][Driver] Update link job on windows

mstorsjo added a comment. In D126291#3555805 , @awarzynski wrote: > Like @rovka pointed out, skipping `CLANG_DEFAULT_RTLIB` should solve your > issue with missing libs Skipping that woulnd't help/work in the clang64 environment in msys2; there's no li

[PATCH] D126291: [flang][Driver] Update link job on windows

mstorsjo added a comment. In D126291#3556096 , @mmuetzel wrote: > The error message I get without `-lc++`: > > ld.lld: error: undefined symbol: std::__1::mutex::lock() > >>> referenced by > libFortranRuntime.a(io-api.cpp.obj):(Fortran::runtime::io::I

[PATCH] D126291: [flang][Driver] Update link job on windows

mstorsjo added a comment. In D126291#3556324 , @mmuetzel wrote: > With this additional change, I no longer need the `-lc++` flag: > > From 965343d8b05bf3cf7a9a3873ea4d2ddcc00a3703 Mon Sep 17 00:00:00 2001 > From: =?UTF-8?q?Markus=20M=C3=BCtzel?= >

[PATCH] D126865: [clang] [Headers] Check __SEH__, when checking if ARM EHABI is implied

mstorsjo added a comment. @MaskRay Thanks for the other reviews! This one is the last one with the same condition added. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126865/new/ https://reviews.llvm.org/D126865 __

[PATCH] D126871: [clang] [MinGW] Use SEH for unwind info on ARM by default

mstorsjo added a comment. In D126871#3560924 , @efriedma wrote: > Oh, just remembered, we should probably release-note this. (You can post > that as a followup.) Sure, can do. Btw, thanks a lot for your reviews, you've spent a lot of time on them! The

[PATCH] D126871: [clang] [MinGW] Use SEH for unwind info on ARM by default

mstorsjo added a comment. In D126871#3561243 , @efriedma wrote: >> But for MSVC style (C++ exceptions and __try/__except) there's still a >> couple backend things that need to be implemented. Do you happen to know >> roughly how much effort that is? > >

[PATCH] D126865: [clang] [Headers] Check __SEH__, when checking if ARM EHABI is implied

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG2293d46a0175: [clang] [Headers] Check __SEH__, when checking if ARM EHABI is implied (authored by mstorsjo). Repository: rG LLVM Github Monorepo

[PATCH] D126871: [clang] [MinGW] Use SEH for unwind info on ARM by default

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGfcb784db4961: [clang] [MinGW] Default to WinEH (SEH) exception handling instead of Dwarf (authored by mstorsjo). Changed prior to commit: https://

[PATCH] D127150: [doc] Add release notes about SEH unwind information on ARM

mstorsjo created this revision. mstorsjo added reviewers: efriedma, MaskRay. Herald added subscribers: StephenFan, kristof.beyls. Herald added a project: All. mstorsjo requested review of this revision. Herald added projects: clang, LLVM. Repository: rG LLVM Github Monorepo https://reviews.llvm

[PATCH] D127150: [doc] Add release notes about SEH unwind information on ARM

mstorsjo updated this revision to Diff 434611. mstorsjo added a comment. Capitalize DWARF. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127150/new/ https://reviews.llvm.org/D127150 Files: clang/docs/ReleaseNotes.rst llvm/docs/ReleaseNotes.rst

[PATCH] D126291: [flang][Driver] Update link job on windows

mstorsjo added a comment. In D126291#3562471 , @mmuetzel wrote: > Found why I thought building on Windows wasn't supported: > https://github.com/llvm/llvm-project/blob/main/flang/README.md?plain=1#L153 > >> The code does not compile with Windows and a com

[PATCH] D127150: [doc] Add release notes about SEH unwind information on ARM

This revision was automatically updated to reflect the committed changes. Closed by commit rG20ca739701d7: [doc] Add release notes about SEH unwind information on ARM (authored by mstorsjo). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127150/new/

[PATCH] D126291: [flang][Driver] Update link job on windows

mstorsjo added inline comments. Comment at: clang/lib/Driver/ToolChains/MinGW.cpp:224 +tools::addFortranRuntimeLibs(TC, CmdArgs); + } + mmuetzel wrote: > MinGW behaves similarly to GNU in many respects. The GNU toolchain file adds > `CmdArgs.push_back("-lm"

[PATCH] D155540: [clangd] Remove extra dependancies for clangd

mstorsjo added a reviewer: sammccall. mstorsjo added a subscriber: sammccall. mstorsjo added a comment. Thanks for looking at my suggestion! At this point, I'd defer the judgement to the code owner, @sammccall - whether it's nicer to have a tighter set of dependencies, or to reduce the risk of c

[PATCH] D61670: [RFC] [MinGW] Allow opting out from .refptr stubs

mstorsjo added subscribers: alvinhochun, jeremyd2019, jacek, aaron.ballman. mstorsjo added a comment. I'm looking to pick this up again - hopefully @rnk has time to discuss what would be a good way forward. So taking it from the top; both GCC and Clang generate `.refptr` stubs when referencing

[PATCH] D157332: [clang] Make init for empty no_unique_address fields a no-op write

mstorsjo updated this revision to Diff 549099. mstorsjo added a comment. Plumb the info from EmitInitializerForField through AggValueSlot and ReturnValueSlot to EmitCall. The fields/variables could use better names. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://rev

[PATCH] D157332: [clang] Make init for empty no_unique_address fields a no-op write

mstorsjo updated this revision to Diff 549517. mstorsjo added a comment. Updated to just check isEmptyRecord in EmitCall. The second part of the test probably is kinda redundant/pointless at this point, and I guess the test comment needs updating too; can do that later when the implementation i

[PATCH] D157332: [clang] Make init for empty no_unique_address fields a no-op write

mstorsjo updated this revision to Diff 549575. mstorsjo added a comment. Move the existing comment into the new if statement, add a comment to the new if. Add a comment to the second part of the testcase, which probably is good to keep anyway. Repository: rG LLVM Github Monorepo CHANGES SIN

[PATCH] D157332: [clang] Make init for empty no_unique_address fields a no-op write

mstorsjo marked an inline comment as done. mstorsjo added a comment. Thanks a lot for the guidance! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157332/new/ https://reviews.llvm.org/D157332 ___ cfe-comm

[PATCH] D150646: [clang][X86] Add __cpuidex function to cpuid.h

mstorsjo added a comment. In D150646#4581664 , @rnk wrote: > I think we should be exposing the `__cpudex` builtin any time we are being > MSVC-like, not under `fms-compatibility`. Would that make things sensible > again? I think that might sound reason

[PATCH] D157794: [Driver] Make /Zi and /Z7 aliases of -g rather than handling them specially

mstorsjo added inline comments. Comment at: clang/test/Driver/cl-options.c:567 -// This test was super sneaky: "/Z7" means "line-tables", but "-gdwarf" occurs -// later on the command line, so it should win. Interestingly the cc1 arguments -// came out right, but had wrong sema

[PATCH] D157332: [clang] Make init for empty no_unique_address fields a no-op write

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGd60c3d08e78d: [clang] Skip stores in init for fields that are empty structs (authored by mstorsjo). Changed prior to commit: https://reviews.llvm.

[PATCH] D157762: [WIP] Implement [[msvc::no_unique_address]]

mstorsjo added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:1409 + + On Windows targets, ``[[no_unique_address]]`` is ignored; use + ``[[msvc::no_unique_address]]`` instead. On MSVC targets, `[[no_unique_address]]` is ignored - it's not ig

[PATCH] D61670: [RFC] [MinGW] Allow opting out from .refptr stubs

mstorsjo updated this revision to Diff 551957. mstorsjo added a comment. Herald added a subscriber: MaskRay. Updated as discussed, adding a new option `-fno-autoimport` (and the corresponding positive one, `-fautoimport`, which is the implicit default), which affects code generation and linking.

[PATCH] D61670: [clang] [MinGW] Add the option -fno-autoimport

mstorsjo added inline comments. Comment at: clang/include/clang/Driver/Options.td:1477 + "automatic dllimport, and enable support for it in the linker. " + "Enabled by default.">>; +} // let Flags = [TargetSpecific] I see that most similar `Bool

[PATCH] D158411: [clang] [MinGW] Pass LTO options to the linker

mstorsjo created this revision. mstorsjo added a reviewer: MaskRay. Herald added subscribers: ormris, steven_wu, hiraditya, inglorion. Herald added a project: All. mstorsjo requested review of this revision. Herald added a project: clang. This matches what is done on other platforms too. This fix

[PATCH] D61670: [clang] [MinGW] Add the option -fno-autoimport

mstorsjo added a comment. In D61670#4604145 , @rnk wrote: > cc +@aeubanks @jyknight to consider using the code model for this purpose Hmm, I don't quite understand this comment; do you suggest that we after all should use the code model for controlling t

[PATCH] D61670: [clang] [MinGW] Add the option -fno-autoimport

mstorsjo added a comment. > I expected the answer would be "yes", so I said "lgtm" and then phrased my > question very awkwardly. Ah, thanks for the clarification! Any opinion on the name, `-fno-autoimport` vs `-fno-auto-import`, given the existing linker option `--disable-auto-import`? Repo

[PATCH] D157331: [clang] Implement C23

mstorsjo added a comment. This change broke building a recent version of gettext. Gettext uses gnulib for polyfilling various utility functions. Since not long ago, these functions internally use ``, https://git.savannah.gnu.org/cgit/gnulib.git/commit/lib/malloca.c?id=ef5a4088d9236a55283d1eb576

[PATCH] D157115: Revert "[clang][X86] Add __cpuidex function to cpuid.h"

mstorsjo added a comment. In D157115#4561497 , @aaron.ballman wrote: > Thank you for this! The revert LGTM, but please wait for @mstorsjo to confirm > this won't cause problems for MinGW folks. I guess this is fine. We should probably sync mingw-w64 a

[PATCH] D157332: [clang] Make init for empty no_unique_address fields a no-op write

mstorsjo created this revision. mstorsjo added reviewers: aaron.ballman, efriedma. Herald added subscribers: steven.zhang, pengfei. Herald added a project: All. mstorsjo requested review of this revision. Herald added a project: clang. An empty struct is handled as a struct with a dummy i8, on all

[PATCH] D157332: [clang] Make init for empty no_unique_address fields a no-op write

mstorsjo added a comment. In D157332#4568341 , @efriedma wrote: > I see what you're getting at here... but I don't think this works quite > right. If the empty class has a non-trivial constructor, we have to pass the > correct "this" address to that co

[PATCH] D157332: [clang] Make init for empty no_unique_address fields a no-op write

mstorsjo added a comment. In D157332#4570290 , @crtrott wrote: > Question: does this bug potentially affect code generation for > AMD/NVIDIA/Intel GPUs? I believe the easiest way to test that is to try compiling `struct S {}; S ret() { return S(); }` i

[PATCH] D157334: [clang] Define _MSC_EXTENSIONS on -gnu if -fms-extensions is set

mstorsjo added a comment. I think this makes sense in principle - however it does diverge from GCC. GCC also supports `-fms-extensions` (but I'm not sure if the set of extensions that are enabled by the option is an exact match between the two cases), but GCC doesn't expose any extra define in

[PATCH] D152054: [OpenMP] Codegen support for thread_limit on target directive

mstorsjo added inline comments. Comment at: openmp/runtime/test/target/target_thread_limit.cpp:28 +// OMP51: target: parallel +// OMP51: target: parallel +// OMP51-NOT: target: parallel sandeepkosuri wrote: > mstorsjo wrote: > > mstorsjo wrote: > > > This test fa

[PATCH] D154176: [Driver][MSVC] Move lld specific flags before inputs

mstorsjo added a comment. Ok, thanks for clarifying. However I still don’t understand the “why” aspect here. You’re writing > so that lld specific flags can be append before inputs Are you planning on adding such flags in a later patch, position dependent flags that need to be supplied before

[PATCH] D154176: [Driver][MSVC] Move lld specific flags before inputs

mstorsjo added a comment. In D154176#4466186 , @mstorsjo wrote: > Ok, thanks for clarifying. However I still don’t understand the “why” aspect > here. You’re writing > >> so that lld specific flags can be append before inputs > > Are you planning on addi

<    4   5   6   7   8   9   10   11   12   13   >