[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] 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] D61670: [clang] [MinGW] Add the option -fno-auto-import

This revision was automatically updated to reflect the committed changes. Closed by commit rGf9f2fdcf03d9: [clang] [MinGW] Add the option -fno-auto-import (authored by mstorsjo). Changed prior to commit: https://reviews.llvm.org/D61670?vs=553624&id=555462#toc Repository: rG LLVM Github Monor

[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 mstorsjo wrote: > This test fails when running (on Windows) on GitHub Actions

[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 This test fails when running (on Windows) on GitHub Actions runners - see ht

[PATCH] D152752: [MS] Fix passing aligned records by value in some cases

mstorsjo added a comment. In D152752#4626234 , @rnk wrote: > I need to get to it, my recollection is that @mstorsjo ran into the same > issue here for mingw and made some changes, I wanted to go dig those up as a > starting point. I may have completely

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

mstorsjo added a comment. In D61670#4621724 , @aeubanks wrote: > I don't have all the context here, but seems fine once the commit description > is updated with the new spelling Thanks. Yeah I've updated the commit message locally (I wonder if the `arc`

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

mstorsjo added a comment. In D152054#4622642 , @vadikp-intel wrote: > Windows importing is now done by name, and new exports do not need to have an > ordinal specified for them i.e. you can add a line with just the API name to > dllexports. Oh, right,

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

mstorsjo added subscribers: vadikp-intel, natgla. mstorsjo added a comment. In D152054#4620927 , @mstorsjo wrote: > This new test is failing on Windows, due to `__kmpc_set_thread_limit` not > being exported - see e.g. > https://github.com/mstorsjo/llvm-

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

mstorsjo added a comment. This new test is failing on Windows, due to `__kmpc_set_thread_limit` not being exported - see e.g. https://github.com/mstorsjo/llvm-mingw/actions/runs/5994183421/job/16264501555. Can someone add it to `openmp/runtime/src/dllexports`? Repository: rG LLVM Github Mon

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

mstorsjo updated this revision to Diff 553624. mstorsjo edited the summary of this revision. mstorsjo added a comment. Updated to use the form -fno-auto-import and similar split it into two separate words everywhere. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://rev

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

mstorsjo added a comment. In D61670#4611790 , @aeubanks wrote: > In D61670#4607313 , @mstorsjo wrote: > >>> I expected the answer would be "yes", so I said "lgtm" and then phrased my >>> question very awkwardly. >>

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

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGa23bf1786be7: [clang] [MinGW] Pass LTO options to the linker (authored by mstorsjo). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTIO

[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] 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] 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 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] 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] 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] 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] 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] 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] 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] 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 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 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] 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] 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] 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 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] 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] 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] 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] D151188: [LLD][COFF] Add LLVM toolchain library paths by default.

mstorsjo added a comment. Maybe add a release note for this too? It's rather significant. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151188/new/ https://reviews.llvm.org/D151188 ___ cfe-commits mailin

[PATCH] D151188: [LLD][COFF] Add LLVM toolchain library paths by default.

mstorsjo accepted this revision. mstorsjo added a comment. LGTM, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151188/new/ https://reviews.llvm.org/D151188 ___ cfe-commits mailing list cfe-commit

[PATCH] D155111: [clangd] Fix build failures observed on build bots for missing libs

mstorsjo added a comment. To clarify the issue - the kind of builds that seems to be broken is builds with `BUILD_SHARED_LIBS=ON`. The reason is that these libraries are needed is because the `clangd` target includes `$`, so all the dependencies of `clangDaemonTweaks` would need to be included

[PATCH] D145302: [clangd] Add library for clangd main function

mstorsjo added a comment. In D145302#4491973 , @glandium wrote: > This broke building with `-DLLVM_LINK_LLVM_DYLIB=ON`: I also ran into this; I pushed a fix in a20d57e83441a69fa2bab86593b18cc0402095d2

[PATCH] D154295: [Driver][MSVC] Support DWARF fission when using LTO on Windows

mstorsjo accepted this revision. mstorsjo added subscribers: hans, rnk. mstorsjo added a comment. This revision is now accepted and ready to land. I think this looks reasonable, unless e.g. @hans or @rnk would disagree. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://r

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

mstorsjo accepted this revision. mstorsjo added a comment. This revision is now accepted and ready to land. In D154176#4466230 , @HaohaiWen wrote: > In D154176#4466190 , @mstorsjo > wrote: > >> In D154176#4466186

[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

[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. Is this a NFC change, as a preparation for a separate change? In that case, please add an NFC tag to the subject - if not, please explain (and test) the expected behaviour change. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.

[PATCH] D154070: [lld/COFF] Add /dwodir to enable DWARF fission with LTO

mstorsjo added a comment. No objection from me here, this seems straightforward. We should probably add a corresponding option in the lld mingw frontend too (as a separate patch later). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154070/new/ htt

[PATCH] D152785: [COFF] Support -gsplit-dwarf for COFF on Windows

mstorsjo added a comment. In D152785#4438544 , @MaskRay wrote: > CodeView is the default debug info format for COFF. Perhaps `-gsplit-dwarf` > should be rejected when the default `-gcodeview` is used, with a test. IIRC some users do generate both CodeVi

[PATCH] D144006: [DebugMetadata][DwarfDebug] Support function-local types in lexical block scopes (4/7)

mstorsjo added a comment. This change triggers failed asserts when compiling code for at least arm and aarch64. It is reproducible with this reduced testcase: $ cat repro.c typedef long long a; typedef int b(); int c, d; long e, f; short g, j; void *h; short i[]; char k; a l,

[PATCH] D152856: [Driver] Allow warning for unclaimed TargetSpecific options

mstorsjo added inline comments. Comment at: llvm/include/llvm/Option/Arg.h:53 + /// This is used for generating an "argument unused" warning (without + /// clang::driver::options::TargetSpecific) or "unsupported option" error + /// (with TargetSpecific). MaskR

[PATCH] D152856: [Driver] Allow warning for unclaimed TargetSpecific options

mstorsjo accepted this revision. mstorsjo added a comment. This revision is now accepted and ready to land. Looks great to me, thanks! (Didn't test it myself yet though.) Just one small nit. Comment at: llvm/include/llvm/Option/Arg.h:53 + /// This is used for generating an "a

[PATCH] D152318: [Fuchsia] Add llvm-strings to Fuchsia clang build

mstorsjo added a comment. In D152318#4402493 , @haowei wrote: > In D152318#4402234 , @mstorsjo > wrote: > >> FWIW, I'm curious about where you need `llvm-strings` in a MinGW setting. >> While it does match a GNU

[PATCH] D152318: [Fuchsia] Add llvm-strings to Fuchsia clang build

mstorsjo added a comment. FWIW, I'm curious about where you need `llvm-strings` in a MinGW setting. While it does match a GNU binutils tool, I'm kinda curious where it is needed. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152318/new/ https://re

[PATCH] D148793: [clang-tidy] Implement an include-cleaner check.

mstorsjo added a comment. In D148793#4393385 , @mgorny wrote: > My educated guess would be that `clangIncludeCleaner` is being linked via > `clang_target_link_libraries` while it's not part of `libclang-cpp`, so it > should go into regular `target_link_

[PATCH] D148793: [clang-tidy] Implement an include-cleaner check.

mstorsjo added a comment. In D148793#4392788 , @mgorny wrote: > I'm getting completely incomprehensible build errors on Gentoo from this I'm also hitting this; it only seems to happen if building with `-DLLVM_LINK_LLVM_DYLIB=ON`. Repository: rG LLVM

[PATCH] D151662: [clang] [test] Fix test failures due to -Wbuiltin-macro-redefined in MinGW mode

mstorsjo abandoned this revision. mstorsjo added a comment. Superseded by D151741 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151662/new/ https://reviews.llvm.org/D151662 __

[PATCH] D151783: [clang] Use the appropriate definition when checking FunctionDecl::isInlineBuiltinDeclaration

mstorsjo accepted this revision. mstorsjo added a comment. This revision is now accepted and ready to land. LGTM, thanks! I'm not familiar with the code in question to know whether this really is the right thing to do etc, but this fixes my testcase (both the reduced one and the full one). Re

[PATCH] D151741: [Lex] Only warn on defining or undefining language-defined builtins

mstorsjo added a comment. Thanks! +1 from me, this does clear up all the issues I've had - it supersedes D151662 and makes the external patch to avoid doing `-U__STRICT_ANSI__` unnecessary. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION ht

[PATCH] D151661: [clang] [test] Narrow down an MSVC specific behaviour to only not covever MinGW

This revision was automatically updated to reflect the committed changes. Closed by commit rG28b26b161c2f: [clang] [test] Narrow down an MSVC specific behaviour to only not covever MinGW (authored by mstorsjo). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llv

[PATCH] D148723: [clang] Restrict Inline Builtin to non-static, non-odr linkage

mstorsjo added a comment. In D148723#4381414 , @serge-sans-paille wrote: > In D148723#4380165 , @mstorsjo > wrote: > >> This causes failed asserts with `_FORTIFY_SOURCE` with the mingw-w64 >> headers. Here's a

[PATCH] D144654: [Lex] Warn when defining or undefining any builtin macro

mstorsjo added a comment. FWIW, I've also run into noisy warnings caused by this, in a few places. D151662 fixes a bunch of clang's tests when run in MinGW configurations, and https://patchwork.ffmpeg.org/project/ffmpeg/patch/20230526104837.20594-1-mar...@mart

[PATCH] D148723: [clang] Restrict Inline Builtin to non-static, non-odr linkage

mstorsjo added a comment. This causes failed asserts with `_FORTIFY_SOURCE` with the mingw-w64 headers. Here's a reduced reproducer: $ cat reduced.c typedef unsigned int size_t; void *memcpy(void *_Dst, const void *_Src, size_t _Size); extern __inline__ __attribute__((__always_inli

[PATCH] D151661: [clang] [test] Narrow down an MSVC specific behaviour to only not covever MinGW

mstorsjo added a comment. In D151661#4379958 , @Endill wrote: > Thank you! > I've asked Aaron what is the best way to check for MSVC-like triple before > relanding this test. So today we both learned. Generally, the canonical way to check for it is `#if

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

mstorsjo added a comment. In D150646#4379543 , @glandium wrote: > There seem to still be two problems with this change, with mingw: > > - with `-fms-extensions`: > > echo '#include "cpuid.h"' | ./clang/bin/clang++ > --target=x86_64-w64-windows-gnu -xc+

[PATCH] D151662: [clang] [test] Fix test failures due to -Wbuiltin-macro-redefined in MinGW mode

mstorsjo created this revision. mstorsjo added reviewers: john.brawn, aaron.ballman. Herald added a project: All. mstorsjo requested review of this revision. Herald added a project: clang. These tests undefine __declspec or __stdcall, both which are builtin macros in MinGW mode. Therefore, build t

[PATCH] D151661: [clang] [test] Narrow down an MSVC specific behaviour to only not covever MinGW

mstorsjo created this revision. mstorsjo added reviewers: Endill, hans, aaron.ballman. Herald added a project: All. mstorsjo requested review of this revision. Herald added a project: clang. This uses the same logic as in c2b256a990590dc8b69930259650cfeb085add03

[PATCH] D151620: [clang-repl] Fix REPL_EXTERNAL_VISIBILITY and building libclang-cpp.dll for MinGW configurations

This revision was automatically updated to reflect the committed changes. Closed by commit rG592e935e115f: [clang-repl] Fix REPL_EXTERNAL_VISIBILITY and building libclang-cpp.dll for… (authored by mstorsjo). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.o

[PATCH] D151620: [clang-repl] Fix REPL_EXTERNAL_VISIBILITY and building libclang-cpp.dll for MinGW configurations

mstorsjo created this revision. mstorsjo added reviewers: junaire, alvinhochun, mati865. Herald added a project: All. mstorsjo requested review of this revision. Herald added a subscriber: bd1976llvm. Herald added a project: clang. This fixes two issues that are observed after 5111286f06e1e10f2474

[PATCH] D151344: Reland "[CMake] Bumps minimum version to 3.20.0.

mstorsjo added inline comments. Comment at: libunwind/src/CMakeLists.txt:28-35 # See add_asm_sources() in compiler-rt for explanation of this workaround. # CMake doesn't work correctly with assembly on AIX. Workaround by compiling # as C files as well. if((APPLE AND CMAKE_V

[PATCH] D144509: [CMake] Bumps minimum version to 3.20.0.

mstorsjo added a comment. In D144509#4349051 , @hans wrote: > In D144509#4347562 , @glandium > wrote: > >> FYI, 65429b9af6a2c99d340ab2dcddd41dab201f399c >>

[PATCH] D144509: [CMake] Bumps minimum version to 3.20.0.

mstorsjo added a comment. In D144509#4347562 , @glandium wrote: > FYI, 65429b9af6a2c99d340ab2dcddd41dab201f399c > is > causing problems on Windows compiler-rt for some reason I haven'

[PATCH] D149997: [clang] [test] Narrow down MSVC specific behaviours from "any windows" to only MSVC/clang-cl

This revision was automatically updated to reflect the committed changes. Closed by commit rGc2b256a99059: Reapply [clang] [test] Narrow down MSVC specific behaviours from "any windows"… (authored by mstorsjo). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llv

[PATCH] D149997: [clang] [test] Narrow down MSVC specific behaviours from "any windows" to only MSVC/clang-cl

mstorsjo updated this revision to Diff 521781. mstorsjo added a comment. Updated to check for `defined(_WIN32) && !defined(__MINGW32__)`. It's a kinda ugly way of checking for an MSVC-like environment specifically (when the MSVC mode is the exception compared with the rest), but neither `_MSC_VE

[PATCH] D149997: [clang] [test] Narrow down MSVC specific behaviours from "any windows" to only MSVC/clang-cl

mstorsjo added a comment. In D149997#4338304 , @rnk wrote: > I think `_MSC_EXTENSIONS` will work, but it's kind of gross. `_MSC_VER` is > controlled by `-fms-compatibility-verson=`, which I think is off by default > or unset at the cc1 level: > https://

[PATCH] D149997: [clang] [test] Narrow down MSVC specific behaviours from "any windows" to only MSVC/clang-cl

mstorsjo added a subscriber: rnk. mstorsjo added a comment. So, the reason why this failed, is that when invoked as `%clang_cc1` in a MSVC/clang-cl style build, `_MSC_VER` isn't predefined, while `_WIN32` is. When invoked via the Clang driver instead of directly going at `-cc1`, `_MSC_VER` does

[PATCH] D149997: [clang] [test] Narrow down MSVC specific behaviours from "any windows" to only MSVC/clang-cl

mstorsjo added a comment. In D149997#4331937 , @thakis wrote: > This broke check-clang on windows: http://45.33.8.238/win/78359/step_7.txt > > (The Driver/split-debug.c failure is something else and since fixed, but the > other two tests are due to this

[PATCH] D149999: [clang-tidy] [test] Narrow down a special case to MSVC mode

This revision was automatically updated to reflect the committed changes. Closed by commit rGb80febdf43d3: [clang-tidy] [test] Narrow down a special case to MSVC mode (authored by mstorsjo). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D14/new/

[PATCH] D149997: [clang] [test] Narrow down MSVC specific behaviours from "any windows" to only MSVC/clang-cl

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG7f037e5645bd: [clang] [test] Narrow down MSVC specific behaviours from "any windows" to only… (authored by mstorsjo). Repository: rG LLVM Github M

[PATCH] D149999: [clang-tidy] [test] Narrow down a special case to MSVC mode

mstorsjo created this revision. mstorsjo added reviewers: alvinhochun, aaron.ballman, hans. Herald added subscribers: PiotrZSL, carlosgalvezp, xazax.hun. Herald added a project: All. mstorsjo requested review of this revision. Herald added a project: clang-tools-extra. For MinGW targets, size_t is

[PATCH] D149997: [clang] [test] Narrow down MSVC specific behaviours from "any windows" to only MSVC/clang-cl

mstorsjo created this revision. mstorsjo added reviewers: aaron.ballman, hans. Herald added a project: All. mstorsjo requested review of this revision. Herald added a project: clang. This fixes running tests with a toolchain that defaults to a MinGW target. Repository: rG LLVM Github Monorepo

[PATCH] D148944: [clang][Driver] Fix crash with unsupported architectures in MinGW and CrossWindows.

mstorsjo added inline comments. Comment at: clang/test/Driver/unsupported-target-arch.c:33 +// RUN: FileCheck --input-file=%t.err --check-prefix=CHECK-NOARCH-CROSSWINDOWS %s +// CHECK-NOARCH-CROSSWINDOWS: error: unknown target triple 'noarch-unknown-windows-itanium', please use

[PATCH] D148944: [clang][Driver] Fix crash with unsupported architectures in MinGW and CrossWindows.

mstorsjo accepted this revision. mstorsjo added a comment. This revision is now accepted and ready to land. LGTM. If you don’t have commit access, please say your preferred git author name for the commit, i.e. `Real Name `. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION http

[PATCH] D146490: [Support] On Windows, ensure that UniqueID is really stable

mstorsjo added a comment. In D146490#4307269 , @falhumai96 wrote: > I would like to follow up on this issue whether or not there has been an > update on it. There's no update currently; I think there's some amount of consensus that this approach, whil

[PATCH] D148944: [clang][Driver] Fix crash with unsupported architectures in MinGW and CrossWindows.

mstorsjo added a comment. I think the code changes make sense; these are trivial to hit by a user, so they shouldn't be `llvm_unreachable`. Comment at: clang/test/Driver/unsupported-target-arch.c:33 +// RUN: FileCheck --input-file=%t.err --check-prefix=CHECK-NOARCH-CROSSWINDOW

[PATCH] D149119: [CMake] Use llvm-nm to extract symbols for staged LTO builds on Windows

mstorsjo added a comment. In D149119#4298024 , @phosek wrote: > Supporting only a single tool and simplifying the script would be my > preference as well. I see that the script already supports `llvm-readobj`, do > we need the `llvm-nm` support in that

[PATCH] D146987: [Assignment Tracking] Enable by default

mstorsjo added a comment. In D146987#4286797 , @jmorse wrote: > /me grumbles about all the world being a VAX, > > @mstorsjo I can't replicate the crash, but can replicate the valgrind > jump-on-uninitialized-value with a small reproducer [0] that doesn't

[PATCH] D146987: [Assignment Tracking] Enable by default

mstorsjo added a comment. I'm also running into issues due to this commit. Unfortunately, the issues don't seem to be entirely deterministic. They can be reproduced with https://martin.st/temp/python-preproc.c: $ clang -target i686-w64-mingw32 -c -g -O3 python-preproc.c Assertion failed: (!

[PATCH] D146591: [dataflow] add HTML logger: browse code/cfg/analysis timeline/state

mstorsjo added a comment. FYI, I'm seeing occasional spurious build breakage from this change. The issue is that even if we've got `add_dependencies(clangAnalysisFlowSensitive clangAnalysisFlowSensitiveResources)`, it looks like `clangAnalysisFlowSensitiveResources` only is considered a depend

[PATCH] D148751: [CMake] Add llvm-lib to Clang bootstrap dependency for LTO builds on Windows

mstorsjo added inline comments. Comment at: clang/CMakeLists.txt:616 -DDYLD_LIBRARY_PATH=${LLVM_LIBRARY_OUTPUT_INTDIR}) -elseif(NOT WIN32) +elseif(WIN32) + add_dependencies(clang-bootstrap-deps llvm-lib) I think I’d prefer to have the condit

[PATCH] D146908: [clang][MinGW] Add asan DLL lib before other libs and objects

mstorsjo added inline comments. Comment at: clang/test/Driver/mingw-sanitizers.c:2 +// RUN: touch %t.a +// RUN: %clang -target i686-windows-gnu %s -### -fsanitize=address -lcomponent %/t.a 2>&1 | FileCheck --check-prefixes=ASAN-ALL,ASAN-I686 -DINPUT=%/t.a %s +// RUN: %clang -tar

[PATCH] D146908: [clang][MinGW] Add asan DLL lib before other libs and objects

mstorsjo accepted this revision. mstorsjo added a comment. This revision is now accepted and ready to land. LGTM overall. I wondered if the modified test runs correctly on Windows (any test that touches paths is prone to break) but I see that it seems to have run successfully on both Windows an

[PATCH] D146908: [clang][MinGW] Add asan link flags before other libs and objects

mstorsjo added inline comments. Comment at: clang/test/Driver/mingw-sanitizers.c:1 -// RUN: %clang -target i686-windows-gnu %s -### -fsanitize=address 2>&1 | FileCheck --check-prefix=ASAN-I686 %s -// ASAN-I686: "{{.*}}libclang_rt.asan_dynamic-i386.dll.a" -// ASAN-I686: "{{[^"]*}

[PATCH] D146908: [clang][MinGW] Add asan link flags before other libs and objects

mstorsjo added inline comments. Comment at: clang/test/Driver/mingw-sanitizers.c:1 -// RUN: %clang -target i686-windows-gnu %s -### -fsanitize=address 2>&1 | FileCheck --check-prefix=ASAN-I686 %s -// ASAN-I686: "{{.*}}libclang_rt.asan_dynamic-i386.dll.a" -// ASAN-I686: "{{[^"]*}

[PATCH] D146908: [clang][MinGW] Add asan link flags before other libs and objects

mstorsjo added a comment. I tested this, and this does fix the repro from the linked issue. I wonder if we do want to move all of these before the app itself, or if it's enough to just move the reference to `asan_dynamic-.dll.a` earlier, and keep the rest as it was? Especially the whole-archive

[PATCH] D146764: [clang] Make predefined expressions string literals under -fms-extensions

mstorsjo added a comment. In D146764#4219278 , @hans wrote: > +mstorsjo is this okay for mingw mode too? I believe the current form of the patch is fine - i.e. disabled by default in mingw mode, but enabled if the extra MS language extensions are enable

[PATCH] D146490: [Support] On Windows, ensure that UniqueID is really stable

mstorsjo added a comment. In D146490#4209495 , @aganea wrote: > Fair enough. There are several choices forward: either we mark the issue as > "Will Not Fix" or I can try only scoping this patch to only keep the handle > for network drives/paths. Any oth

[PATCH] D146165: docs: add some documentation on Windows SDK search

mstorsjo added a comment. Looks reasonable I guess - but I think it would be good to mention the env variables `INCLUDE` and `LIB` too, for alternative ways of finding the same things - even if it's not strictly the same as what this new section talks about. Repository: rG LLVM Github Monor

[PATCH] D144651: [Serialization] Place command line defines in the correct file

mstorsjo added a reviewer: aaron.ballman. mstorsjo added a subscriber: aaron.ballman. mstorsjo added a comment. Adding @aaron.ballman as fallback Clang reviewer here. While I did touch code in the vicinity of this area recently, I'm not familiar enough with the whole area to take on reviewing it

[PATCH] D145007: Driver: introduce GNU spellings to control MSVC paths

mstorsjo accepted this revision. mstorsjo added a comment. This revision is now accepted and ready to land. If others don't have anything to add on this matter, I guess this is fine. The new option names feel rather wordy and unwieldy (especially compared to the clang-cl forms they're aliases fo

[PATCH] D145007: Driver: introduce GNU spellings to control MSVC paths

mstorsjo added reviewers: akhuang, hans, thakis, zahen, pkasting. mstorsjo added a comment. Looks reasonable to me, adding a few others for opinions. Should we have some tests for this? I guess testing these is somewhat environment reliant though, so it's probably not so easy... Repository:

[PATCH] D142224: [Support] Emulate SIGPIPE handling in raw_fd_ostream write for Windows

mstorsjo added inline comments. Comment at: llvm/lib/Support/Windows/Signals.inc:834 + int RetCode = (int)EP->ExceptionRecord->ExceptionCode; + if (RetCode == (0xE000 | EX_IOERR)) +return; erichkeane wrote: > aganea wrote: > > erichkeane wrote: > > > Th

[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] 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] 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

  1   2   3   4   5   6   7   8   9   10   >