[PATCH] D111952: [clang] [MinGW] Guess the right ix86 arch name spelling as sysroot

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

I guess a downside of this solution, is that if an `i686` sysroot exists next 
to the clang binary, it becomes practically impossible to test codegen 
differences when you run it with various `-target iX86-w64-mingw32` options, as 
they'd all be corrected back to `i686`. (This wouldn't be an issue if the 
autodetection was done in `computeTargetTriple` only when the `-m32`/`-m64` 
options are used though.)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D111952/new/

https://reviews.llvm.org/D111952

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D111579: [clang] Fix DIFile directory root on Windows

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added inline comments.



Comment at: clang/test/CodeGen/debug-prefix-map.c:24
+// CHECK-NO-MAIN-FILE-NAME: !DIFile(filename: 
"{{/|C:}}UNLIKELY_PATH{{/|}}empty{{/|}}{{.*}}",
+// CHECK-NO-MAIN-FILE-NAME-SAME:directory: "")
+// CHECK-NO-MAIN-FILE-NAME: !DIFile(filename: 
"{{/|C:}}UNLIKELY_PATH{{/|}}empty{{/|}}Inputs{{/|}}stdio.h",

keith wrote:
> keith wrote:
> > mstorsjo wrote:
> > > I presume that this patch goes on top of D111457? It might be good to set 
> > > that one as the parent revision of this one, so that the premerge test 
> > > runs applies them on top of each other (right now, this one failed to 
> > > apply).
> > > 
> > > Isn't this particular change present (`directory` being `""` here) 
> > > already after the previous patch?
> > > 
> > > 
> > That is marked as the parent of this patch in phab, but maybe I need to do 
> > something else to get them to apply?
> > Isn't this particular change present (directory being "" here) already 
> > after the previous patch?
> 
> This does still require this change since otherwise directory still gets `C:`
> 
> 
Hmm, it seems like the premerge testing managed to start running this before 
the parent was set (and I didn't notice it when I looked at the patches the 
last round). If you reupload the patch later when this already is set, I think 
it should work though - but no hurry with that right now.



Comment at: clang/test/CodeGen/debug-prefix-map.c:24
+// CHECK-NO-MAIN-FILE-NAME: !DIFile(filename: 
"{{/|C:}}UNLIKELY_PATH{{/|}}empty{{/|}}{{.*}}",
+// CHECK-NO-MAIN-FILE-NAME-SAME:directory: "")
+// CHECK-NO-MAIN-FILE-NAME: !DIFile(filename: 
"{{/|C:}}UNLIKELY_PATH{{/|}}empty{{/|}}Inputs{{/|}}stdio.h",

mstorsjo wrote:
> keith wrote:
> > keith wrote:
> > > mstorsjo wrote:
> > > > I presume that this patch goes on top of D111457? It might be good to 
> > > > set that one as the parent revision of this one, so that the premerge 
> > > > test runs applies them on top of each other (right now, this one failed 
> > > > to apply).
> > > > 
> > > > Isn't this particular change present (`directory` being `""` here) 
> > > > already after the previous patch?
> > > > 
> > > > 
> > > That is marked as the parent of this patch in phab, but maybe I need to 
> > > do something else to get them to apply?
> > > Isn't this particular change present (directory being "" here) already 
> > > after the previous patch?
> > 
> > This does still require this change since otherwise directory still gets 
> > `C:`
> > 
> > 
> Hmm, it seems like the premerge testing managed to start running this before 
> the parent was set (and I didn't notice it when I looked at the patches the 
> last round). If you reupload the patch later when this already is set, I 
> think it should work though - but no hurry with that right now.
Oh, ok.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D111579/new/

https://reviews.llvm.org/D111579

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D111579: [clang] Fix DIFile directory root on Windows

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added inline comments.



Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:439
+if (llvm::sys::path::root_path(DirBuf) == DirBuf) {
+  // Don't strip the common prefix if it is only the root ("/" or "C:\")
   // since that would make LLVM diagnostic locations confusing.

This change is correct IMO.



Comment at: clang/test/CodeGen/debug-prefix-map.c:24
+// CHECK-NO-MAIN-FILE-NAME: !DIFile(filename: 
"{{/|C:}}UNLIKELY_PATH{{/|}}empty{{/|}}{{.*}}",
+// CHECK-NO-MAIN-FILE-NAME-SAME:directory: "")
+// CHECK-NO-MAIN-FILE-NAME: !DIFile(filename: 
"{{/|C:}}UNLIKELY_PATH{{/|}}empty{{/|}}Inputs{{/|}}stdio.h",

I presume that this patch goes on top of D111457? It might be good to set that 
one as the parent revision of this one, so that the premerge test runs applies 
them on top of each other (right now, this one failed to apply).

Isn't this particular change present (`directory` being `""` here) already 
after the previous patch?




Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D111579/new/

https://reviews.llvm.org/D111579

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D111457: [clang][test] Add lit helper for windows paths

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

Wouldn't this one also be solved pretty much the same, but differently, by 
changing `if (llvm::sys::path::is_absolute(RemappedFile)) {` into 
`is_absolute_gnu`? Since we're remapping debug paths, it's plausible that the 
target path can be a different style (when cross compiling, where debug prefix 
remapping is kinda important), and then it's probably good to use a more lax 
definition of whether a path is absolute. (Alternatively, we maybe should try 
to detect the kind of path used and use the appropriate style as argument to 
`is_absolute`, but I don't think that necessarily helps here.)

(Overall I think it might be good to have the native separators and root 
available for tests though, even if this particular test might make sense both 
as-is and with native separators.)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D111457/new/

https://reviews.llvm.org/D111457

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D105169: [Clang/Test]: Rename enable_noundef_analysis to disable-noundef-analysis and turn it off by default

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

I've bisected a crash in generated code down to this commit. The code that 
crashes is clean when run in ubsan. The misbehaviour happens across 4 tested 
architectures (i686, x86_64, armv7, aarch64).

The misbehaviour occurs in this preprocessed source, 
https://martin.st/temp/seek-preproc.c, compiled with `clang -target 
x86_64-linux-gnu -c -O3 seek-preproc.c`.

The whole failing testcase can be reproduced (on e.g. linux) with these steps:

  git clone git://source.ffmpeg.org/ffmpeg
  cd ffmpeg
  ./configure --cc=clang
  make -j4 fate-seek-acodec-flac

(The failing object file is `libavformat/seek.o`.)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105169/new/

https://reviews.llvm.org/D105169

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D105169: [Clang/Test]: Rename enable_noundef_analysis to disable-noundef-analysis and turn it off by default

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

In D105169#3069220 , @aqjune wrote:

> It seems the original code has a use of an uninitialized variable.
> Line 4420 at seek-preproc.c (function `ff_seek_frame_binary`):
>
>int64_t pos_min=pos_min, pos_max=pos_max, pos, pos_limit; // pos_min and 
> pos_max are self-assigned.
>   ...
>   if (sti->index_entries) {
>  ...
>   }
>   // pos_min and pos_max are used as arguments below
>   pos = ff_gen_search(s, stream_index, target_ts, pos_min, pos_max, pos_limit,
>ts_min, ts_max, flags, , avif->read_timestamp);
>
> https://gist.github.com/aqjune/3bd0ea19bbc12b4744843c0c070e994c
>
> If the branch is not taken, `pos_min` and `pos_max` are read while they are 
> still uninitialized.
>
> I guess the variables are self-assigned to avoid warnings?

Yes, I believe so. If the branch is not taken, `pos_min` and `pos_max` are 
undefined when entering `ff_gen_search`. (I would assume that their value isn't 
ever used within `ff_gen_search` in that case.) But regardless of that, in this 
case, the generated code crashes around this line, 
https://gist.github.com/aqjune/3bd0ea19bbc12b4744843c0c070e994c#file-ff_seek_frame_binary-c-L39,
 before entering `ff_gen_search` - and within that branch, those variables are 
properly set before they're used.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105169/new/

https://reviews.llvm.org/D105169

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D111952: [clang] [MinGW] Guess the right ix86 arch name spelling as sysroot

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo created this revision.
mstorsjo added reviewers: phosek, MaskRay.
Herald added subscribers: pengfei, kristof.beyls.
mstorsjo requested review of this revision.
Herald added a project: clang.

For x86, most contempory mingw toolchains use i686 as 32 bit
x86 arch target.

As long as the target triple is set to the right form, this works
fine, either as the compiler's default target, or via e.g.
a triple prefix like i686-w64-mingw32-clang.

However, if the unprefixed toolchain targets x86_64, but the user
tries to switch it to target 32 bit by adding the -m32 option, the
computeTargetTriple function in Clang, together with
Triple::get32BitArchVariant, sets the arch to i386. This causes
the right sysroot to not be found.

When targeting an arch where there are potential spelling ambiguities
with respect to the sysroots (i386 and arm), check if the driver can
find a sysroot with the arch name - if not, try a couple other
candidates.

Other design ideas considered: It would fit in better with the design
to do this kind of arch name fixups in computeTargetTriple (it
already has a couple other OS/arch specific cases), but the
heuristics for detecting the potential right choice ties in quite
closely with how the toolchain looks for the implicit sysroot to use,
so I'd prefer to keep that logic in the toolchain::MinGW class/file.

This is done as a wrapper function before invoking the toolchain::MinGW()
constructor, because the MinGW() constructor can't run code of its own
until the superclass ToolChain() constructor returns. The fixups need to
be done before the ToolChain() constructor, because it uses the Triple
for per-target runtime directories.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D111952

Files:
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/MinGW.cpp
  clang/lib/Driver/ToolChains/MinGW.h
  clang/test/Driver/mingw-sysroot.cpp

Index: clang/test/Driver/mingw-sysroot.cpp
===
--- clang/test/Driver/mingw-sysroot.cpp
+++ clang/test/Driver/mingw-sysroot.cpp
@@ -12,6 +12,7 @@
 // RUN: mkdir -p %T/testroot-clang/bin
 // RUN: ln -s %clang %T/testroot-clang/bin/x86_64-w64-mingw32-clang
 // RUN: ln -s %S/Inputs/mingw_ubuntu_posix_tree/usr/x86_64-w64-mingw32 %T/testroot-clang/x86_64-w64-mingw32
+// RUN: ln -s %S/Inputs/mingw_arch_tree/usr/i686-w64-mingw32 %T/testroot-clang/i686-w64-mingw32
 
 
 // If we find a gcc in the path with the right triplet prefix, pick that as
@@ -36,3 +37,14 @@
 // the libgcc directory:
 
 // RUN: env "PATH=%T/testroot-gcc/bin:%PATH%" %T/testroot-gcc/bin/x86_64-w64-mingw32-clang -target x86_64-w64-mingw32 -rtlib=platform -stdlib=libstdc++ --sysroot="" -c -### %s 2>&1 | FileCheck -check-prefix=CHECK_TESTROOT_GCC %s
+
+
+// If the user requests a different arch via the -m32 option, which changes
+// x86_64 into i386, check that the driver notices that it can't find a
+// sysroot for i386 but there is one for i686, and uses that one.
+// (In practice, this usecase is when using an unprefixed native clang
+// that defaults to x86_64 mingw, but it's easier to test in cross setups
+// with symlinks, like the other tests here.)
+
+// RUN: env "PATH=%T/testroot-gcc/bin:%PATH%" %T/testroot-clang/bin/x86_64-w64-mingw32-clang -target x86_64-w64-mingw32 -m32 -rtlib=compiler-rt -stdlib=libstdc++ --sysroot="" -c -### %s 2>&1 | FileCheck -check-prefix=CHECK_TESTROOT_CLANG_I686 %s
+// CHECK_TESTROOT_CLANG_I686: "{{[^"]+}}/testroot-clang{{/|}}i686-w64-mingw32{{/|}}include"
Index: clang/lib/Driver/ToolChains/MinGW.h
===
--- clang/lib/Driver/ToolChains/MinGW.h
+++ clang/lib/Driver/ToolChains/MinGW.h
@@ -56,10 +56,15 @@
 namespace toolchains {
 
 class LLVM_LIBRARY_VISIBILITY MinGW : public ToolChain {
-public:
+private:
   MinGW(const Driver , const llvm::Triple ,
 const llvm::opt::ArgList );
 
+public:
+  static std::unique_ptr Create(const Driver ,
+   const llvm::Triple ,
+   const llvm::opt::ArgList );
+
   bool HasNativeLLVMSupport() const override;
 
   bool IsIntegratedAssemblerDefault() const override;
@@ -103,8 +108,6 @@
   mutable std::unique_ptr Preprocessor;
   mutable std::unique_ptr Compiler;
   void findGccLibDir();
-  llvm::ErrorOr findGcc();
-  llvm::ErrorOr findClangRelativeSysroot();
 
   bool NativeLLVMSupport;
 };
Index: clang/lib/Driver/ToolChains/MinGW.cpp
===
--- clang/lib/Driver/ToolChains/MinGW.cpp
+++ clang/lib/Driver/ToolChains/MinGW.cpp
@@ -369,9 +369,9 @@
   }
 }
 
-llvm::ErrorOr toolchains::MinGW::findGcc() {
+static llvm::ErrorOr findGcc(const llvm::Triple ) {
   llvm::SmallVector, 2> Gccs;
-  Gccs.emplace_back(getTriple().getArchName());
+  Gccs.emplace_back(T.getArchName());
   Gccs[0] += "-w64-mingw32-gcc";
   Gccs.emplace_back("mingw32-gcc");
   // 

[PATCH] D105169: [Clang/Test]: Rename enable_noundef_analysis to disable-noundef-analysis and turn it off by default

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

In D105169#3069555 , @aqjune wrote:

> I see, the crash is happening at the line because SimplifyCFG removes the 
> `sti->index_entries` null pointer check (which seems valid to me).
> If `stl->index_entries` was null, it would lead to uses of uninitialized 
> variables as function arguments, which is UB.
> SimplifyCFG relies on the information and removes the `stl->index_entries` 
> null check.

Thanks! That explains it. Although the actual crash is due to yet another 
removed condition:

  int64_t pos_min=pos_min, pos_max=pos_max, pos, pos_limit; // pos_min and 
pos_max are undef
  ...
  if (sti->index_entries) {
 index = av_index_search_timestamp();
 if (index >= 0) {
 ... (dereference sti->index_entries+index)
 ... (initialize pos_min and pos_max)
 }
  }
  // If sti->index_entries was NULL, UB must happen at the call below because 
undef is passed to ff_gen_search's noundef arg.
  pos = ff_gen_search(s, stream_index, target_ts, pos_min, pos_max, pos_limit,
   ts_min, ts_max, flags, , avif->read_timestamp);

Both of the nested if conditions are removed:

  int64_t pos_min=pos_min, pos_max=pos_max, pos, pos_limit;
  ...
  if (true) { // Changed to true!
 index = av_index_search_timestamp();
 if (true) { // Also changed to true
 ... (dereference sti->index_entries+index) // This can crash with 
index = -1
 ... (initialize pos_min and pos_max)
 }
  }
  pos = ff_gen_search(s, stream_index, target_ts, pos_min, pos_max, pos_limit,
   ts_min, ts_max, flags, , avif->read_timestamp);



> I think the solution is to initialize `pos_min` and `pos_max` to some value.
> If `sti->index_entries` is null, they are never used inside `ff_gen_search` 
> anyway, so initializing it into any value (e.g. 0) will work.

Yes, any value should be fine (and I guess 0 is the easiest to optimize for the 
compiler). Just as background - this is in a codebase that really tries to 
avoid default variable initializations unless they're proven to be necessary. 
But here they clearly are due to the UB rules of the language.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105169/new/

https://reviews.llvm.org/D105169

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D111081: [clang] [MinGW] Fix paths on Gentoo

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

Looks ok to me - WDYT @mgorny?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D111081/new/

https://reviews.llvm.org/D111081

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D111707: [clang] [Windows] Mark PIC as implicitly enabled for aarch64, just like for x86_64

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added inline comments.



Comment at: clang/test/Driver/pic.c:322
 // RUN:   | FileCheck %s --check-prefix=CHECK-PIC2
+// RUN: %clang -c %s -target aarch64-windows-msvc -### 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=CHECK-PIC2

MaskRay wrote:
> perhaps change windows-pic.cpp instead
That file actually doesn't test the thing that this patch touches (if I remove 
the existing cases for `x86_64` in the modified functions, that test still 
passes but this one breaks), so I thought this one is a better fit, wrt what 
the test actually tests.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D111707/new/

https://reviews.llvm.org/D111707

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D111707: [clang] [Windows] Mark PIC as implicitly enabled for aarch64, just like for x86_64

[Martin Storsjö via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGb541845ea082: [clang] [Windows] Mark PIC as implicitly 
enabled for aarch64, just like for… (authored by mstorsjo).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D111707/new/

https://reviews.llvm.org/D111707

Files:
  clang/lib/Driver/ToolChains/MSVC.cpp
  clang/lib/Driver/ToolChains/MinGW.cpp
  clang/test/Driver/pic.c


Index: clang/test/Driver/pic.c
===
--- clang/test/Driver/pic.c
+++ clang/test/Driver/pic.c
@@ -314,8 +314,12 @@
 // RUN: %clang -c %s -target aarch64-linux-android24 -fno-PIE -### 2>&1 \
 // RUN:   | FileCheck %s --check-prefix=CHECK-NO-PIC
 //
-// On Windows-X64 PIC is enabled by default
+// On Windows x86_64 and aarch64 PIC is enabled by default
 // RUN: %clang -c %s -target x86_64-pc-windows-msvc18.0.0 -### 2>&1 \
 // RUN:   | FileCheck %s --check-prefix=CHECK-PIC2
 // RUN: %clang -c %s -target x86_64-pc-windows-gnu -### 2>&1 \
 // RUN:   | FileCheck %s --check-prefix=CHECK-PIC2
+// RUN: %clang -c %s -target aarch64-windows-msvc -### 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=CHECK-PIC2
+// RUN: %clang -c %s -target aarch64-windows-gnu -### 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=CHECK-PIC2
Index: clang/lib/Driver/ToolChains/MinGW.cpp
===
--- clang/lib/Driver/ToolChains/MinGW.cpp
+++ clang/lib/Driver/ToolChains/MinGW.cpp
@@ -474,13 +474,15 @@
 }
 
 bool toolchains::MinGW::isPICDefault() const {
-  return getArch() == llvm::Triple::x86_64;
+  return getArch() == llvm::Triple::x86_64 ||
+ getArch() == llvm::Triple::aarch64;
 }
 
 bool toolchains::MinGW::isPIEDefault() const { return false; }
 
 bool toolchains::MinGW::isPICDefaultForced() const {
-  return getArch() == llvm::Triple::x86_64;
+  return getArch() == llvm::Triple::x86_64 ||
+ getArch() == llvm::Triple::aarch64;
 }
 
 llvm::ExceptionHandling
Index: clang/lib/Driver/ToolChains/MSVC.cpp
===
--- clang/lib/Driver/ToolChains/MSVC.cpp
+++ clang/lib/Driver/ToolChains/MSVC.cpp
@@ -795,7 +795,8 @@
 }
 
 bool MSVCToolChain::isPICDefault() const {
-  return getArch() == llvm::Triple::x86_64;
+  return getArch() == llvm::Triple::x86_64 ||
+ getArch() == llvm::Triple::aarch64;
 }
 
 bool MSVCToolChain::isPIEDefault() const {
@@ -803,7 +804,8 @@
 }
 
 bool MSVCToolChain::isPICDefaultForced() const {
-  return getArch() == llvm::Triple::x86_64;
+  return getArch() == llvm::Triple::x86_64 ||
+ getArch() == llvm::Triple::aarch64;
 }
 
 void MSVCToolChain::AddCudaIncludeArgs(const ArgList ,


Index: clang/test/Driver/pic.c
===
--- clang/test/Driver/pic.c
+++ clang/test/Driver/pic.c
@@ -314,8 +314,12 @@
 // RUN: %clang -c %s -target aarch64-linux-android24 -fno-PIE -### 2>&1 \
 // RUN:   | FileCheck %s --check-prefix=CHECK-NO-PIC
 //
-// On Windows-X64 PIC is enabled by default
+// On Windows x86_64 and aarch64 PIC is enabled by default
 // RUN: %clang -c %s -target x86_64-pc-windows-msvc18.0.0 -### 2>&1 \
 // RUN:   | FileCheck %s --check-prefix=CHECK-PIC2
 // RUN: %clang -c %s -target x86_64-pc-windows-gnu -### 2>&1 \
 // RUN:   | FileCheck %s --check-prefix=CHECK-PIC2
+// RUN: %clang -c %s -target aarch64-windows-msvc -### 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=CHECK-PIC2
+// RUN: %clang -c %s -target aarch64-windows-gnu -### 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=CHECK-PIC2
Index: clang/lib/Driver/ToolChains/MinGW.cpp
===
--- clang/lib/Driver/ToolChains/MinGW.cpp
+++ clang/lib/Driver/ToolChains/MinGW.cpp
@@ -474,13 +474,15 @@
 }
 
 bool toolchains::MinGW::isPICDefault() const {
-  return getArch() == llvm::Triple::x86_64;
+  return getArch() == llvm::Triple::x86_64 ||
+ getArch() == llvm::Triple::aarch64;
 }
 
 bool toolchains::MinGW::isPIEDefault() const { return false; }
 
 bool toolchains::MinGW::isPICDefaultForced() const {
-  return getArch() == llvm::Triple::x86_64;
+  return getArch() == llvm::Triple::x86_64 ||
+ getArch() == llvm::Triple::aarch64;
 }
 
 llvm::ExceptionHandling
Index: clang/lib/Driver/ToolChains/MSVC.cpp
===
--- clang/lib/Driver/ToolChains/MSVC.cpp
+++ clang/lib/Driver/ToolChains/MSVC.cpp
@@ -795,7 +795,8 @@
 }
 
 bool MSVCToolChain::isPICDefault() const {
-  return getArch() == llvm::Triple::x86_64;
+  return getArch() == llvm::Triple::x86_64 ||
+ getArch() == llvm::Triple::aarch64;
 }
 
 bool MSVCToolChain::isPIEDefault() const {
@@ -803,7 +804,8 @@
 }
 
 bool MSVCToolChain::isPICDefaultForced() const {
-  return getArch() == llvm::Triple::x86_64;
+  return getArch() == llvm::Triple::x86_64 ||
+ getArch() 

[PATCH] D111195: [clang][Tooling] Use Windows command lines on all Windows, except Cygwin

[Martin Storsjö via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGd9b9a7f42870: [clang][Tooling] Use Windows command lines on 
all Windows, except Cygwin (authored by jeremyd2019, committed by mstorsjo).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D95/new/

https://reviews.llvm.org/D95

Files:
  clang/lib/Tooling/JSONCompilationDatabase.cpp


Index: clang/lib/Tooling/JSONCompilationDatabase.cpp
===
--- clang/lib/Tooling/JSONCompilationDatabase.cpp
+++ clang/lib/Tooling/JSONCompilationDatabase.cpp
@@ -135,15 +135,12 @@
 std::vector unescapeCommandLine(JSONCommandLineSyntax Syntax,
  StringRef EscapedCommandLine) {
   if (Syntax == JSONCommandLineSyntax::AutoDetect) {
+#ifdef _WIN32
+// Assume Windows command line parsing on Win32
+Syntax = JSONCommandLineSyntax::Windows;
+#else
 Syntax = JSONCommandLineSyntax::Gnu;
-llvm::Triple Triple(llvm::sys::getProcessTriple());
-if (Triple.getOS() == llvm::Triple::OSType::Win32) {
-  // Assume Windows command line parsing on Win32 unless the triple
-  // explicitly tells us otherwise.
-  if (!Triple.hasEnvironment() ||
-  Triple.getEnvironment() == llvm::Triple::EnvironmentType::MSVC)
-Syntax = JSONCommandLineSyntax::Windows;
-}
+#endif
   }
 
   if (Syntax == JSONCommandLineSyntax::Windows) {


Index: clang/lib/Tooling/JSONCompilationDatabase.cpp
===
--- clang/lib/Tooling/JSONCompilationDatabase.cpp
+++ clang/lib/Tooling/JSONCompilationDatabase.cpp
@@ -135,15 +135,12 @@
 std::vector unescapeCommandLine(JSONCommandLineSyntax Syntax,
  StringRef EscapedCommandLine) {
   if (Syntax == JSONCommandLineSyntax::AutoDetect) {
+#ifdef _WIN32
+// Assume Windows command line parsing on Win32
+Syntax = JSONCommandLineSyntax::Windows;
+#else
 Syntax = JSONCommandLineSyntax::Gnu;
-llvm::Triple Triple(llvm::sys::getProcessTriple());
-if (Triple.getOS() == llvm::Triple::OSType::Win32) {
-  // Assume Windows command line parsing on Win32 unless the triple
-  // explicitly tells us otherwise.
-  if (!Triple.hasEnvironment() ||
-  Triple.getEnvironment() == llvm::Triple::EnvironmentType::MSVC)
-Syntax = JSONCommandLineSyntax::Windows;
-}
+#endif
   }
 
   if (Syntax == JSONCommandLineSyntax::Windows) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D111195: [clang][Tooling] Use Windows command lines on all Windows, except Cygwin

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo accepted this revision.
mstorsjo added a comment.
This revision is now accepted and ready to land.

LGTM


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D95/new/

https://reviews.llvm.org/D95

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D111081: [clang] [MinGW] Fix paths on Gentoo

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a reviewer: mstorsjo.
mstorsjo added a subscriber: cfe-commits.
mstorsjo added a comment.

(I’ll have a look and comment on the patch later.)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D111081/new/

https://reviews.llvm.org/D111081

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D111081: [clang] [MinGW] Fix paths on Gentoo

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

The change looks ok to me, but I think it'd be good to have a testcase for 
finding these paths (IIRC there are a bunch of simulated sysroots under 
clang/test/Driver).


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D111081/new/

https://reviews.llvm.org/D111081

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D111195: [clang][Tooling] Use Windows command lines on all Windows, except Cygwin

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a subscriber: rnk.
mstorsjo added a comment.

This looks ok to me I guess. Technically in this case, this is pretty much 
equivalent to `#ifdef _WIN32`, i.e. any form when running on windows, where the 
process considers itself windows (cygwin doesn't define `_WIN32`), but I guess 
keeping it based on `getProcessTriple()` is fine too.

I'll leave it open for discussion for a while, but if there's no further 
opinions I can accept it later.

@rnk Do you happen to have any experience here?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D95/new/

https://reviews.llvm.org/D95

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42225: libcxx: Provide overloads for basic_filebuf::open() et al that take wchar_t* filenames on Windows.

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

In D42225#3020688 , @CaseyCarter wrote:

> In D42225#2963216 , @ldionne wrote:
>
>> In D42225#2963190 , @mstorsjo wrote:
>>
>>> In D42225#2962348 , @ldionne wrote:
>>>
 @pcc @mstorsjo Are we aware of anyone using these extensions?

 I would like to suggest that we either remove this extension if it's not 
 useful, or make it unconditional (not only on Windows) if we really think 
 it's that useful (but I'd like that to come with at least a paper 
 proposing to add them to the standard). Carrying around an extension 
 that's only enabled on one platform (and not the most widely used platform 
 for libc++ at that) is kind of awkward.
>>>
>>> This extension is fairly essential - without it, you can't interact with 
>>> files that have names outside of the 8 bit charset on Windows (and exactly 
>>> what the 8 bit charset is, can vary from system to system). I can't point 
>>> to a specific user of it, but I'd expect there to be numerous out there.
>>>
>>> Making it universally available sounds like a sensible strategy forward - 
>>> although I don't think I have the bandwidth to take on making it a 
>>> standards proposal. Maybe someone from Microsoft (who invented this 
>>> extension) can collaborate on it?
>>
>> @CaseyCarter Any appetite for that?
>
> This isn't an extension for us. The Standard mandates overloads of 
> `basic_filebuf::open`, `basic_ifstream::open`, `basic_ofstream::open`, and 
> `basic_fstream::open`, as well as constructors for `basic_ifstream`, 
> `basic_ofstream`, and `basic_fstream` that take `const 
> filesystem::path::value_type*` which are "only be [sic] provided on systems 
> where `filesystem::path::value_type` is not `char`." ([fstream.syn]/3) 
> Unsurprisingly, `filesystem::path::value_type` for us is `wchar_t`. If you're 
> supporting our mess of narrow character encodings, you may want to consider 
> `wchar_t` for `filesystem::path` on Win32 as well. If you are only supporting 
> UTF-8, I suppose you could instead transcode filenames to UTF-16 and use 
> _wfopen?

We also use `wchar_t` for `filesystem::path::value_type` on Windows, 
essentially following the MS STL when it comes to those details. I think also 
libstdc++ might have added these `wchar_t` overloads these days.


Repository:
  rCXX libc++

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D42225/new/

https://reviews.llvm.org/D42225

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D111952: [clang] [MinGW] Guess the right ix86 arch name spelling as sysroot

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo updated this revision to Diff 381795.
mstorsjo added a comment.

Moved the fixup into `computeTargetTriple`, so it's only done after applying 
the `-m32`/`-m64` options.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D111952/new/

https://reviews.llvm.org/D111952

Files:
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/MinGW.cpp
  clang/lib/Driver/ToolChains/MinGW.h
  clang/test/Driver/mingw-sysroot.cpp

Index: clang/test/Driver/mingw-sysroot.cpp
===
--- clang/test/Driver/mingw-sysroot.cpp
+++ clang/test/Driver/mingw-sysroot.cpp
@@ -12,6 +12,7 @@
 // RUN: mkdir -p %T/testroot-clang/bin
 // RUN: ln -s %clang %T/testroot-clang/bin/x86_64-w64-mingw32-clang
 // RUN: ln -s %S/Inputs/mingw_ubuntu_posix_tree/usr/x86_64-w64-mingw32 %T/testroot-clang/x86_64-w64-mingw32
+// RUN: ln -s %S/Inputs/mingw_arch_tree/usr/i686-w64-mingw32 %T/testroot-clang/i686-w64-mingw32
 
 
 // If we find a gcc in the path with the right triplet prefix, pick that as
@@ -36,3 +37,14 @@
 // the libgcc directory:
 
 // RUN: env "PATH=%T/testroot-gcc/bin:%PATH%" %T/testroot-gcc/bin/x86_64-w64-mingw32-clang -target x86_64-w64-mingw32 -rtlib=platform -stdlib=libstdc++ --sysroot="" -c -### %s 2>&1 | FileCheck -check-prefix=CHECK_TESTROOT_GCC %s
+
+
+// If the user requests a different arch via the -m32 option, which changes
+// x86_64 into i386, check that the driver notices that it can't find a
+// sysroot for i386 but there is one for i686, and uses that one.
+// (In practice, this usecase is when using an unprefixed native clang
+// that defaults to x86_64 mingw, but it's easier to test in cross setups
+// with symlinks, like the other tests here.)
+
+// RUN: env "PATH=%T/testroot-gcc/bin:%PATH%" %T/testroot-clang/bin/x86_64-w64-mingw32-clang -target x86_64-w64-mingw32 -m32 -rtlib=compiler-rt -stdlib=libstdc++ --sysroot="" -c -### %s 2>&1 | FileCheck -check-prefix=CHECK_TESTROOT_CLANG_I686 %s
+// CHECK_TESTROOT_CLANG_I686: "{{[^"]+}}/testroot-clang{{/|}}i686-w64-mingw32{{/|}}include"
Index: clang/lib/Driver/ToolChains/MinGW.h
===
--- clang/lib/Driver/ToolChains/MinGW.h
+++ clang/lib/Driver/ToolChains/MinGW.h
@@ -60,6 +60,9 @@
   MinGW(const Driver , const llvm::Triple ,
 const llvm::opt::ArgList );
 
+  static void FixTripleArch(const Driver , llvm::Triple ,
+const llvm::opt::ArgList );
+
   bool HasNativeLLVMSupport() const override;
 
   bool IsIntegratedAssemblerDefault() const override;
@@ -103,8 +106,6 @@
   mutable std::unique_ptr Preprocessor;
   mutable std::unique_ptr Compiler;
   void findGccLibDir();
-  llvm::ErrorOr findGcc();
-  llvm::ErrorOr findClangRelativeSysroot();
 
   bool NativeLLVMSupport;
 };
Index: clang/lib/Driver/ToolChains/MinGW.cpp
===
--- clang/lib/Driver/ToolChains/MinGW.cpp
+++ clang/lib/Driver/ToolChains/MinGW.cpp
@@ -369,9 +369,9 @@
   }
 }
 
-llvm::ErrorOr toolchains::MinGW::findGcc() {
+static llvm::ErrorOr findGcc(const llvm::Triple ) {
   llvm::SmallVector, 2> Gccs;
-  Gccs.emplace_back(getTriple().getArchName());
+  Gccs.emplace_back(T.getArchName());
   Gccs[0] += "-w64-mingw32-gcc";
   Gccs.emplace_back("mingw32-gcc");
   // Please do not add "gcc" here
@@ -381,13 +381,14 @@
   return make_error_code(std::errc::no_such_file_or_directory);
 }
 
-llvm::ErrorOr toolchains::MinGW::findClangRelativeSysroot() {
+static llvm::ErrorOr
+findClangRelativeSysroot(const Driver , const llvm::Triple ,
+ std::string ) {
   llvm::SmallVector, 2> Subdirs;
-  Subdirs.emplace_back(getTriple().str());
-  Subdirs.emplace_back(getTriple().getArchName());
+  Subdirs.emplace_back(T.str());
+  Subdirs.emplace_back(T.getArchName());
   Subdirs[1] += "-w64-mingw32";
-  StringRef ClangRoot =
-  llvm::sys::path::parent_path(getDriver().getInstalledDir());
+  StringRef ClangRoot = llvm::sys::path::parent_path(D.getInstalledDir());
   StringRef Sep = llvm::sys::path::get_separator();
   for (StringRef CandidateSubdir : Subdirs) {
 if (llvm::sys::fs::is_directory(ClangRoot + Sep + CandidateSubdir)) {
@@ -404,13 +405,16 @@
   RocmInstallation(D, Triple, Args) {
   getProgramPaths().push_back(getDriver().getInstalledDir());
 
+  // The sequence for detecting a sysroot here should be kept in sync with
+  // the testTriple function below.
   if (getDriver().SysRoot.size())
 Base = getDriver().SysRoot;
   // Look for /../; if found, use /.. as the
   // base as it could still be a base for a gcc setup with libgcc.
-  else if (llvm::ErrorOr TargetSubdir = findClangRelativeSysroot())
+  else if (llvm::ErrorOr TargetSubdir =
+   findClangRelativeSysroot(getDriver(), getTriple(), SysrootName))
 Base = std::string(llvm::sys::path::parent_path(TargetSubdir.get()));
-  else if (llvm::ErrorOr 

[PATCH] D111952: [clang] [MinGW] Guess the right ix86 arch name spelling as sysroot

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

In D111952#3068680 , @mstorsjo wrote:

> I guess a downside of this solution, is that if an `i686` sysroot exists next 
> to the clang binary, it becomes practically impossible to test codegen 
> differences when you run it with various `-target iX86-w64-mingw32` options, 
> as they'd all be corrected back to `i686`. (This wouldn't be an issue if the 
> autodetection was done in `computeTargetTriple` only when the `-m32`/`-m64` 
> options are used though.)

Ok, it turned out to not be that bad to move the fixup into 
`computeTargetTriple` to this location after all, it looks fairly neat and not 
that much out of line compared to the existing triple tweaks done there.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D111952/new/

https://reviews.llvm.org/D111952

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D112387: [clang] [MinGW] Rename the 'Arch' member to 'SysrootName'. NFC.

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo created this revision.
mstorsjo added a reviewer: MaskRay.
mstorsjo requested review of this revision.
Herald added a project: clang.

This string isn't a plain architecture name, but contains the whole
subdir name used for the sysroot, which often is equal to the target
triple.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D112387

Files:
  clang/lib/Driver/ToolChains/MinGW.cpp
  clang/lib/Driver/ToolChains/MinGW.h

Index: clang/lib/Driver/ToolChains/MinGW.h
===
--- clang/lib/Driver/ToolChains/MinGW.h
+++ clang/lib/Driver/ToolChains/MinGW.h
@@ -99,7 +99,7 @@
   std::string Base;
   std::string GccLibDir;
   std::string Ver;
-  std::string Arch;
+  std::string SysrootName;
   mutable std::unique_ptr Preprocessor;
   mutable std::unique_ptr Compiler;
   void findGccLibDir();
Index: clang/lib/Driver/ToolChains/MinGW.cpp
===
--- clang/lib/Driver/ToolChains/MinGW.cpp
+++ clang/lib/Driver/ToolChains/MinGW.cpp
@@ -349,20 +349,20 @@
 }
 
 void toolchains::MinGW::findGccLibDir() {
-  llvm::SmallVector, 2> Archs;
-  Archs.emplace_back(getTriple().getArchName());
-  Archs[0] += "-w64-mingw32";
-  Archs.emplace_back("mingw32");
-  if (Arch.empty())
-Arch = std::string(Archs[0].str());
+  llvm::SmallVector, 2> SysrootNames;
+  SysrootNames.emplace_back(getTriple().getArchName());
+  SysrootNames[0] += "-w64-mingw32";
+  SysrootNames.emplace_back("mingw32");
+  if (SysrootName.empty())
+SysrootName = std::string(SysrootNames[0].str());
   // lib: Arch Linux, Ubuntu, Windows
   // lib64: openSUSE Linux
   for (StringRef CandidateLib : {"lib", "lib64"}) {
-for (StringRef CandidateArch : Archs) {
+for (StringRef CandidateSysroot : SysrootNames) {
   llvm::SmallString<1024> LibDir(Base);
-  llvm::sys::path::append(LibDir, CandidateLib, "gcc", CandidateArch);
+  llvm::sys::path::append(LibDir, CandidateLib, "gcc", CandidateSysroot);
   if (findGccVersion(LibDir, GccLibDir, Ver)) {
-Arch = std::string(CandidateArch);
+SysrootName = std::string(CandidateSysroot);
 return;
   }
 }
@@ -391,7 +391,7 @@
   StringRef Sep = llvm::sys::path::get_separator();
   for (StringRef CandidateSubdir : Subdirs) {
 if (llvm::sys::fs::is_directory(ClangRoot + Sep + CandidateSubdir)) {
-  Arch = std::string(CandidateSubdir);
+  SysrootName = std::string(CandidateSubdir);
   return (ClangRoot + Sep + CandidateSubdir).str();
 }
   }
@@ -423,10 +423,10 @@
   // correct crtbegin.o ,cetend.o would be found.
   getFilePaths().push_back(GccLibDir);
   getFilePaths().push_back(
-  (Base + Arch + llvm::sys::path::get_separator() + "lib").str());
+  (Base + SysrootName + llvm::sys::path::get_separator() + "lib").str());
   getFilePaths().push_back(Base + "lib");
   // openSUSE
-  getFilePaths().push_back(Base + Arch + "/sys-root/mingw/lib");
+  getFilePaths().push_back(Base + SysrootName + "/sys-root/mingw/lib");
 
   NativeLLVMSupport =
   Args.getLastArgValue(options::OPT_fuse_ld_EQ, CLANG_DEFAULT_LINKER)
@@ -573,11 +573,12 @@
   if (GetRuntimeLibType(DriverArgs) == ToolChain::RLT_Libgcc) {
 // openSUSE
 addSystemInclude(DriverArgs, CC1Args,
- Base + Arch + "/sys-root/mingw/include");
+ Base + SysrootName + "/sys-root/mingw/include");
   }
 
   addSystemInclude(DriverArgs, CC1Args,
-   Base + Arch + llvm::sys::path::get_separator() + "include");
+   Base + SysrootName + llvm::sys::path::get_separator() +
+   "include");
   addSystemInclude(DriverArgs, CC1Args, Base + "include");
 }
 
@@ -596,8 +597,9 @@
 .str();
 if (getDriver().getVFS().exists(TargetDir))
   addSystemInclude(DriverArgs, CC1Args, TargetDir);
-addSystemInclude(DriverArgs, CC1Args, Base + Arch + Slash + "include" +
-  Slash + "c++" + Slash + "v1");
+addSystemInclude(DriverArgs, CC1Args,
+ Base + SysrootName + Slash + "include" + Slash + "c++" +
+ Slash + "v1");
 addSystemInclude(DriverArgs, CC1Args,
  Base + "include" + Slash + "c++" + Slash + "v1");
 break;
@@ -606,9 +608,10 @@
   case ToolChain::CST_Libstdcxx:
 llvm::SmallVector, 4> CppIncludeBases;
 CppIncludeBases.emplace_back(Base);
-llvm::sys::path::append(CppIncludeBases[0], Arch, "include", "c++");
+llvm::sys::path::append(CppIncludeBases[0], SysrootName, "include", "c++");
 CppIncludeBases.emplace_back(Base);
-llvm::sys::path::append(CppIncludeBases[1], Arch, "include", "c++", Ver);
+llvm::sys::path::append(CppIncludeBases[1], SysrootName, "include", "c++",
+Ver);
 CppIncludeBases.emplace_back(Base);
 llvm::sys::path::append(CppIncludeBases[2], "include", 

[PATCH] D114651: [clang-cl] Expose -Wall to clang-cl by unaliasing -Wall, keeping /Wall as alias to -Weverything

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

In D114651#3158807 , @rnk wrote:

> I will add that multiple users have run into this problem, and I think it 
> might be more practical to consider unaliasing Wall altogether. Clang doesn't 
> emit the same set of warnings as MSVC. Anyone seriously using `clang-cl 
> /Wall` is going to receive a pile of `-WcxxNN-compat` warnings that are 
> self-contradictory, and they are going to need to curate a set of 
> clang-specific warning flags anyway.

One possible downside with that, is that you'd easily end up making 
cl-compatible build files that works nicely with clang-cl but spew warnings 
with cl.exe. (Although maybe cl.exe's -Wall produce a much safer/saner set of 
warnings?) But I don't feel strongly about it..


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114651/new/

https://reviews.llvm.org/D114651

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D113254: [clang] Fix a misadjusted path style comparison in a unittest

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

Ping @dexonsmith


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113254/new/

https://reviews.llvm.org/D113254

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D114064: [clang] [MinGW] Pass --no-demangle through to the mingw linker

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a reviewer: MaskRay.
mstorsjo added a comment.

Adding more potential reviewers. For context, see 
https://github.com/llvm/llvm-project/blob/llvmorg-14-init/clang/lib/Driver/Driver.cpp#L335-L353
 where the original option is filtered out.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114064/new/

https://reviews.llvm.org/D114064

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100810: [llvm] Use `GNUInstallDirs` to support custom installation dirs

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added inline comments.



Comment at: llvm/CMakeLists.txt:75
 set(LLVM_ENABLE_PROJECTS "" CACHE STRING
-   "Semicolon-separated list of projects to build 
(${LLVM_KNOWN_PROJECTS}), or \"all\".")
+   "Semicolon-separated list of projects to build (${LLVM_KNOWN_PROJECTS}), or 
\"all\".")
 foreach(proj ${LLVM_ENABLE_PROJECTS})

Nit: This looks like a spurious unrelated change to whitespace?



Comment at: llvm/CMakeLists.txt:164
   set(LLVM_CCACHE_MAXSIZE "" CACHE STRING "Size of ccache")
-  set(LLVM_CCACHE_DIR "" CACHE STRING "Directory to keep ccached data")
+  set(LLVM_CCACHE_DIR "" CACHE PATH "Directory to keep ccached data")
   set(LLVM_CCACHE_PARAMS "CCACHE_CPP2=yes CCACHE_HASHDIR=yes"

Could this bit be split out to separate change, to keep this as small as 
possible - unless it's strictly needed and tied to this one?



Comment at: llvm/CMakeLists.txt:294
 
-set(LLVM_UTILS_INSTALL_DIR "${LLVM_TOOLS_INSTALL_DIR}" CACHE STRING
+set(LLVM_UTILS_INSTALL_DIR "${LLVM_TOOLS_INSTALL_DIR}" CACHE PATH
 "Path to install LLVM utilities (enabled by LLVM_INSTALL_UTILS=ON) 
(defaults to LLVM_TOOLS_INSTALL_DIR)")

Nit: Unrelated and can be split out?



Comment at: llvm/CMakeLists.txt:408
 
-set(LLVM_Z3_INSTALL_DIR "" CACHE STRING "Install directory of the Z3 solver.")
+set(LLVM_Z3_INSTALL_DIR "" CACHE PATH "Install directory of the Z3 solver.")
 

Unrelated?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100810/new/

https://reviews.llvm.org/D100810

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D114064: [clang] [MinGW] Pass --no-demangle through to the mingw linker

[Martin Storsjö via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGa8877c5ccc0e: [clang] [MinGW] Pass --no-demangle through to 
the mingw linker (authored by mstorsjo).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114064/new/

https://reviews.llvm.org/D114064

Files:
  clang/lib/Driver/ToolChains/MinGW.cpp
  clang/test/Driver/Xlinker-args.c


Index: clang/test/Driver/Xlinker-args.c
===
--- clang/test/Driver/Xlinker-args.c
+++ clang/test/Driver/Xlinker-args.c
@@ -12,6 +12,11 @@
 // RUN:   -Wl,two,--no-demangle,three -Xlinker four -z five -r %s 2> %t
 // RUN: FileCheck -check-prefix=LINUX < %t %s
 
+/// Check that --no-demangle gets forwarded to the mingw linker
+// RUN: %clang -target x86_64-w64-mingw32 -### \
+// RUN:   -Wl,--no-demangle %s 2> %t
+// RUN: FileCheck -check-prefix=MINGW < %t %s
+
 // RUN: %clang -target powerpc-unknown-aix -### \
 // RUN:   -b one -b two %s 2> %t
 // RUN: FileCheck -check-prefix=AIX < %t %s
@@ -23,6 +28,7 @@
 // DARWIN-NOT: --no-demangle
 // DARWIN: "one" "two" "three" "four" "-z" "five" "-r"
 // LINUX: "--no-demangle" "-e" "_start" "one" "two" "three" "four" "-z" "five" 
"-r" {{.*}} "-T" "a.lds"
+// MINGW: "--no-demangle"
 // AIX: "-b" "one" "-b" "two"
 // NOT-AIX: error: unsupported option '-b' for target 'powerpc-unknown-linux'
 
Index: clang/lib/Driver/ToolChains/MinGW.cpp
===
--- clang/lib/Driver/ToolChains/MinGW.cpp
+++ clang/lib/Driver/ToolChains/MinGW.cpp
@@ -164,6 +164,9 @@
 CmdArgs.push_back("--enable-auto-image-base");
   }
 
+  if (Args.hasArg(options::OPT_Z_Xlinker__no_demangle))
+CmdArgs.push_back("--no-demangle");
+
   CmdArgs.push_back("-o");
   const char *OutputFile = Output.getFilename();
   // GCC implicitly adds an .exe extension if it is given an output file name


Index: clang/test/Driver/Xlinker-args.c
===
--- clang/test/Driver/Xlinker-args.c
+++ clang/test/Driver/Xlinker-args.c
@@ -12,6 +12,11 @@
 // RUN:   -Wl,two,--no-demangle,three -Xlinker four -z five -r %s 2> %t
 // RUN: FileCheck -check-prefix=LINUX < %t %s
 
+/// Check that --no-demangle gets forwarded to the mingw linker
+// RUN: %clang -target x86_64-w64-mingw32 -### \
+// RUN:   -Wl,--no-demangle %s 2> %t
+// RUN: FileCheck -check-prefix=MINGW < %t %s
+
 // RUN: %clang -target powerpc-unknown-aix -### \
 // RUN:   -b one -b two %s 2> %t
 // RUN: FileCheck -check-prefix=AIX < %t %s
@@ -23,6 +28,7 @@
 // DARWIN-NOT: --no-demangle
 // DARWIN: "one" "two" "three" "four" "-z" "five" "-r"
 // LINUX: "--no-demangle" "-e" "_start" "one" "two" "three" "four" "-z" "five" "-r" {{.*}} "-T" "a.lds"
+// MINGW: "--no-demangle"
 // AIX: "-b" "one" "-b" "two"
 // NOT-AIX: error: unsupported option '-b' for target 'powerpc-unknown-linux'
 
Index: clang/lib/Driver/ToolChains/MinGW.cpp
===
--- clang/lib/Driver/ToolChains/MinGW.cpp
+++ clang/lib/Driver/ToolChains/MinGW.cpp
@@ -164,6 +164,9 @@
 CmdArgs.push_back("--enable-auto-image-base");
   }
 
+  if (Args.hasArg(options::OPT_Z_Xlinker__no_demangle))
+CmdArgs.push_back("--no-demangle");
+
   CmdArgs.push_back("-o");
   const char *OutputFile = Output.getFilename();
   // GCC implicitly adds an .exe extension if it is given an output file name
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116503: [clang] Add arguments for silencing unused argument warnings for some but not all arguments

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo created this revision.
mstorsjo added reviewers: aaron.ballman, sepavloff, phosek, MaskRay.
Herald added a subscriber: dang.
mstorsjo requested review of this revision.
Herald added a project: clang.

When passing a set of flags to configure defaults for a specific
target (similar to the cmake settings `CLANG_DEFAULT_RTLIB`,
`CLANG_DEFAULT_UNWINDLIB`, `CLANG_DEFAULT_CXX_STDLIB` and
`CLANG_DEFAULT_LINKER`, but without hardcoding it in the binary),
some of the flags may cause warnings (e.g. `-stdlib=` when compiling C
code). Allow requesting selectively ignoring unused arguments among
some of the arguments on the command line, without needing to resort
to `-Qunused-arguments` or `-Wno-unused-command-line-argument`.

Fix up the existing diagnostics.c testcase. It was added in
response to PR12181 to fix handling of
`-Werror=unused-command-line-argument`, but the command line option
in the test (`-fzyzzybalubah`) now triggers "error: unknown argument"
instead of the intended warning. Change it into a linker input
(`-lfoo`) which triggers the intended diagnostic. Extend the
existing test case to check more cases and make sure that it keeps
testing the intended case.

Add testing of the new option to this existing test.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D116503

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/diagnostics.c

Index: clang/test/Driver/diagnostics.c
===
--- clang/test/Driver/diagnostics.c
+++ clang/test/Driver/diagnostics.c
@@ -1,9 +1,44 @@
 // Parse diagnostic arguments in the driver
 // PR12181
 
+// Exactly which arguments are warned about and which aren't differ based
+// on what target is selected. -stdlib= and -fuse-ld= emit diagnostics when
+// compiling C code, for e.g. *-linux-gnu. Linker inputs, like -lfoo, emit
+// diagnostics when only compiling for all targets.
+
+// This is normally a non-fatal warning:
+// RUN: %clang -target x86_64-apple-darwin10 \
+// RUN:   -fsyntax-only -lfoo %s 2>&1 | FileCheck %s
+
+// Either with a specific -Werror=unused.. or a blanket -Werror, this
+// causes the command to fail.
+// RUN: not %clang -target x86_64-apple-darwin10 \
+// RUN:   -fsyntax-only -lfoo \
+// RUN:   -Werror=unused-command-line-argument %s 2>&1 | FileCheck %s
+
 // RUN: not %clang -target x86_64-apple-darwin10 \
-// RUN:   -fsyntax-only -fzyzzybalubah \
-// RUN:   -Werror=unused-command-line-argument %s
+// RUN:   -fsyntax-only -lfoo -Werror %s 2>&1 | FileCheck %s
 
+// With a specific -Wno-..., no diagnostic should be printed.
+// RUN: %clang -target x86_64-apple-darwin10 \
+// RUN:   -fsyntax-only -lfoo -Werror \
+// RUN:   -Wno-unused-command-line-argument %s 2>&1 | count 0
+
+// With -Qunused-arguments, no diagnostic should be printed.
+// RUN: %clang -target x86_64-apple-darwin10 \
+// RUN:   -fsyntax-only -lfoo -Werror \
+// RUN:   -Qunused-arguments %s 2>&1 | count 0
+
+// With the argument enclosed in --{start,end}-no-unused-arguments,
+// there's no diagnostic.
+// RUN: %clang -target x86_64-apple-darwin10 -fsyntax-only \
+// RUN:   --start-no-unused-arguments -lfoo --end-no-unused-arguments \
+// RUN:   -Werror %s 2>&1 | count 0
+
+// With --{start,end}-no-unused-argument around a different argument, it
+// still warns about the unused argument.
 // RUN: not %clang -target x86_64-apple-darwin10 \
-// RUN:   -fsyntax-only -fzyzzybalubah -Werror %s
+// RUN:   --start-no-unused-arguments -fsyntax-only --end-no-unused-arguments \
+// RUN:   -lfoo -Werror %s 2>&1 | FileCheck %s
+
+// CHECK: -lfoo: 'linker' input unused
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -367,7 +367,20 @@
   bool HasNostdlib = Args.hasArg(options::OPT_nostdlib);
   bool HasNostdlibxx = Args.hasArg(options::OPT_nostdlibxx);
   bool HasNodefaultlib = Args.hasArg(options::OPT_nodefaultlibs);
+  bool IgnoreUnused = false;
   for (Arg *A : Args) {
+if (IgnoreUnused)
+  A->claim();
+
+if (A->getOption().matches(options::OPT_start_no_unused_arguments)) {
+  IgnoreUnused = true;
+  continue;
+}
+if (A->getOption().matches(options::OPT_end_no_unused_arguments)) {
+  IgnoreUnused = false;
+  continue;
+}
+
 // Unfortunately, we have to parse some forwarding options (-Xassembler,
 // -Xlinker, -Xpreprocessor) because we either integrate their functionality
 // (assembler and preprocessor), or bypass a previous driver ('collect2').
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -1076,6 +1076,8 @@
 def emit_merged_ifs : Flag<["-"], "emit-merged-ifs">,
   Flags<[CC1Option]>, Group,
   

[PATCH] D116503: [clang] Add arguments for silencing unused argument warnings for some but not all arguments

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo updated this revision to Diff 397476.
mstorsjo added a comment.

Regenerated ClangCommandLineReference.rst (with the relevant changes), removed 
NoXarchOption.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116503/new/

https://reviews.llvm.org/D116503

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/diagnostics.c

Index: clang/test/Driver/diagnostics.c
===
--- clang/test/Driver/diagnostics.c
+++ clang/test/Driver/diagnostics.c
@@ -1,9 +1,44 @@
 // Parse diagnostic arguments in the driver
 // PR12181
 
+// Exactly which arguments are warned about and which aren't differ based
+// on what target is selected. -stdlib= and -fuse-ld= emit diagnostics when
+// compiling C code, for e.g. *-linux-gnu. Linker inputs, like -lfoo, emit
+// diagnostics when only compiling for all targets.
+
+// This is normally a non-fatal warning:
+// RUN: %clang -target x86_64-apple-darwin10 \
+// RUN:   -fsyntax-only -lfoo %s 2>&1 | FileCheck %s
+
+// Either with a specific -Werror=unused.. or a blanket -Werror, this
+// causes the command to fail.
+// RUN: not %clang -target x86_64-apple-darwin10 \
+// RUN:   -fsyntax-only -lfoo \
+// RUN:   -Werror=unused-command-line-argument %s 2>&1 | FileCheck %s
+
 // RUN: not %clang -target x86_64-apple-darwin10 \
-// RUN:   -fsyntax-only -fzyzzybalubah \
-// RUN:   -Werror=unused-command-line-argument %s
+// RUN:   -fsyntax-only -lfoo -Werror %s 2>&1 | FileCheck %s
 
+// With a specific -Wno-..., no diagnostic should be printed.
+// RUN: %clang -target x86_64-apple-darwin10 \
+// RUN:   -fsyntax-only -lfoo -Werror \
+// RUN:   -Wno-unused-command-line-argument %s 2>&1 | count 0
+
+// With -Qunused-arguments, no diagnostic should be printed.
+// RUN: %clang -target x86_64-apple-darwin10 \
+// RUN:   -fsyntax-only -lfoo -Werror \
+// RUN:   -Qunused-arguments %s 2>&1 | count 0
+
+// With the argument enclosed in --{start,end}-no-unused-arguments,
+// there's no diagnostic.
+// RUN: %clang -target x86_64-apple-darwin10 -fsyntax-only \
+// RUN:   --start-no-unused-arguments -lfoo --end-no-unused-arguments \
+// RUN:   -Werror %s 2>&1 | count 0
+
+// With --{start,end}-no-unused-argument around a different argument, it
+// still warns about the unused argument.
 // RUN: not %clang -target x86_64-apple-darwin10 \
-// RUN:   -fsyntax-only -fzyzzybalubah -Werror %s
+// RUN:   --start-no-unused-arguments -fsyntax-only --end-no-unused-arguments \
+// RUN:   -lfoo -Werror %s 2>&1 | FileCheck %s
+
+// CHECK: -lfoo: 'linker' input unused
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -367,7 +367,20 @@
   bool HasNostdlib = Args.hasArg(options::OPT_nostdlib);
   bool HasNostdlibxx = Args.hasArg(options::OPT_nostdlibxx);
   bool HasNodefaultlib = Args.hasArg(options::OPT_nodefaultlibs);
+  bool IgnoreUnused = false;
   for (Arg *A : Args) {
+if (IgnoreUnused)
+  A->claim();
+
+if (A->getOption().matches(options::OPT_start_no_unused_arguments)) {
+  IgnoreUnused = true;
+  continue;
+}
+if (A->getOption().matches(options::OPT_end_no_unused_arguments)) {
+  IgnoreUnused = false;
+  continue;
+}
+
 // Unfortunately, we have to parse some forwarding options (-Xassembler,
 // -Xlinker, -Xpreprocessor) because we either integrate their functionality
 // (assembler and preprocessor), or bypass a previous driver ('collect2').
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -1076,6 +1076,8 @@
 def emit_merged_ifs : Flag<["-"], "emit-merged-ifs">,
   Flags<[CC1Option]>, Group,
   HelpText<"Generate Interface Stub Files, emit merged text not binary.">;
+def end_no_unused_arguments : Flag<["--"], "end-no-unused-arguments">, Flags<[CoreOption]>,
+  HelpText<"Start emitting warnings for unused driver arguments">;
 def interface_stub_version_EQ : JoinedOrSeparate<["-"], "interface-stub-version=">, Flags<[CC1Option]>;
 def exported__symbols__list : Separate<["-"], "exported_symbols_list">;
 def e : JoinedOrSeparate<["-"], "e">, Flags<[LinkerInput]>, Group;
@@ -3913,6 +3915,8 @@
 def single__module : Flag<["-"], "single_module">;
 def specs_EQ : Joined<["-", "--"], "specs=">, Group;
 def specs : Separate<["-", "--"], "specs">, Flags<[Unsupported]>;
+def start_no_unused_arguments : Flag<["--"], "start-no-unused-arguments">, Flags<[CoreOption]>,
+  HelpText<"Don't emit warnings about unused arguments for the following arguments">;
 def static_libgcc : Flag<["-"], "static-libgcc">;
 def static_libstdcxx : Flag<["-"], "static-libstdc++">;
 def static : Flag<["-", "--"], "static">, Group, 

[PATCH] D116503: [clang] Add arguments for silencing unused argument warnings for some but not all arguments

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

In D116503#3221083 , @MaskRay wrote:

> A more searchable subject (mentioning the new option in the subject is useful 
> for archaeology) may be `Add 
> --start-no-unused-arguments/--end-no-unused-arguments to silence some unused 
> argument warnings`

Thanks, that's indeed a better summary (although the subject line ends up even 
longer than now). Will change it to that form.




Comment at: clang/docs/ClangCommandLineReference.rst:1342
+
+Don't warn for unused arguments between these arguments.
+

MaskRay wrote:
> The sentence will be scrubbed.
> 
> Use `path/to/clang-tblgen -gen-opt-docs -I llvm/include -I 
> clang/include/clang/Driver clang/include/clang/Driver/ClangOptionDocs.td > 
> clang/docs/ClangCommandLineReference.rst` to regenerate this rst.
Ok, will do. There's lots of other changes in the file though, from other 
options that haven't been added here, but I'm just including the changes for 
the options I'm adding in this commit.



Comment at: clang/include/clang/Driver/Options.td:3918
 def specs : Separate<["-", "--"], "specs">, Flags<[Unsupported]>;
+def start_no_unused_arguments : Flag<["--"], "start-no-unused-arguments">, 
Flags<[NoXarchOption, CoreOption]>,
+  HelpText<"Don't emit warnings about unused arguments for the following 
arguments">;

MaskRay wrote:
> Drop `NoXarchOption`. The option can be used with Darwin -Xarch 
> (`test/Driver/Xarch.c`) though it is unnecessary to test it.
Ok, will do.



Comment at: clang/test/Driver/diagnostics.c:21
+// RUN:   -fsyntax-only -lfoo -Werror %s 2>&1 | FileCheck %s
 
+// With a specific -Wno-..., no diagnostic should be printed.

MaskRay wrote:
> Looks like no command has an output. In case there is one, make sure `-o 
> /dev/null`
As all commands use `-fsyntax-only`, I presume there should be no output. So do 
I read your comment correctly that we should have `-o /dev/null` specifically 
_if_ we'd have a test that lack `-fsyntax-only`, or do you want me to add it 
just in case? (The preexisting test doesn't have that and just use 
`-fsyntax-only`.)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116503/new/

https://reviews.llvm.org/D116503

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116020: [clang][#52782] Bail on incomplete parameter type in stdcall name mangling

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo accepted this revision.
mstorsjo added a comment.
This revision is now accepted and ready to land.

LGTM, the change seems sensible to me. But I’d prefer if you could hold off 
pushing it for another couple days, as others who might want to comment might 
not be following during the holiday season.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116020/new/

https://reviews.llvm.org/D116020

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D114064: [clang] [MinGW] Pass --no-demangle through to the mingw linker

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

In D114064#3216147 , @Ericson2314 
wrote:

> Not sure what the exact division of labor is between the GNU and MinGW 
> backends, but assuming there is no way to share this between them the patch 
> looks good.

Yeah. Also at this point, this option should probably rather be handled as 
opt-out for targets that don’t support it, instead of having every single 
target do manual work to pass it through.

Additionally, this probably doesn’t handle sequences of `--no-demangle 
--demangle` correctly, but this just gets this target to the same level as e.g. 
GNU.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114064/new/

https://reviews.llvm.org/D114064

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116485: [clang][MinGW] Explicitly ignore `-fPIC` & friends

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo accepted this revision.
mstorsjo added a comment.
This revision is now accepted and ready to land.

LGTM, thanks. The fact that GCC used to emit a non-fatal warning for this 
option, but no longer does, probably is the key deciding factor here.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116485/new/

https://reviews.llvm.org/D116485

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D112430: [ARM][libunwind] add PACBTI-M support for libunwind

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

Looks overall ok, without diving in very deep, except for this one change that 
seemed unrelated.




Comment at: libunwind/src/Unwind-EHABI.cpp:312
   uint8_t registers = getByte(data, offset++);
-  if (registers & 0xf0 || !registers)
+  if (registers & 0xf0)
 return _URC_FAILURE;

This particular change looks unrelated to the rest, on a quick glance


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112430/new/

https://reviews.llvm.org/D112430

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D109856: [libunwind][ARM] Handle end of stack during unwind

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo accepted this revision.
mstorsjo added a comment.
This revision is now accepted and ready to land.

In D109856#3155235 , @danielkiss 
wrote:

> In D109856#3154763 , @mstorsjo 
> wrote:
>
>> Looks reasonable I think. Is this a deficiency in the EHABI implementation 
>> only, i.e. this aspect works as it should in the regular dwarf 
>> implementation?
>
> Yes, it works on dwarf already ( also the test pass on X86/Aarch64 ).

Ok, thanks for the clarification!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D109856/new/

https://reviews.llvm.org/D109856

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D114576: [PR52549][clang-cl] Predefine _MSVC_EXECUTION_CHARACTER_SET

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

Would it be possible to move setting of this flag to somewhere else (e.g. 
somewhere in Driver/ToolChains/MSVC.cpp), so that it would be picked up also 
when building with the gcc-style driver with `--target=*-windows-msvc`? Overall 
in earlier discussions, there's been mostly positive reception to making that 
mode more usable even if it isn't quite as supported as actually invoking 
`clang-cl`. (For some things, like the equivalent of the cl level `-MD`/`-MT` 
options, there's currently nothing available in the gcc style driver - but for 
a feature like this, I don't see any reason why it shouldn't be available 
everywhere.)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114576/new/

https://reviews.llvm.org/D114576

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D114576: [PR52549][clang-cl] Predefine _MSVC_EXECUTION_CHARACTER_SET

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo accepted this revision.
mstorsjo added a comment.
This revision is now accepted and ready to land.

Thanks! This LGTM - but maybe wait a while if someone else has something to 
say. (Although I guess americans are on holiday today, so there's probably not 
many that will see and care today.)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114576/new/

https://reviews.llvm.org/D114576

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D114651: [clang-cl] Expose -Wall to clang-cl by unaliasing -Wall, keeping /Wall as alias to -Weverything

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

In D114651#3156374 , @zero9178 wrote:

> Is the deviation from MSVC behaviour here intentional? MSVC flags allow both 
> using a `/` as well as `-` as prefix. That means Both `-Wall` and `/Wall` are 
> accepted by MSVC as well as clang-cl and in both compilers currently lead to 
> ALL warnings being emitted. So this patch would deviate from that behaviour 
> as well as add confusion as every other option behaves the same in MSVC and 
> clang-cl, regardless of whether `-` or `/` is used as prefix.

+1, while it's annoying with `-Wall` not doing the expected thing when using 
(clang-)cl, that's to be expected - cl and gcc style drivers have entirely 
different options overall, so one generally have to take care and use the right 
kind of options for them (and it's just a bit inconvenient that some options 
overlap but differ). So specialcasing an exception for this particular option 
doesn't seem worth it. Even in the context of cl-only options, I tend to always 
use the slash form, because there's less risk of mixups.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114651/new/

https://reviews.llvm.org/D114651

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D113268: [clangd] Fix tests with forward slash as separator on windows

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

Sorry for taking so long to get back to you on this matter...

In D113268#3112026 , @sammccall wrote:

>> For someone unfamiliar with the tool, is there a simple "smoke test 
>> procedure" I could try out with it to kick the tyres?
>
> There's `clangd --check=some_file.cc` which simulates opening up a file in an 
> editor and clicking around in it.
> It does require a `compile_command.json` in a parent directory to know about 
> build flags.
> To try it out for real you need to connect an editor to it with a plugin 
> (like vscode-clangd).

Thanks! I tested both `clangd --check=some_file.cc` and vscode-clangd with a 
build of clangd set to prefer forward slashes, and it all worked fine so far. I 
didn't do any exhaustive use of it in vscode, but at least regular use seemed 
just fine.

> But realistic problems with paths tend to come from interactions with 
> external sources.
> For example, we had a bug that involved:
>
> - vscode sending paths like `file://c:/test.cc` (lowercase C), which made its 
> way into our AST cache
> - cmake creating `compile_commands.json` like `C:\test.cc` (uppercase C), 
> which made its way into our index
> - when renaming a symbol, results from the two weren't deduplicated against 
> each other so the edit was applied twice

Yup - I've seen lots of similar issues in other clang tests too. The best way 
forward to fix those would probably be to add some helper to `llvm::sys::path` 
for doing path comparisons, which would ignore case differences (on windows and 
macos) and/or separator style differences (on windows). But

Anyway, this change (and a build that prefers generating paths with forward 
slashes) seems to work as expected with regards to that - and I updated the 
patch with what was requested.

The whole setup for this mode isn't settled, so there's no buildbot for this 
mode (yet), we'll see if we end up setting one up. In any case, most of this 
change shouldn't be too intrusive I hope.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113268/new/

https://reviews.llvm.org/D113268

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D113268: [clangd] Fix tests with forward slash as separator on windows

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo updated this revision to Diff 388857.
mstorsjo added a comment.

Added a comment about the surprising change where the order of append and 
native is switched. Made the modification to `testRoot()` less hacky.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113268/new/

https://reviews.llvm.org/D113268

Files:
  clang-tools-extra/clangd/tool/ClangdMain.cpp
  clang-tools-extra/clangd/unittests/TestFS.cpp
  clang-tools-extra/clangd/unittests/TestFS.h
  clang-tools-extra/clangd/unittests/URITests.cpp


Index: clang-tools-extra/clangd/unittests/URITests.cpp
===
--- clang-tools-extra/clangd/unittests/URITests.cpp
+++ clang-tools-extra/clangd/unittests/URITests.cpp
@@ -133,8 +133,10 @@
 
 TEST(URITest, Resolve) {
 #ifdef _WIN32
-  EXPECT_THAT(resolveOrDie(parseOrDie("file:///c%3a/x/y/z")), "c:\\x\\y\\z");
-  EXPECT_THAT(resolveOrDie(parseOrDie("file:///c:/x/y/z")), "c:\\x\\y\\z");
+  llvm::SmallString<16> Ref("c:\\x\\y\\z");
+  llvm::sys::path::native(Ref);
+  EXPECT_THAT(resolveOrDie(parseOrDie("file:///c%3a/x/y/z")), Ref);
+  EXPECT_THAT(resolveOrDie(parseOrDie("file:///c:/x/y/z")), Ref);
 #else
   EXPECT_EQ(resolveOrDie(parseOrDie("file:/a/b/c")), "/a/b/c");
   EXPECT_EQ(resolveOrDie(parseOrDie("file://auth/a/b/c")), "//auth/a/b/c");
@@ -148,13 +150,17 @@
 
 TEST(URITest, ResolveUNC) {
 #ifdef _WIN32
+  llvm::SmallString<32> RefExample("example.com\\x\\y\\z");
+  llvm::sys::path::native(RefExample);
+  llvm::SmallString<16> RefLocalhost("127.0.0.1\\x\\y\\z");
+  llvm::sys::path::native(RefLocalhost);
   EXPECT_THAT(resolveOrDie(parseOrDie("file://example.com/x/y/z")),
-  "example.com\\x\\y\\z");
+  RefExample);
   EXPECT_THAT(resolveOrDie(parseOrDie("file://127.0.0.1/x/y/z")),
-  "127.0.0.1\\x\\y\\z");
+  RefLocalhost);
   // Ensure non-traditional file URI still resolves to correct UNC path.
   EXPECT_THAT(resolveOrDie(parseOrDie("file:127.0.0.1/x/y/z")),
-  "127.0.0.1\\x\\y\\z");
+  RefLocalhost);
 #else
   EXPECT_THAT(resolveOrDie(parseOrDie("file://example.com/x/y/z")),
   "//example.com/x/y/z");
Index: clang-tools-extra/clangd/unittests/TestFS.h
===
--- clang-tools-extra/clangd/unittests/TestFS.h
+++ clang-tools-extra/clangd/unittests/TestFS.h
@@ -75,7 +75,7 @@
 };
 
 // Returns an absolute (fake) test directory for this OS.
-const char *testRoot();
+std::string testRoot();
 
 // Returns a suitable absolute path for this OS.
 std::string testPath(PathRef File,
Index: clang-tools-extra/clangd/unittests/TestFS.cpp
===
--- clang-tools-extra/clangd/unittests/TestFS.cpp
+++ clang-tools-extra/clangd/unittests/TestFS.cpp
@@ -71,9 +71,11 @@
   FileName, std::move(CommandLine), "")};
 }
 
-const char *testRoot() {
+std::string testRoot() {
 #ifdef _WIN32
-  return "C:\\clangd-test";
+  llvm::SmallString<16> NativeRoot("C:\\clangd-test");
+  llvm::sys::path::native(NativeRoot);
+  return std::string(NativeRoot);
 #else
   return "/clangd-test";
 #endif
Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -541,15 +541,20 @@
   Body);
 Body = Body.ltrim('/');
 llvm::SmallString<16> Path(Body);
-path::native(Path);
 fs::make_absolute(TestScheme::TestDir, Path);
+// Convert the whole path to native separators after concatenating with
+// TestDir; TestDir might contain separators other than the preferred
+// on Windows if forward slashes are preferred.
+path::native(Path);
 return std::string(Path);
   }
 
   llvm::Expected
   uriFromAbsolutePath(llvm::StringRef AbsolutePath) const override {
 llvm::StringRef Body = AbsolutePath;
-if (!Body.consume_front(TestScheme::TestDir))
+llvm::SmallString<16> NativeTestDir(TestDir);
+llvm::sys::path::native(NativeTestDir);
+if (!Body.consume_front(NativeTestDir))
   return error("Path {0} doesn't start with root {1}", AbsolutePath,
TestDir);
 


Index: clang-tools-extra/clangd/unittests/URITests.cpp
===
--- clang-tools-extra/clangd/unittests/URITests.cpp
+++ clang-tools-extra/clangd/unittests/URITests.cpp
@@ -133,8 +133,10 @@
 
 TEST(URITest, Resolve) {
 #ifdef _WIN32
-  EXPECT_THAT(resolveOrDie(parseOrDie("file:///c%3a/x/y/z")), "c:\\x\\y\\z");
-  EXPECT_THAT(resolveOrDie(parseOrDie("file:///c:/x/y/z")), "c:\\x\\y\\z");
+  llvm::SmallString<16> Ref("c:\\x\\y\\z");
+  llvm::sys::path::native(Ref);
+  EXPECT_THAT(resolveOrDie(parseOrDie("file:///c%3a/x/y/z")), Ref);
+  

[PATCH] D110216: [clang] retain type sugar in auto / template argument deduction

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

This broke building ANGLE as part of Qt 5.15 for a mingw target, with the 
following error:

  
../../../3rdparty/angle/src/libANGLE/renderer/d3d/d3d11/ResourceManager11.cpp:532:38:
 error: explicit instantiation of 'allocate' does not refer to a function 
template, variable template, member function, member class, or static data 
member
  ANGLE_RESOURCE_TYPE_OP(Instantitate, ANGLE_INSTANTIATE_OP)
   ^
  
../../../3rdparty/angle/src/libANGLE/renderer/d3d/d3d11/ResourceManager11.h:301:15:
 note: candidate template ignored: could not match 'GetInitDataFromD3D11' 
(aka 'typename 
InitDataType::Value>::Value') against 
'const D3D11_SUBRESOURCE_DATA' 
  gl::Error allocate(Renderer11 *renderer,
^
  
../../../3rdparty/angle/src/libANGLE/renderer/d3d/d3d11/ResourceManager11.cpp:358:30:
 note: candidate template ignored: could not match 'GetInitDataFromD3D11' 
(aka 'typename 
InitDataType::Value>::Value') against 
'const D3D11_SUBRESOURCE_DATA'
  gl::Error ResourceManager11::allocate(Renderer11 *renderer,
   ^

Do you happen to know what's going on here? The files mentioned are 
https://code.qt.io/cgit/qt/qtbase.git/tree/src/3rdparty/angle/src/libANGLE/renderer/d3d/d3d11/ResourceManager11.cpp?h=5.15.1
 and 
https://code.qt.io/cgit/qt/qtbase.git/tree/src/3rdparty/angle/src/libANGLE/renderer/d3d/d3d11/ResourceManager11.h?h=5.15.1.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110216/new/

https://reviews.llvm.org/D110216

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D110216: [clang] retain type sugar in auto / template argument deduction

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

In D110216#3130193 , @mizvekov wrote:

> In D110216#3130184 , @mstorsjo 
> wrote:
>
>> Do you happen to know what's going on here? The files mentioned are 
>> https://code.qt.io/cgit/qt/qtbase.git/tree/src/3rdparty/angle/src/libANGLE/renderer/d3d/d3d11/ResourceManager11.cpp?h=5.15.1
>>  and 
>> https://code.qt.io/cgit/qt/qtbase.git/tree/src/3rdparty/angle/src/libANGLE/renderer/d3d/d3d11/ResourceManager11.h?h=5.15.1.
>
> Thanks for the report!
>
> Not immediately obvious to me, would need some time to isolate / create a 
> minimal test case.
> Would you be in a position to extract a preprocessed source file + command 
> line which repros this error?

Sure: https://martin.st/temp/angle-preproc.cpp, built with `clang -target 
x86_64-w64-mingw32 -std=c++17 -w -c angle-preproc.cpp`.

> Also, feel free to revert if it helps you.

No big hurry for me wrt that yet, at least until we know which part is at fault 
here.

(I haven't tested with the very latest Qt and/or ANGLE - the full build setup 
for this is rather complex.)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110216/new/

https://reviews.llvm.org/D110216

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D114064: [clang] [MinGW] Pass --no-demangle through to the mingw linker

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo created this revision.
mstorsjo added a reviewer: rnk.
mstorsjo requested review of this revision.
Herald added a project: clang.

Clang has custom handling of --no-demangle, where it is removed
from the input -Wl and -Xlinker options, and readded specifically
by the drivers where it's known to be supported.

Both ld.bfd and lld support the --no-demangle option.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D114064

Files:
  clang/lib/Driver/ToolChains/MinGW.cpp
  clang/test/Driver/Xlinker-args.c


Index: clang/test/Driver/Xlinker-args.c
===
--- clang/test/Driver/Xlinker-args.c
+++ clang/test/Driver/Xlinker-args.c
@@ -12,6 +12,11 @@
 // RUN:   -Wl,two,--no-demangle,three -Xlinker four -z five -r %s 2> %t
 // RUN: FileCheck -check-prefix=LINUX < %t %s
 
+/// Check that --no-demangle gets forwarded to the mingw linker
+// RUN: %clang -target x86_64-w64-mingw32 -### \
+// RUN:   -Wl,--no-demangle %s 2> %t
+// RUN: FileCheck -check-prefix=MINGW < %t %s
+
 // RUN: %clang -target powerpc-unknown-aix -### \
 // RUN:   -b one -b two %s 2> %t
 // RUN: FileCheck -check-prefix=AIX < %t %s
@@ -23,6 +28,7 @@
 // DARWIN-NOT: --no-demangle
 // DARWIN: "one" "two" "three" "four" "-z" "five" "-r"
 // LINUX: "--no-demangle" "-e" "_start" "one" "two" "three" "four" "-z" "five" 
"-r" {{.*}} "-T" "a.lds"
+// MINGW: "--no-demangle"
 // AIX: "-b" "one" "-b" "two"
 // NOT-AIX: error: unsupported option '-b' for target 'powerpc-unknown-linux'
 
Index: clang/lib/Driver/ToolChains/MinGW.cpp
===
--- clang/lib/Driver/ToolChains/MinGW.cpp
+++ clang/lib/Driver/ToolChains/MinGW.cpp
@@ -164,6 +164,9 @@
 CmdArgs.push_back("--enable-auto-image-base");
   }
 
+  if (Args.hasArg(options::OPT_Z_Xlinker__no_demangle))
+CmdArgs.push_back("--no-demangle");
+
   CmdArgs.push_back("-o");
   const char *OutputFile = Output.getFilename();
   // GCC implicitly adds an .exe extension if it is given an output file name


Index: clang/test/Driver/Xlinker-args.c
===
--- clang/test/Driver/Xlinker-args.c
+++ clang/test/Driver/Xlinker-args.c
@@ -12,6 +12,11 @@
 // RUN:   -Wl,two,--no-demangle,three -Xlinker four -z five -r %s 2> %t
 // RUN: FileCheck -check-prefix=LINUX < %t %s
 
+/// Check that --no-demangle gets forwarded to the mingw linker
+// RUN: %clang -target x86_64-w64-mingw32 -### \
+// RUN:   -Wl,--no-demangle %s 2> %t
+// RUN: FileCheck -check-prefix=MINGW < %t %s
+
 // RUN: %clang -target powerpc-unknown-aix -### \
 // RUN:   -b one -b two %s 2> %t
 // RUN: FileCheck -check-prefix=AIX < %t %s
@@ -23,6 +28,7 @@
 // DARWIN-NOT: --no-demangle
 // DARWIN: "one" "two" "three" "four" "-z" "five" "-r"
 // LINUX: "--no-demangle" "-e" "_start" "one" "two" "three" "four" "-z" "five" "-r" {{.*}} "-T" "a.lds"
+// MINGW: "--no-demangle"
 // AIX: "-b" "one" "-b" "two"
 // NOT-AIX: error: unsupported option '-b' for target 'powerpc-unknown-linux'
 
Index: clang/lib/Driver/ToolChains/MinGW.cpp
===
--- clang/lib/Driver/ToolChains/MinGW.cpp
+++ clang/lib/Driver/ToolChains/MinGW.cpp
@@ -164,6 +164,9 @@
 CmdArgs.push_back("--enable-auto-image-base");
   }
 
+  if (Args.hasArg(options::OPT_Z_Xlinker__no_demangle))
+CmdArgs.push_back("--no-demangle");
+
   CmdArgs.push_back("-o");
   const char *OutputFile = Output.getFilename();
   // GCC implicitly adds an .exe extension if it is given an output file name
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D112430: [ARM][libunwind] add PACBTI-M support for libunwind

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo accepted this revision.
mstorsjo added a comment.
This revision is now accepted and ready to land.

I guess this is ok then. I can't say I've followed every bit in detail, but it 
looks sensible overall.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112430/new/

https://reviews.llvm.org/D112430

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D109856: [libunwind][ARM] Handle end of stack during unwind

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

Looks reasonable I think. Is this a deficiency in the EHABI implementation 
only, i.e. this aspect works as it should in the regular dwarf implementation?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D109856/new/

https://reviews.llvm.org/D109856

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D112387: [clang] [MinGW] Rename the 'Arch' member to 'SysrootName'. NFC.

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo updated this revision to Diff 382798.
mstorsjo added a comment.

Renamed the variable to `SubdirName`. Will push tomorrow.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112387/new/

https://reviews.llvm.org/D112387

Files:
  clang/lib/Driver/ToolChains/MinGW.cpp
  clang/lib/Driver/ToolChains/MinGW.h

Index: clang/lib/Driver/ToolChains/MinGW.h
===
--- clang/lib/Driver/ToolChains/MinGW.h
+++ clang/lib/Driver/ToolChains/MinGW.h
@@ -99,7 +99,7 @@
   std::string Base;
   std::string GccLibDir;
   std::string Ver;
-  std::string Arch;
+  std::string SubdirName;
   mutable std::unique_ptr Preprocessor;
   mutable std::unique_ptr Compiler;
   void findGccLibDir();
Index: clang/lib/Driver/ToolChains/MinGW.cpp
===
--- clang/lib/Driver/ToolChains/MinGW.cpp
+++ clang/lib/Driver/ToolChains/MinGW.cpp
@@ -349,20 +349,20 @@
 }
 
 void toolchains::MinGW::findGccLibDir() {
-  llvm::SmallVector, 2> Archs;
-  Archs.emplace_back(getTriple().getArchName());
-  Archs[0] += "-w64-mingw32";
-  Archs.emplace_back("mingw32");
-  if (Arch.empty())
-Arch = std::string(Archs[0].str());
+  llvm::SmallVector, 2> SubdirNames;
+  SubdirNames.emplace_back(getTriple().getArchName());
+  SubdirNames[0] += "-w64-mingw32";
+  SubdirNames.emplace_back("mingw32");
+  if (SubdirName.empty())
+SubdirName = std::string(SubdirNames[0].str());
   // lib: Arch Linux, Ubuntu, Windows
   // lib64: openSUSE Linux
   for (StringRef CandidateLib : {"lib", "lib64"}) {
-for (StringRef CandidateArch : Archs) {
+for (StringRef CandidateSysroot : SubdirNames) {
   llvm::SmallString<1024> LibDir(Base);
-  llvm::sys::path::append(LibDir, CandidateLib, "gcc", CandidateArch);
+  llvm::sys::path::append(LibDir, CandidateLib, "gcc", CandidateSysroot);
   if (findGccVersion(LibDir, GccLibDir, Ver)) {
-Arch = std::string(CandidateArch);
+SubdirName = std::string(CandidateSysroot);
 return;
   }
 }
@@ -391,7 +391,7 @@
   StringRef Sep = llvm::sys::path::get_separator();
   for (StringRef CandidateSubdir : Subdirs) {
 if (llvm::sys::fs::is_directory(ClangRoot + Sep + CandidateSubdir)) {
-  Arch = std::string(CandidateSubdir);
+  SubdirName = std::string(CandidateSubdir);
   return (ClangRoot + Sep + CandidateSubdir).str();
 }
   }
@@ -423,10 +423,10 @@
   // correct crtbegin.o ,cetend.o would be found.
   getFilePaths().push_back(GccLibDir);
   getFilePaths().push_back(
-  (Base + Arch + llvm::sys::path::get_separator() + "lib").str());
+  (Base + SubdirName + llvm::sys::path::get_separator() + "lib").str());
   getFilePaths().push_back(Base + "lib");
   // openSUSE
-  getFilePaths().push_back(Base + Arch + "/sys-root/mingw/lib");
+  getFilePaths().push_back(Base + SubdirName + "/sys-root/mingw/lib");
 
   NativeLLVMSupport =
   Args.getLastArgValue(options::OPT_fuse_ld_EQ, CLANG_DEFAULT_LINKER)
@@ -573,11 +573,12 @@
   if (GetRuntimeLibType(DriverArgs) == ToolChain::RLT_Libgcc) {
 // openSUSE
 addSystemInclude(DriverArgs, CC1Args,
- Base + Arch + "/sys-root/mingw/include");
+ Base + SubdirName + "/sys-root/mingw/include");
   }
 
   addSystemInclude(DriverArgs, CC1Args,
-   Base + Arch + llvm::sys::path::get_separator() + "include");
+   Base + SubdirName + llvm::sys::path::get_separator() +
+   "include");
   addSystemInclude(DriverArgs, CC1Args, Base + "include");
 }
 
@@ -596,8 +597,9 @@
 .str();
 if (getDriver().getVFS().exists(TargetDir))
   addSystemInclude(DriverArgs, CC1Args, TargetDir);
-addSystemInclude(DriverArgs, CC1Args, Base + Arch + Slash + "include" +
-  Slash + "c++" + Slash + "v1");
+addSystemInclude(DriverArgs, CC1Args,
+ Base + SubdirName + Slash + "include" + Slash + "c++" +
+ Slash + "v1");
 addSystemInclude(DriverArgs, CC1Args,
  Base + "include" + Slash + "c++" + Slash + "v1");
 break;
@@ -606,9 +608,10 @@
   case ToolChain::CST_Libstdcxx:
 llvm::SmallVector, 4> CppIncludeBases;
 CppIncludeBases.emplace_back(Base);
-llvm::sys::path::append(CppIncludeBases[0], Arch, "include", "c++");
+llvm::sys::path::append(CppIncludeBases[0], SubdirName, "include", "c++");
 CppIncludeBases.emplace_back(Base);
-llvm::sys::path::append(CppIncludeBases[1], Arch, "include", "c++", Ver);
+llvm::sys::path::append(CppIncludeBases[1], SubdirName, "include", "c++",
+Ver);
 CppIncludeBases.emplace_back(Base);
 llvm::sys::path::append(CppIncludeBases[2], "include", "c++", Ver);
 CppIncludeBases.emplace_back(GccLibDir);
@@ -616,7 +619,7 @@
 for (auto  : 

[PATCH] D111952: [clang] [MinGW] Guess the right ix86 arch name spelling as sysroot

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo updated this revision to Diff 382801.
mstorsjo added a comment.

Rebased after renaming `SysrootName` to `SubdirName`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D111952/new/

https://reviews.llvm.org/D111952

Files:
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/MinGW.cpp
  clang/lib/Driver/ToolChains/MinGW.h
  clang/test/Driver/mingw-sysroot.cpp

Index: clang/test/Driver/mingw-sysroot.cpp
===
--- clang/test/Driver/mingw-sysroot.cpp
+++ clang/test/Driver/mingw-sysroot.cpp
@@ -12,6 +12,7 @@
 // RUN: mkdir -p %T/testroot-clang/bin
 // RUN: ln -s %clang %T/testroot-clang/bin/x86_64-w64-mingw32-clang
 // RUN: ln -s %S/Inputs/mingw_ubuntu_posix_tree/usr/x86_64-w64-mingw32 %T/testroot-clang/x86_64-w64-mingw32
+// RUN: ln -s %S/Inputs/mingw_arch_tree/usr/i686-w64-mingw32 %T/testroot-clang/i686-w64-mingw32
 
 
 // If we find a gcc in the path with the right triplet prefix, pick that as
@@ -36,3 +37,14 @@
 // the libgcc directory:
 
 // RUN: env "PATH=%T/testroot-gcc/bin:%PATH%" %T/testroot-gcc/bin/x86_64-w64-mingw32-clang -target x86_64-w64-mingw32 -rtlib=platform -stdlib=libstdc++ --sysroot="" -c -### %s 2>&1 | FileCheck -check-prefix=CHECK_TESTROOT_GCC %s
+
+
+// If the user requests a different arch via the -m32 option, which changes
+// x86_64 into i386, check that the driver notices that it can't find a
+// sysroot for i386 but there is one for i686, and uses that one.
+// (In practice, this usecase is when using an unprefixed native clang
+// that defaults to x86_64 mingw, but it's easier to test in cross setups
+// with symlinks, like the other tests here.)
+
+// RUN: env "PATH=%T/testroot-gcc/bin:%PATH%" %T/testroot-clang/bin/x86_64-w64-mingw32-clang -target x86_64-w64-mingw32 -m32 -rtlib=compiler-rt -stdlib=libstdc++ --sysroot="" -c -### %s 2>&1 | FileCheck -check-prefix=CHECK_TESTROOT_CLANG_I686 %s
+// CHECK_TESTROOT_CLANG_I686: "{{[^"]+}}/testroot-clang{{/|}}i686-w64-mingw32{{/|}}include"
Index: clang/lib/Driver/ToolChains/MinGW.h
===
--- clang/lib/Driver/ToolChains/MinGW.h
+++ clang/lib/Driver/ToolChains/MinGW.h
@@ -60,6 +60,9 @@
   MinGW(const Driver , const llvm::Triple ,
 const llvm::opt::ArgList );
 
+  static void FixTripleArch(const Driver , llvm::Triple ,
+const llvm::opt::ArgList );
+
   bool HasNativeLLVMSupport() const override;
 
   bool IsIntegratedAssemblerDefault() const override;
@@ -103,8 +106,6 @@
   mutable std::unique_ptr Preprocessor;
   mutable std::unique_ptr Compiler;
   void findGccLibDir();
-  llvm::ErrorOr findGcc();
-  llvm::ErrorOr findClangRelativeSysroot();
 
   bool NativeLLVMSupport;
 };
Index: clang/lib/Driver/ToolChains/MinGW.cpp
===
--- clang/lib/Driver/ToolChains/MinGW.cpp
+++ clang/lib/Driver/ToolChains/MinGW.cpp
@@ -369,9 +369,9 @@
   }
 }
 
-llvm::ErrorOr toolchains::MinGW::findGcc() {
+static llvm::ErrorOr findGcc(const llvm::Triple ) {
   llvm::SmallVector, 2> Gccs;
-  Gccs.emplace_back(getTriple().getArchName());
+  Gccs.emplace_back(T.getArchName());
   Gccs[0] += "-w64-mingw32-gcc";
   Gccs.emplace_back("mingw32-gcc");
   // Please do not add "gcc" here
@@ -381,13 +381,14 @@
   return make_error_code(std::errc::no_such_file_or_directory);
 }
 
-llvm::ErrorOr toolchains::MinGW::findClangRelativeSysroot() {
+static llvm::ErrorOr
+findClangRelativeSysroot(const Driver , const llvm::Triple ,
+ std::string ) {
   llvm::SmallVector, 2> Subdirs;
-  Subdirs.emplace_back(getTriple().str());
-  Subdirs.emplace_back(getTriple().getArchName());
+  Subdirs.emplace_back(T.str());
+  Subdirs.emplace_back(T.getArchName());
   Subdirs[1] += "-w64-mingw32";
-  StringRef ClangRoot =
-  llvm::sys::path::parent_path(getDriver().getInstalledDir());
+  StringRef ClangRoot = llvm::sys::path::parent_path(D.getInstalledDir());
   StringRef Sep = llvm::sys::path::get_separator();
   for (StringRef CandidateSubdir : Subdirs) {
 if (llvm::sys::fs::is_directory(ClangRoot + Sep + CandidateSubdir)) {
@@ -404,13 +405,16 @@
   RocmInstallation(D, Triple, Args) {
   getProgramPaths().push_back(getDriver().getInstalledDir());
 
+  // The sequence for detecting a sysroot here should be kept in sync with
+  // the testTriple function below.
   if (getDriver().SysRoot.size())
 Base = getDriver().SysRoot;
   // Look for /../; if found, use /.. as the
   // base as it could still be a base for a gcc setup with libgcc.
-  else if (llvm::ErrorOr TargetSubdir = findClangRelativeSysroot())
+  else if (llvm::ErrorOr TargetSubdir =
+   findClangRelativeSysroot(getDriver(), getTriple(), SubdirName))
 Base = std::string(llvm::sys::path::parent_path(TargetSubdir.get()));
-  else if (llvm::ErrorOr GPPName = findGcc())
+  else if (llvm::ErrorOr 

[PATCH] D112387: [clang] [MinGW] Rename the 'Arch' member to 'SysrootName'. NFC.

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

In D112387#3090707 , @MaskRay wrote:

> How is this different from `clang::driver::Driver::SysRoot`?
>
> From `SysrootName + "/sys-root"` looks like the `sys-root` directory is the 
> sysroot? Perhaps there is a better way describing `SysrootName` but I cannot 
> think of it...
> `SysrootName` may be good enough.

Yeah, it’s kinda vague what it is. It’s mainly a triple, except that it also 
can be “mingw32” or similar, in some cases. I guess it could be `SubdirName` 
too, or something related to that.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112387/new/

https://reviews.llvm.org/D112387

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100756: [llvm-rc] [4/4] Add a GNU windres-like frontend to llvm-rc

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

In D100756#3088035 , @nickdesaulniers 
wrote:

> thanks for this patch @mstorsjo ! This is help AOSP LLVM move our windows 
> builds of LLVM away from MinGW to LLVM! 
> https://android-review.googlesource.com/c/toolchain/llvm_android/+/1867380/

Thanks for letting me know - that’s nice to hear that it benefits you too!

Btw, a little nitpick about naming - MinGW is the SDK/environment that you’re 
still using, but you moved from GNU binutils windres to llvm windres - it’s 
just as little/much mingw as before. :P


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100756/new/

https://reviews.llvm.org/D100756

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D111952: [clang] [MinGW] Guess the right ix86 arch name spelling as sysroot

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo updated this revision to Diff 382946.
mstorsjo marked an inline comment as done.
mstorsjo added a comment.

Fixed the naming of the new function, using `--target=` in the newly added test 
lines, fixed a case of missed clang-format.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D111952/new/

https://reviews.llvm.org/D111952

Files:
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/MinGW.cpp
  clang/lib/Driver/ToolChains/MinGW.h
  clang/test/Driver/mingw-sysroot.cpp

Index: clang/test/Driver/mingw-sysroot.cpp
===
--- clang/test/Driver/mingw-sysroot.cpp
+++ clang/test/Driver/mingw-sysroot.cpp
@@ -12,6 +12,7 @@
 // RUN: mkdir -p %T/testroot-clang/bin
 // RUN: ln -s %clang %T/testroot-clang/bin/x86_64-w64-mingw32-clang
 // RUN: ln -s %S/Inputs/mingw_ubuntu_posix_tree/usr/x86_64-w64-mingw32 %T/testroot-clang/x86_64-w64-mingw32
+// RUN: ln -s %S/Inputs/mingw_arch_tree/usr/i686-w64-mingw32 %T/testroot-clang/i686-w64-mingw32
 
 
 // If we find a gcc in the path with the right triplet prefix, pick that as
@@ -36,3 +37,14 @@
 // the libgcc directory:
 
 // RUN: env "PATH=%T/testroot-gcc/bin:%PATH%" %T/testroot-gcc/bin/x86_64-w64-mingw32-clang -target x86_64-w64-mingw32 -rtlib=platform -stdlib=libstdc++ --sysroot="" -c -### %s 2>&1 | FileCheck -check-prefix=CHECK_TESTROOT_GCC %s
+
+
+// If the user requests a different arch via the -m32 option, which changes
+// x86_64 into i386, check that the driver notices that it can't find a
+// sysroot for i386 but there is one for i686, and uses that one.
+// (In practice, the real usecase is when using an unprefixed native clang
+// that defaults to x86_64 mingw, but it's easier to test this in cross setups
+// with symlinks, like the other tests here.)
+
+// RUN: env "PATH=%T/testroot-gcc/bin:%PATH%" %T/testroot-clang/bin/x86_64-w64-mingw32-clang --target=x86_64-w64-mingw32 -m32 -rtlib=compiler-rt -stdlib=libstdc++ --sysroot="" -c -### %s 2>&1 | FileCheck -check-prefix=CHECK_TESTROOT_CLANG_I686 %s
+// CHECK_TESTROOT_CLANG_I686: "{{[^"]+}}/testroot-clang{{/|}}i686-w64-mingw32{{/|}}include"
Index: clang/lib/Driver/ToolChains/MinGW.h
===
--- clang/lib/Driver/ToolChains/MinGW.h
+++ clang/lib/Driver/ToolChains/MinGW.h
@@ -60,6 +60,9 @@
   MinGW(const Driver , const llvm::Triple ,
 const llvm::opt::ArgList );
 
+  static void fixTripleArch(const Driver , llvm::Triple ,
+const llvm::opt::ArgList );
+
   bool HasNativeLLVMSupport() const override;
 
   bool IsIntegratedAssemblerDefault() const override;
@@ -103,8 +106,6 @@
   mutable std::unique_ptr Preprocessor;
   mutable std::unique_ptr Compiler;
   void findGccLibDir();
-  llvm::ErrorOr findGcc();
-  llvm::ErrorOr findClangRelativeSysroot();
 
   bool NativeLLVMSupport;
 };
Index: clang/lib/Driver/ToolChains/MinGW.cpp
===
--- clang/lib/Driver/ToolChains/MinGW.cpp
+++ clang/lib/Driver/ToolChains/MinGW.cpp
@@ -369,9 +369,9 @@
   }
 }
 
-llvm::ErrorOr toolchains::MinGW::findGcc() {
+static llvm::ErrorOr findGcc(const llvm::Triple ) {
   llvm::SmallVector, 2> Gccs;
-  Gccs.emplace_back(getTriple().getArchName());
+  Gccs.emplace_back(T.getArchName());
   Gccs[0] += "-w64-mingw32-gcc";
   Gccs.emplace_back("mingw32-gcc");
   // Please do not add "gcc" here
@@ -381,13 +381,14 @@
   return make_error_code(std::errc::no_such_file_or_directory);
 }
 
-llvm::ErrorOr toolchains::MinGW::findClangRelativeSysroot() {
+static llvm::ErrorOr
+findClangRelativeSysroot(const Driver , const llvm::Triple ,
+ std::string ) {
   llvm::SmallVector, 2> Subdirs;
-  Subdirs.emplace_back(getTriple().str());
-  Subdirs.emplace_back(getTriple().getArchName());
+  Subdirs.emplace_back(T.str());
+  Subdirs.emplace_back(T.getArchName());
   Subdirs[1] += "-w64-mingw32";
-  StringRef ClangRoot =
-  llvm::sys::path::parent_path(getDriver().getInstalledDir());
+  StringRef ClangRoot = llvm::sys::path::parent_path(D.getInstalledDir());
   StringRef Sep = llvm::sys::path::get_separator();
   for (StringRef CandidateSubdir : Subdirs) {
 if (llvm::sys::fs::is_directory(ClangRoot + Sep + CandidateSubdir)) {
@@ -404,13 +405,16 @@
   RocmInstallation(D, Triple, Args) {
   getProgramPaths().push_back(getDriver().getInstalledDir());
 
+  // The sequence for detecting a sysroot here should be kept in sync with
+  // the testTriple function below.
   if (getDriver().SysRoot.size())
 Base = getDriver().SysRoot;
   // Look for /../; if found, use /.. as the
   // base as it could still be a base for a gcc setup with libgcc.
-  else if (llvm::ErrorOr TargetSubdir = findClangRelativeSysroot())
+  else if (llvm::ErrorOr TargetSubdir =
+   findClangRelativeSysroot(getDriver(), getTriple(), SubdirName))
 Base = 

[PATCH] D112387: [clang] [MinGW] Rename the 'Arch' member to 'SysrootName'. NFC.

[Martin Storsjö via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG897c86dec5af: [clang] [MinGW] Rename the Arch 
member to SubdirName. NFC. (authored by mstorsjo).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112387/new/

https://reviews.llvm.org/D112387

Files:
  clang/lib/Driver/ToolChains/MinGW.cpp
  clang/lib/Driver/ToolChains/MinGW.h

Index: clang/lib/Driver/ToolChains/MinGW.h
===
--- clang/lib/Driver/ToolChains/MinGW.h
+++ clang/lib/Driver/ToolChains/MinGW.h
@@ -99,7 +99,7 @@
   std::string Base;
   std::string GccLibDir;
   std::string Ver;
-  std::string Arch;
+  std::string SubdirName;
   mutable std::unique_ptr Preprocessor;
   mutable std::unique_ptr Compiler;
   void findGccLibDir();
Index: clang/lib/Driver/ToolChains/MinGW.cpp
===
--- clang/lib/Driver/ToolChains/MinGW.cpp
+++ clang/lib/Driver/ToolChains/MinGW.cpp
@@ -349,20 +349,20 @@
 }
 
 void toolchains::MinGW::findGccLibDir() {
-  llvm::SmallVector, 2> Archs;
-  Archs.emplace_back(getTriple().getArchName());
-  Archs[0] += "-w64-mingw32";
-  Archs.emplace_back("mingw32");
-  if (Arch.empty())
-Arch = std::string(Archs[0].str());
+  llvm::SmallVector, 2> SubdirNames;
+  SubdirNames.emplace_back(getTriple().getArchName());
+  SubdirNames[0] += "-w64-mingw32";
+  SubdirNames.emplace_back("mingw32");
+  if (SubdirName.empty())
+SubdirName = std::string(SubdirNames[0].str());
   // lib: Arch Linux, Ubuntu, Windows
   // lib64: openSUSE Linux
   for (StringRef CandidateLib : {"lib", "lib64"}) {
-for (StringRef CandidateArch : Archs) {
+for (StringRef CandidateSysroot : SubdirNames) {
   llvm::SmallString<1024> LibDir(Base);
-  llvm::sys::path::append(LibDir, CandidateLib, "gcc", CandidateArch);
+  llvm::sys::path::append(LibDir, CandidateLib, "gcc", CandidateSysroot);
   if (findGccVersion(LibDir, GccLibDir, Ver)) {
-Arch = std::string(CandidateArch);
+SubdirName = std::string(CandidateSysroot);
 return;
   }
 }
@@ -391,7 +391,7 @@
   StringRef Sep = llvm::sys::path::get_separator();
   for (StringRef CandidateSubdir : Subdirs) {
 if (llvm::sys::fs::is_directory(ClangRoot + Sep + CandidateSubdir)) {
-  Arch = std::string(CandidateSubdir);
+  SubdirName = std::string(CandidateSubdir);
   return (ClangRoot + Sep + CandidateSubdir).str();
 }
   }
@@ -423,10 +423,10 @@
   // correct crtbegin.o ,cetend.o would be found.
   getFilePaths().push_back(GccLibDir);
   getFilePaths().push_back(
-  (Base + Arch + llvm::sys::path::get_separator() + "lib").str());
+  (Base + SubdirName + llvm::sys::path::get_separator() + "lib").str());
   getFilePaths().push_back(Base + "lib");
   // openSUSE
-  getFilePaths().push_back(Base + Arch + "/sys-root/mingw/lib");
+  getFilePaths().push_back(Base + SubdirName + "/sys-root/mingw/lib");
 
   NativeLLVMSupport =
   Args.getLastArgValue(options::OPT_fuse_ld_EQ, CLANG_DEFAULT_LINKER)
@@ -573,11 +573,12 @@
   if (GetRuntimeLibType(DriverArgs) == ToolChain::RLT_Libgcc) {
 // openSUSE
 addSystemInclude(DriverArgs, CC1Args,
- Base + Arch + "/sys-root/mingw/include");
+ Base + SubdirName + "/sys-root/mingw/include");
   }
 
   addSystemInclude(DriverArgs, CC1Args,
-   Base + Arch + llvm::sys::path::get_separator() + "include");
+   Base + SubdirName + llvm::sys::path::get_separator() +
+   "include");
   addSystemInclude(DriverArgs, CC1Args, Base + "include");
 }
 
@@ -596,8 +597,9 @@
 .str();
 if (getDriver().getVFS().exists(TargetDir))
   addSystemInclude(DriverArgs, CC1Args, TargetDir);
-addSystemInclude(DriverArgs, CC1Args, Base + Arch + Slash + "include" +
-  Slash + "c++" + Slash + "v1");
+addSystemInclude(DriverArgs, CC1Args,
+ Base + SubdirName + Slash + "include" + Slash + "c++" +
+ Slash + "v1");
 addSystemInclude(DriverArgs, CC1Args,
  Base + "include" + Slash + "c++" + Slash + "v1");
 break;
@@ -606,9 +608,10 @@
   case ToolChain::CST_Libstdcxx:
 llvm::SmallVector, 4> CppIncludeBases;
 CppIncludeBases.emplace_back(Base);
-llvm::sys::path::append(CppIncludeBases[0], Arch, "include", "c++");
+llvm::sys::path::append(CppIncludeBases[0], SubdirName, "include", "c++");
 CppIncludeBases.emplace_back(Base);
-llvm::sys::path::append(CppIncludeBases[1], Arch, "include", "c++", Ver);
+llvm::sys::path::append(CppIncludeBases[1], SubdirName, "include", "c++",
+Ver);
 CppIncludeBases.emplace_back(Base);
 llvm::sys::path::append(CppIncludeBases[2], "include", "c++", Ver);
 

[PATCH] D112289: Support: Use sys::path::is_style_{posix,windows}() in a few places

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo accepted this revision.
mstorsjo added a comment.
This revision is now accepted and ready to land.

LGTM, too. D112786  does a couple more things 
that do pretty much the same - I don't mind if you want to fold them into this, 
or keep it as-is, with the bits that are closer to separator handling split out 
for clarity.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112289/new/

https://reviews.llvm.org/D112289

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D111952: [clang] [MinGW] Guess the right ix86 arch name spelling as sysroot

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added inline comments.



Comment at: clang/lib/Driver/ToolChains/MinGW.h:63
 
+  static void FixTripleArch(const Driver , llvm::Triple ,
+const llvm::opt::ArgList );

MaskRay wrote:
> Use `functionName` for newer functions. (It's unfortunate that Clang has much 
> inconsistency but we should do the right thing for new methods.)
Sure, will change it.



Comment at: clang/test/Driver/mingw-sysroot.cpp:49
+
+// RUN: env "PATH=%T/testroot-gcc/bin:%PATH%" 
%T/testroot-clang/bin/x86_64-w64-mingw32-clang -target x86_64-w64-mingw32 -m32 
-rtlib=compiler-rt -stdlib=libstdc++ --sysroot="" -c -### %s 2>&1 | FileCheck 
-check-prefix=CHECK_TESTROOT_CLANG_I686 %s
+// CHECK_TESTROOT_CLANG_I686: 
"{{[^"]+}}/testroot-clang{{/|}}i686-w64-mingw32{{/|}}include"

MaskRay wrote:
> Changing `PATH` seems unreliable for some test environments.
> 
> Any alternative way to test this?
> 
> `--target=` is preferred over `-target ` for driver options. `-target ` is 
> legacy but we probably need to keep indefinitely for its wide usage.
> 
> `linux-cross.cpp` has some nice tests which may inspire this file ? :)
Using `PATH` isn't very nice, no, but it's at least reliable enough to run on 
the current testing setups?

I don't see any good input for testing this in linux-cross.cpp - that file uses 
explicit `--sysroot=` parameters all over the place, while this test file is 
mostly about how to best implicitly deduce the sysroot when one isn't specified.

This file (both the old and the newly added tests) relies on checking what 
directories it finds in `/../`, so it requires 
fake-installing the clang binary by symlinking into a test toolchain tree.

I guess it could be possible to execute that fake-installed clang binary by 
specifying the direct path to it, instead of adding it to `PATH` - but it's an 
important usecase to make sure that it works when the literal path isn't 
spelled out in `argv[0]` too.

I can change to use `--target=` in the added lines, but the rest of the file 
uses the other spelling anyway (and I don't think it's on topic to do such 
rewrites as part of this patch).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D111952/new/

https://reviews.llvm.org/D111952

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D113269: [clang-move] Fix unit tests with forward slash as separator on windows

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added inline comments.



Comment at: clang-tools-extra/unittests/clang-move/ClangMoveTests.cpp:235
 
- // std::string IncludeArg = Twine("-I" + WorkingDir;
+ // std::string IncludeArg = Twine("-I" + Dir;
   tooling::runToolOnCodeWithArgs(

aaron.ballman wrote:
> Might as well remove this comment entirely? I don't think it adds any value, 
> it looks like debug code that was left in by accident.
Sure, I can remove it.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113269/new/

https://reviews.llvm.org/D113269

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D113264: [clang] [DirectoryWatcher] Remove leading \\?\ from GetFinalPathNameByHandleW

[Martin Storsjö via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG2ca6fc34fc08: [clang] [DirectoryWatcher] Remove leading \\?\ 
from GetFinalPathNameByHandleW (authored by mstorsjo).

Changed prior to commit:
  https://reviews.llvm.org/D113264?vs=385025=385591#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113264/new/

https://reviews.llvm.org/D113264

Files:
  clang/lib/DirectoryWatcher/windows/DirectoryWatcher-windows.cpp


Index: clang/lib/DirectoryWatcher/windows/DirectoryWatcher-windows.cpp
===
--- clang/lib/DirectoryWatcher/windows/DirectoryWatcher-windows.cpp
+++ clang/lib/DirectoryWatcher/windows/DirectoryWatcher-windows.cpp
@@ -88,10 +88,15 @@
   // handle to the watcher and performing synchronous operations.
   {
 DWORD Size = GetFinalPathNameByHandleW(DirectoryHandle, NULL, 0, 0);
-std::unique_ptr Buffer{new WCHAR[Size]};
+std::unique_ptr Buffer{new WCHAR[Size + 1]};
 Size = GetFinalPathNameByHandleW(DirectoryHandle, Buffer.get(), Size, 0);
 Buffer[Size] = L'\0';
-llvm::sys::windows::UTF16ToUTF8(Buffer.get(), Size, Path);
+WCHAR *Data = Buffer.get();
+if (Size >= 4 && ::memcmp(Data, L"?\\", 8) == 0) {
+  Data += 4;
+  Size -= 4;
+}
+llvm::sys::windows::UTF16ToUTF8(Data, Size, Path);
   }
 
   size_t EntrySize = sizeof(FILE_NOTIFY_INFORMATION) + MAX_PATH * 
sizeof(WCHAR);


Index: clang/lib/DirectoryWatcher/windows/DirectoryWatcher-windows.cpp
===
--- clang/lib/DirectoryWatcher/windows/DirectoryWatcher-windows.cpp
+++ clang/lib/DirectoryWatcher/windows/DirectoryWatcher-windows.cpp
@@ -88,10 +88,15 @@
   // handle to the watcher and performing synchronous operations.
   {
 DWORD Size = GetFinalPathNameByHandleW(DirectoryHandle, NULL, 0, 0);
-std::unique_ptr Buffer{new WCHAR[Size]};
+std::unique_ptr Buffer{new WCHAR[Size + 1]};
 Size = GetFinalPathNameByHandleW(DirectoryHandle, Buffer.get(), Size, 0);
 Buffer[Size] = L'\0';
-llvm::sys::windows::UTF16ToUTF8(Buffer.get(), Size, Path);
+WCHAR *Data = Buffer.get();
+if (Size >= 4 && ::memcmp(Data, L"?\\", 8) == 0) {
+  Data += 4;
+  Size -= 4;
+}
+llvm::sys::windows::UTF16ToUTF8(Data, Size, Path);
   }
 
   size_t EntrySize = sizeof(FILE_NOTIFY_INFORMATION) + MAX_PATH * sizeof(WCHAR);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D113269: [clang-move] Fix unit tests with forward slash as separator on windows

[Martin Storsjö via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG98f0bf74ca6d: [clang-move] Fix unit tests with forward slash 
as separator on windows (authored by mstorsjo).

Changed prior to commit:
  https://reviews.llvm.org/D113269?vs=385035=385592#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113269/new/

https://reviews.llvm.org/D113269

Files:
  clang-tools-extra/unittests/clang-move/ClangMoveTests.cpp


Index: clang-tools-extra/unittests/clang-move/ClangMoveTests.cpp
===
--- clang-tools-extra/unittests/clang-move/ClangMoveTests.cpp
+++ clang-tools-extra/unittests/clang-move/ClangMoveTests.cpp
@@ -208,7 +208,9 @@
DeclarationReporter *const Reporter = nullptr) {
   clang::RewriterTestContext Context;
 
-  Context.InMemoryFileSystem->setCurrentWorkingDirectory(WorkingDir);
+  llvm::SmallString<16> Dir(WorkingDir);
+  llvm::sys::path::native(Dir);
+  Context.InMemoryFileSystem->setCurrentWorkingDirectory(Dir);
 
   std::map FileToFileID;
 
@@ -224,13 +226,12 @@
   CreateFiles(TestCCName, CC);
 
   std::map FileToReplacements;
-  ClangMoveContext MoveContext = {Spec, FileToReplacements, WorkingDir, "LLVM",
+  ClangMoveContext MoveContext = {Spec, FileToReplacements, Dir.c_str(), 
"LLVM",
   Reporter != nullptr};
 
   auto Factory = std::make_unique(
   , Reporter);
 
- // std::string IncludeArg = Twine("-I" + WorkingDir;
   tooling::runToolOnCodeWithArgs(
   Factory->create(), CC, Context.InMemoryFileSystem,
   {"-std=c++11", "-fparse-all-comments", "-I."}, TestCCName, "clang-move",


Index: clang-tools-extra/unittests/clang-move/ClangMoveTests.cpp
===
--- clang-tools-extra/unittests/clang-move/ClangMoveTests.cpp
+++ clang-tools-extra/unittests/clang-move/ClangMoveTests.cpp
@@ -208,7 +208,9 @@
DeclarationReporter *const Reporter = nullptr) {
   clang::RewriterTestContext Context;
 
-  Context.InMemoryFileSystem->setCurrentWorkingDirectory(WorkingDir);
+  llvm::SmallString<16> Dir(WorkingDir);
+  llvm::sys::path::native(Dir);
+  Context.InMemoryFileSystem->setCurrentWorkingDirectory(Dir);
 
   std::map FileToFileID;
 
@@ -224,13 +226,12 @@
   CreateFiles(TestCCName, CC);
 
   std::map FileToReplacements;
-  ClangMoveContext MoveContext = {Spec, FileToReplacements, WorkingDir, "LLVM",
+  ClangMoveContext MoveContext = {Spec, FileToReplacements, Dir.c_str(), "LLVM",
   Reporter != nullptr};
 
   auto Factory = std::make_unique(
   , Reporter);
 
- // std::string IncludeArg = Twine("-I" + WorkingDir;
   tooling::runToolOnCodeWithArgs(
   Factory->create(), CC, Context.InMemoryFileSystem,
   {"-std=c++11", "-fparse-all-comments", "-I."}, TestCCName, "clang-move",
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D113268: [clangd] Fix tests with forward slash as separator on windows

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a subscriber: rnk.
mstorsjo added a comment.

FWIW, this change is not about mingw, it's about making the 
windows-with-forward-slashes configuration usable.

The windows-with-forward-slashes configuration works just as fine in MSVC 
configurations, and that's where I've tested it. A MSVC build with `ninja 
check-all` that succeeded before, still succeed with `-DLLVM_ 
WINDOWS_PREFER_FORWARD_SLASH=ON` (with respect to clangd) after this change.

As for testing strategy, it could be concievable to add a configuration to e.g. 
the buildkite premerge tests that build+test everything in a 
`-DLLVM_WINDOWS_PREFER_FORWARD_SLASH=ON` environment too. (I've seen interest 
from @rnk to push towards such a setup, possibly, so adding testing 
configurations for both modes is probably not inconcievable.)




Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:546
 fs::make_absolute(TestScheme::TestDir, Path);
+path::native(Path);
 return std::string(Path);

sammccall wrote:
> This change is an example of something subtle that I don't understand, and 
> don't expect other maintainers to understand, which is therefore fragile.
Hmm, I'll have to revisit this particular change and see if it really was 
necessary in the end, and add a comment explaining the subtlety.



Comment at: clang-tools-extra/clangd/unittests/TestFS.h:79
+llvm::SmallString<16> testRootBuf();
+#define testRoot() testRootBuf().c_str()
 

sammccall wrote:
> If we want to make this more broadly compatible, we'd need to finder 
> cleaner/less invasive ways...
Sure, this is admittedly a bit hacky, but managed to make tests pass with a 
fairly minimal amount of changes.



Comment at: clang-tools-extra/clangd/unittests/URITests.cpp:137
+  llvm::SmallString<16> Ref("c:\\x\\y\\z");
+  llvm::sys::path::native(Ref);
+  EXPECT_THAT(resolveOrDie(parseOrDie("file:///c%3a/x/y/z")), Ref);

sammccall wrote:
> This seems incorrect. Any build running on windows should be able to handle 
> backslash paths read from compile_commands.json etc.
> 
> 
Yes, but that's not what the changes does. Regardless of the preference for 
forward or backwards slashes, both configurations (at least when it comes to 
LLVMSupport general functions) support both types of paths, that's not changed.

Previously, this test verified that if you resolve `file:///c%a/x/y/z` you get 
`c:\\x\\y\\z`. Now in a forward slash environment, you get `c:/x/y/z` - which 
is expected.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113268/new/

https://reviews.llvm.org/D113268

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D113268: [clangd] Fix tests with forward slash as separator on windows

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

I forgot to add - I agree with the general sentiment though, that it’s not 
worth bending over backwards to make tests pass in a setup that’s known not to 
work in real use though.

Then again, if someone were to want to actually step up to make it work in 
mingw setups, I guess that shouldn’t be blocked either (although I can’t say I 
have the bandwidth to take on that - I’m not currently a user of clangd).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113268/new/

https://reviews.llvm.org/D113268

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D113268: [clangd] Fix tests with forward slash as separator on windows

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

In D113268#3111798 , @sammccall wrote:

> In D113268#3111709 , @mstorsjo 
> wrote:
>
>> FWIW, this change is not about mingw, it's about making the 
>> windows-with-forward-slashes configuration usable.
>
> OK - can I ask why the windows-with-forward-slashes configuration is valuable 
> to support?
> I was assuming it was mostly useful to folks building with mingw, sounds like 
> that's not all!

Well you're right that my own original need is for mingw setups, with various 
tools parsing and using the output of e.g. `clang -v` and `clang 
--print-resource-dir` etc, and using it in contexts where backslashes require 
extra care. I'm not familiar with all the other cases that others have had that 
have indicated a need for this though, but it's something that does come up 
semi-regularly, and as Windows itself mostly accepts this alternative form, 
it's a setup with possibly "less sharp corners".

>> The windows-with-forward-slashes configuration works just as fine in MSVC 
>> configurations, and that's where I've tested it. A MSVC build with `ninja 
>> check-all` that succeeded before, still succeed with `-DLLVM_ 
>> WINDOWS_PREFER_FORWARD_SLASH=ON` (with respect to clangd) after this change.
>>
>> As for testing strategy, it could be concievable to add a configuration to 
>> e.g. the buildkite premerge tests that build+test everything in a 
>> `-DLLVM_WINDOWS_PREFER_FORWARD_SLASH=ON` environment too. (I've seen 
>> interest from @rnk to push towards such a setup, possibly, so adding testing 
>> configurations for both modes is probably not inconcievable.)
>
> That's great to hear. It'd really be a prerequisite to keeping this 
> configuration passing, as we don't have regular access to windows machines & 
> rely on the buildbots to catch test problems.
>
> I think the biggest testing problem we have though is that we don't have a 
> good way of defining integration tests with paths set up the right way (since 
> we can't really use absolute paths in our tests, and most of our bugs involve 
> comparing absolute paths).

Yup, I can totally understand that. And as I haven't tested clangd in this 
setup other than the unit tests, I'm not sure if it works entirely in practice 
though - with other unit tests in Clang I've seen lots of cases where e.g. 
paths aren't considered as in the same directory when paths are expressed with 
different separators.

For someone unfamiliar with the tool, is there a simple "smoke test procedure" 
I could try out with it to kick the tyres?

Anyway, I'll have a look at the seemingly odd/fragile change and get back to 
you on that.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113268/new/

https://reviews.llvm.org/D113268

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D113268: [clangd] Fix tests with forward slash as separator on windows

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

Oh, I realized that issues relating to “mingw” probably are from msys (which 
often is used together, but is an entirely different thing) - yeah I can see 
that there’d be lots of unfixable issues with that, and I’m not signing up for 
looking into that…

Msys is the unix emulation layer/runtime, which uses unix style paths like 
`/c/Windows`. Mingw is just normal plain win32 with slightly different tools.

This patch and the new option is just about `C:/Windows` vs `C:\Windows`. Most 
win32 apis (and llvm in general) accept both forms, the new option is just 
about which one of the two is generated when llvm/clang generate paths.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113268/new/

https://reviews.llvm.org/D113268

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D111199: [Clang][LLVM][Attr] support btf_type_tag attribute

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

I went ahead and reverted this, as it caused crashes when compiling a number of 
projects. The most reduced testcase is this:

  $ cat reduced.c 
  void a(*);
  void a() {}
  $ clang -c reduced.c -O2 -g

A full case (which reduces into this) is this, 
https://martin.st/temp/iconv-preproc.c, built with `clang -target 
i686-w64-mingw32 -w -c -O2 -g iconv-preproc.c`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D99/new/

https://reviews.llvm.org/D99

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D113254: [clang] Fix a misadjusted path style comparison in a unittest

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo created this revision.
mstorsjo added a reviewer: dexonsmith.
mstorsjo requested review of this revision.
Herald added a project: clang.

This was changed incorrectly by accident in
99023627010bbfefb71e25a2b4d056de1cbd354e 
.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D113254

Files:
  clang/unittests/Driver/ToolChainTest.cpp


Index: clang/unittests/Driver/ToolChainTest.cpp
===
--- clang/unittests/Driver/ToolChainTest.cpp
+++ clang/unittests/Driver/ToolChainTest.cpp
@@ -151,7 +151,7 @@
 llvm::raw_string_ostream OS(S);
 C->getDefaultToolChain().printVerboseInfo(OS);
   }
-  if (is_style_windows(llvm::sys::path::Style::windows))
+  if (is_style_windows(llvm::sys::path::Style::native))
 std::replace(S.begin(), S.end(), '\\', '/');
   EXPECT_EQ("Found candidate GCC installation: "
 "/home/test/bin/../lib/gcc/arm-linux-gnueabi/4.6.1\n"


Index: clang/unittests/Driver/ToolChainTest.cpp
===
--- clang/unittests/Driver/ToolChainTest.cpp
+++ clang/unittests/Driver/ToolChainTest.cpp
@@ -151,7 +151,7 @@
 llvm::raw_string_ostream OS(S);
 C->getDefaultToolChain().printVerboseInfo(OS);
   }
-  if (is_style_windows(llvm::sys::path::Style::windows))
+  if (is_style_windows(llvm::sys::path::Style::native))
 std::replace(S.begin(), S.end(), '\\', '/');
   EXPECT_EQ("Found candidate GCC installation: "
 "/home/test/bin/../lib/gcc/arm-linux-gnueabi/4.6.1\n"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D113268: [clangd] Fix tests with forward slash as separator on windows

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo created this revision.
mstorsjo added reviewers: sammccall, aaron.ballman.
Herald added subscribers: usaxena95, kadircet, arphaman.
mstorsjo requested review of this revision.
Herald added subscribers: MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

If running on Windows with a build configured to prefer forward
slashes, these changes fix the tests.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D113268

Files:
  clang-tools-extra/clangd/tool/ClangdMain.cpp
  clang-tools-extra/clangd/unittests/TestFS.cpp
  clang-tools-extra/clangd/unittests/TestFS.h
  clang-tools-extra/clangd/unittests/URITests.cpp


Index: clang-tools-extra/clangd/unittests/URITests.cpp
===
--- clang-tools-extra/clangd/unittests/URITests.cpp
+++ clang-tools-extra/clangd/unittests/URITests.cpp
@@ -133,8 +133,10 @@
 
 TEST(URITest, Resolve) {
 #ifdef _WIN32
-  EXPECT_THAT(resolveOrDie(parseOrDie("file:///c%3a/x/y/z")), "c:\\x\\y\\z");
-  EXPECT_THAT(resolveOrDie(parseOrDie("file:///c:/x/y/z")), "c:\\x\\y\\z");
+  llvm::SmallString<16> Ref("c:\\x\\y\\z");
+  llvm::sys::path::native(Ref);
+  EXPECT_THAT(resolveOrDie(parseOrDie("file:///c%3a/x/y/z")), Ref);
+  EXPECT_THAT(resolveOrDie(parseOrDie("file:///c:/x/y/z")), Ref);
 #else
   EXPECT_EQ(resolveOrDie(parseOrDie("file:/a/b/c")), "/a/b/c");
   EXPECT_EQ(resolveOrDie(parseOrDie("file://auth/a/b/c")), "//auth/a/b/c");
@@ -148,13 +150,17 @@
 
 TEST(URITest, ResolveUNC) {
 #ifdef _WIN32
+  llvm::SmallString<32> RefExample("example.com\\x\\y\\z");
+  llvm::sys::path::native(RefExample);
+  llvm::SmallString<16> RefLocalhost("127.0.0.1\\x\\y\\z");
+  llvm::sys::path::native(RefLocalhost);
   EXPECT_THAT(resolveOrDie(parseOrDie("file://example.com/x/y/z")),
-  "example.com\\x\\y\\z");
+  RefExample);
   EXPECT_THAT(resolveOrDie(parseOrDie("file://127.0.0.1/x/y/z")),
-  "127.0.0.1\\x\\y\\z");
+  RefLocalhost);
   // Ensure non-traditional file URI still resolves to correct UNC path.
   EXPECT_THAT(resolveOrDie(parseOrDie("file:127.0.0.1/x/y/z")),
-  "127.0.0.1\\x\\y\\z");
+  RefLocalhost);
 #else
   EXPECT_THAT(resolveOrDie(parseOrDie("file://example.com/x/y/z")),
   "//example.com/x/y/z");
Index: clang-tools-extra/clangd/unittests/TestFS.h
===
--- clang-tools-extra/clangd/unittests/TestFS.h
+++ clang-tools-extra/clangd/unittests/TestFS.h
@@ -75,7 +75,8 @@
 };
 
 // Returns an absolute (fake) test directory for this OS.
-const char *testRoot();
+llvm::SmallString<16> testRootBuf();
+#define testRoot() testRootBuf().c_str()
 
 // Returns a suitable absolute path for this OS.
 std::string testPath(PathRef File,
Index: clang-tools-extra/clangd/unittests/TestFS.cpp
===
--- clang-tools-extra/clangd/unittests/TestFS.cpp
+++ clang-tools-extra/clangd/unittests/TestFS.cpp
@@ -71,11 +71,14 @@
   FileName, std::move(CommandLine), "")};
 }
 
-const char *testRoot() {
+llvm::SmallString<16> testRootBuf() {
 #ifdef _WIN32
-  return "C:\\clangd-test";
+  llvm::SmallString<16> NativeRoot("C:\\clangd-test");
+  llvm::sys::path::native(NativeRoot);
+  return NativeRoot;
 #else
-  return "/clangd-test";
+  llvm::SmallString<16> Root("/clangd-test");
+  return Root;
 #endif
 }
 
Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -542,15 +542,17 @@
   Body);
 Body = Body.ltrim('/');
 llvm::SmallString<16> Path(Body);
-path::native(Path);
 fs::make_absolute(TestScheme::TestDir, Path);
+path::native(Path);
 return std::string(Path);
   }
 
   llvm::Expected
   uriFromAbsolutePath(llvm::StringRef AbsolutePath) const override {
 llvm::StringRef Body = AbsolutePath;
-if (!Body.consume_front(TestScheme::TestDir))
+llvm::SmallString<16> NativeTestDir(TestDir);
+llvm::sys::path::native(NativeTestDir);
+if (!Body.consume_front(NativeTestDir))
   return error("Path {0} doesn't start with root {1}", AbsolutePath,
TestDir);
 


Index: clang-tools-extra/clangd/unittests/URITests.cpp
===
--- clang-tools-extra/clangd/unittests/URITests.cpp
+++ clang-tools-extra/clangd/unittests/URITests.cpp
@@ -133,8 +133,10 @@
 
 TEST(URITest, Resolve) {
 #ifdef _WIN32
-  EXPECT_THAT(resolveOrDie(parseOrDie("file:///c%3a/x/y/z")), "c:\\x\\y\\z");
-  EXPECT_THAT(resolveOrDie(parseOrDie("file:///c:/x/y/z")), "c:\\x\\y\\z");
+  llvm::SmallString<16> Ref("c:\\x\\y\\z");
+  llvm::sys::path::native(Ref);
+  EXPECT_THAT(resolveOrDie(parseOrDie("file:///c%3a/x/y/z")), Ref);
+  

[PATCH] D113264: [clang] [DirectoryWatcher] Remove leading \\?\ from GetFinalPathNameByHandleW

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo created this revision.
mstorsjo added a reviewer: aaron.ballman.
mstorsjo requested review of this revision.
Herald added a project: clang.

This is needed for the paths to work when using forward slashes;
this fixes the DirectoryWatcherTests unit tests.

Also allocate missing space for the null terminator, which seems to
have been missing all along (writing the terminator out of bounds).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D113264

Files:
  clang/lib/DirectoryWatcher/windows/DirectoryWatcher-windows.cpp


Index: clang/lib/DirectoryWatcher/windows/DirectoryWatcher-windows.cpp
===
--- clang/lib/DirectoryWatcher/windows/DirectoryWatcher-windows.cpp
+++ clang/lib/DirectoryWatcher/windows/DirectoryWatcher-windows.cpp
@@ -88,10 +88,15 @@
   // handle to the watcher and performing synchronous operations.
   {
 DWORD Size = GetFinalPathNameByHandleW(DirectoryHandle, NULL, 0, 0);
-std::unique_ptr Buffer{new WCHAR[Size]};
+std::unique_ptr Buffer{new WCHAR[Size+1]};
 Size = GetFinalPathNameByHandleW(DirectoryHandle, Buffer.get(), Size, 0);
 Buffer[Size] = L'\0';
-llvm::sys::windows::UTF16ToUTF8(Buffer.get(), Size, Path);
+WCHAR *Data = Buffer.get();
+if (Size >= 4 && ::memcmp(Data, L"?\\", 8) == 0) {
+  Data += 4;
+  Size -= 4;
+}
+llvm::sys::windows::UTF16ToUTF8(Data, Size, Path);
   }
 
   size_t EntrySize = sizeof(FILE_NOTIFY_INFORMATION) + MAX_PATH * 
sizeof(WCHAR);


Index: clang/lib/DirectoryWatcher/windows/DirectoryWatcher-windows.cpp
===
--- clang/lib/DirectoryWatcher/windows/DirectoryWatcher-windows.cpp
+++ clang/lib/DirectoryWatcher/windows/DirectoryWatcher-windows.cpp
@@ -88,10 +88,15 @@
   // handle to the watcher and performing synchronous operations.
   {
 DWORD Size = GetFinalPathNameByHandleW(DirectoryHandle, NULL, 0, 0);
-std::unique_ptr Buffer{new WCHAR[Size]};
+std::unique_ptr Buffer{new WCHAR[Size+1]};
 Size = GetFinalPathNameByHandleW(DirectoryHandle, Buffer.get(), Size, 0);
 Buffer[Size] = L'\0';
-llvm::sys::windows::UTF16ToUTF8(Buffer.get(), Size, Path);
+WCHAR *Data = Buffer.get();
+if (Size >= 4 && ::memcmp(Data, L"?\\", 8) == 0) {
+  Data += 4;
+  Size -= 4;
+}
+llvm::sys::windows::UTF16ToUTF8(Data, Size, Path);
   }
 
   size_t EntrySize = sizeof(FILE_NOTIFY_INFORMATION) + MAX_PATH * sizeof(WCHAR);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D113269: [clang-move] Fix unit tests with forward slash as separator on windows

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo created this revision.
mstorsjo added reviewers: aaron.ballman, sammccall.
mstorsjo requested review of this revision.
Herald added a project: clang-tools-extra.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D113269

Files:
  clang-tools-extra/unittests/clang-move/ClangMoveTests.cpp


Index: clang-tools-extra/unittests/clang-move/ClangMoveTests.cpp
===
--- clang-tools-extra/unittests/clang-move/ClangMoveTests.cpp
+++ clang-tools-extra/unittests/clang-move/ClangMoveTests.cpp
@@ -208,7 +208,9 @@
DeclarationReporter *const Reporter = nullptr) {
   clang::RewriterTestContext Context;
 
-  Context.InMemoryFileSystem->setCurrentWorkingDirectory(WorkingDir);
+  llvm::SmallString<16> Dir(WorkingDir);
+  llvm::sys::path::native(Dir);
+  Context.InMemoryFileSystem->setCurrentWorkingDirectory(Dir);
 
   std::map FileToFileID;
 
@@ -224,13 +226,13 @@
   CreateFiles(TestCCName, CC);
 
   std::map FileToReplacements;
-  ClangMoveContext MoveContext = {Spec, FileToReplacements, WorkingDir, "LLVM",
+  ClangMoveContext MoveContext = {Spec, FileToReplacements, Dir.c_str(), 
"LLVM",
   Reporter != nullptr};
 
   auto Factory = std::make_unique(
   , Reporter);
 
- // std::string IncludeArg = Twine("-I" + WorkingDir;
+ // std::string IncludeArg = Twine("-I" + Dir;
   tooling::runToolOnCodeWithArgs(
   Factory->create(), CC, Context.InMemoryFileSystem,
   {"-std=c++11", "-fparse-all-comments", "-I."}, TestCCName, "clang-move",


Index: clang-tools-extra/unittests/clang-move/ClangMoveTests.cpp
===
--- clang-tools-extra/unittests/clang-move/ClangMoveTests.cpp
+++ clang-tools-extra/unittests/clang-move/ClangMoveTests.cpp
@@ -208,7 +208,9 @@
DeclarationReporter *const Reporter = nullptr) {
   clang::RewriterTestContext Context;
 
-  Context.InMemoryFileSystem->setCurrentWorkingDirectory(WorkingDir);
+  llvm::SmallString<16> Dir(WorkingDir);
+  llvm::sys::path::native(Dir);
+  Context.InMemoryFileSystem->setCurrentWorkingDirectory(Dir);
 
   std::map FileToFileID;
 
@@ -224,13 +226,13 @@
   CreateFiles(TestCCName, CC);
 
   std::map FileToReplacements;
-  ClangMoveContext MoveContext = {Spec, FileToReplacements, WorkingDir, "LLVM",
+  ClangMoveContext MoveContext = {Spec, FileToReplacements, Dir.c_str(), "LLVM",
   Reporter != nullptr};
 
   auto Factory = std::make_unique(
   , Reporter);
 
- // std::string IncludeArg = Twine("-I" + WorkingDir;
+ // std::string IncludeArg = Twine("-I" + Dir;
   tooling::runToolOnCodeWithArgs(
   Factory->create(), CC, Context.InMemoryFileSystem,
   {"-std=c++11", "-fparse-all-comments", "-I."}, TestCCName, "clang-move",
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D113268: [clangd] Fix tests with forward slash as separator on windows

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added inline comments.



Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:546
 fs::make_absolute(TestScheme::TestDir, Path);
+path::native(Path);
 return std::string(Path);

mstorsjo wrote:
> sammccall wrote:
> > This change is an example of something subtle that I don't understand, and 
> > don't expect other maintainers to understand, which is therefore fragile.
> Hmm, I'll have to revisit this particular change and see if it really was 
> necessary in the end, and add a comment explaining the subtlety.
Ah, yes, now I remember what this particular change does:

`TestScheme::TestDir` is a haredcoded constant string; 
`fs::make_absolute(TestScheme::TestDir, Path);` concatenates the 
always-backslashes `TestDir` with `Path` (with the currently preferred 
separator inbetween). By calling `native()` afterwards, it converts the whole 
resulting path to the native form.

An alternative would be to provide `TestDir` in the preferred form (e.g. like 
wrapping it in a function that returns a variable string, like in 
TestFS.{h,cpp}.) - that'd maybe be more straightforward, but also feels a bit 
needless as we're calling `native()` on it anyway.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113268/new/

https://reviews.llvm.org/D113268

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D111457: [clang][test] Add lit helper for windows paths

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a subscriber: rnk.
mstorsjo added a comment.

In D111457#3085273 , @keith wrote:

> Yep this makes sense. I would rather not scope that into this if that's ok. 
> Since this test was already invalid and broken (actual fix in 
> https://reviews.llvm.org/D111579) I would rather land these and then take 
> that on to unblock my original use case (https://reviews.llvm.org/D111587) if 
> I can find some more time

Ok, fair enough - I guess that sounds reasonable, so that'd be a +1 from me to 
this patch. Ideally it'd be good to find someone more authoritative around 
clang and these areas than me, but I guess reviewer attention is scarce and 
hard to come by. @rnk - does this seem sensible to you?

Let's give it a little more time for someone else to comment on it, otherwise I 
guess I could approve it...

Then for the future cleanup, one might be able to stop using the native 
substitutions, by keeping one test entirely using possibly-foreign posix style 
forward slashes, and one using the windows style paths (with that test possibly 
limited to only executed when on windows, if necessary - but being able to map 
things to a windows path while cross compiling is good too).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D111457/new/

https://reviews.llvm.org/D111457

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D111457: [clang][test] Add lit helper for windows paths

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

In D111457#3073726 , @keith wrote:

> In D111457#3066508 , @mstorsjo 
> wrote:
>
>> Wouldn't this one also be solved pretty much the same, but differently, by 
>> changing `if (llvm::sys::path::is_absolute(RemappedFile)) {` into 
>> `is_absolute_gnu`?
>
> I think it //could// but also users could still remap to native window's 
> paths, so likely we'd want this test as well I think either way?

I guess this might be a good addition as a new test, yeah, but I think it would 
be good to keep this test as is too, and change the code to use 
`is_absolute_gnu` (and fix up the test reference here to expect an empty 
directory in the output).

>> Since we're remapping debug paths, it's plausible that the target path can 
>> be a different style (when cross compiling, where debug prefix remapping is 
>> kinda important), and then it's probably good to use a more lax definition 
>> of whether a path is absolute. (Alternatively, we maybe should try to detect 
>> the kind of path used and use the appropriate style as argument to 
>> `is_absolute`, but I don't think that necessarily helps here.)
>
> Good point here, I could definitely see wanting to support the entire matrix 
> of host windows paths vs not, and target windows paths vs not, but I think 
> that would require significantly more changes for other places that call 
> `llvm::sys::path::*` APIs and also use the default `native` argument (I'm not 
> sure how difficult this would be, but it would require a bit of auditing)

I would expect that to mostly work so far, except for corner cases like these. 
Auditing probably doesn't hurt if one wants to spend the effort, otherwise I'd 
just expect it to work and try it out and see if one runs into any issues 
somewhere, if someone has such a usecase.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D111457/new/

https://reviews.llvm.org/D111457

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D111952: [clang] [MinGW] Guess the right ix86 arch name spelling as sysroot

[Martin Storsjö via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd758069f5e0d: [clang] [MinGW] Guess the right ix86 arch name 
spelling as sysroot (authored by mstorsjo).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D111952/new/

https://reviews.llvm.org/D111952

Files:
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/MinGW.cpp
  clang/lib/Driver/ToolChains/MinGW.h
  clang/test/Driver/mingw-sysroot.cpp

Index: clang/test/Driver/mingw-sysroot.cpp
===
--- clang/test/Driver/mingw-sysroot.cpp
+++ clang/test/Driver/mingw-sysroot.cpp
@@ -12,6 +12,7 @@
 // RUN: mkdir -p %T/testroot-clang/bin
 // RUN: ln -s %clang %T/testroot-clang/bin/x86_64-w64-mingw32-clang
 // RUN: ln -s %S/Inputs/mingw_ubuntu_posix_tree/usr/x86_64-w64-mingw32 %T/testroot-clang/x86_64-w64-mingw32
+// RUN: ln -s %S/Inputs/mingw_arch_tree/usr/i686-w64-mingw32 %T/testroot-clang/i686-w64-mingw32
 
 
 // If we find a gcc in the path with the right triplet prefix, pick that as
@@ -36,3 +37,14 @@
 // the libgcc directory:
 
 // RUN: env "PATH=%T/testroot-gcc/bin:%PATH%" %T/testroot-gcc/bin/x86_64-w64-mingw32-clang -target x86_64-w64-mingw32 -rtlib=platform -stdlib=libstdc++ --sysroot="" -c -### %s 2>&1 | FileCheck -check-prefix=CHECK_TESTROOT_GCC %s
+
+
+// If the user requests a different arch via the -m32 option, which changes
+// x86_64 into i386, check that the driver notices that it can't find a
+// sysroot for i386 but there is one for i686, and uses that one.
+// (In practice, the real usecase is when using an unprefixed native clang
+// that defaults to x86_64 mingw, but it's easier to test this in cross setups
+// with symlinks, like the other tests here.)
+
+// RUN: env "PATH=%T/testroot-gcc/bin:%PATH%" %T/testroot-clang/bin/x86_64-w64-mingw32-clang --target=x86_64-w64-mingw32 -m32 -rtlib=compiler-rt -stdlib=libstdc++ --sysroot="" -c -### %s 2>&1 | FileCheck -check-prefix=CHECK_TESTROOT_CLANG_I686 %s
+// CHECK_TESTROOT_CLANG_I686: "{{[^"]+}}/testroot-clang{{/|}}i686-w64-mingw32{{/|}}include"
Index: clang/lib/Driver/ToolChains/MinGW.h
===
--- clang/lib/Driver/ToolChains/MinGW.h
+++ clang/lib/Driver/ToolChains/MinGW.h
@@ -60,6 +60,9 @@
   MinGW(const Driver , const llvm::Triple ,
 const llvm::opt::ArgList );
 
+  static void fixTripleArch(const Driver , llvm::Triple ,
+const llvm::opt::ArgList );
+
   bool HasNativeLLVMSupport() const override;
 
   bool IsIntegratedAssemblerDefault() const override;
@@ -103,8 +106,6 @@
   mutable std::unique_ptr Preprocessor;
   mutable std::unique_ptr Compiler;
   void findGccLibDir();
-  llvm::ErrorOr findGcc();
-  llvm::ErrorOr findClangRelativeSysroot();
 
   bool NativeLLVMSupport;
 };
Index: clang/lib/Driver/ToolChains/MinGW.cpp
===
--- clang/lib/Driver/ToolChains/MinGW.cpp
+++ clang/lib/Driver/ToolChains/MinGW.cpp
@@ -369,9 +369,9 @@
   }
 }
 
-llvm::ErrorOr toolchains::MinGW::findGcc() {
+static llvm::ErrorOr findGcc(const llvm::Triple ) {
   llvm::SmallVector, 2> Gccs;
-  Gccs.emplace_back(getTriple().getArchName());
+  Gccs.emplace_back(T.getArchName());
   Gccs[0] += "-w64-mingw32-gcc";
   Gccs.emplace_back("mingw32-gcc");
   // Please do not add "gcc" here
@@ -381,13 +381,14 @@
   return make_error_code(std::errc::no_such_file_or_directory);
 }
 
-llvm::ErrorOr toolchains::MinGW::findClangRelativeSysroot() {
+static llvm::ErrorOr
+findClangRelativeSysroot(const Driver , const llvm::Triple ,
+ std::string ) {
   llvm::SmallVector, 2> Subdirs;
-  Subdirs.emplace_back(getTriple().str());
-  Subdirs.emplace_back(getTriple().getArchName());
+  Subdirs.emplace_back(T.str());
+  Subdirs.emplace_back(T.getArchName());
   Subdirs[1] += "-w64-mingw32";
-  StringRef ClangRoot =
-  llvm::sys::path::parent_path(getDriver().getInstalledDir());
+  StringRef ClangRoot = llvm::sys::path::parent_path(D.getInstalledDir());
   StringRef Sep = llvm::sys::path::get_separator();
   for (StringRef CandidateSubdir : Subdirs) {
 if (llvm::sys::fs::is_directory(ClangRoot + Sep + CandidateSubdir)) {
@@ -404,13 +405,16 @@
   RocmInstallation(D, Triple, Args) {
   getProgramPaths().push_back(getDriver().getInstalledDir());
 
+  // The sequence for detecting a sysroot here should be kept in sync with
+  // the testTriple function below.
   if (getDriver().SysRoot.size())
 Base = getDriver().SysRoot;
   // Look for /../; if found, use /.. as the
   // base as it could still be a base for a gcc setup with libgcc.
-  else if (llvm::ErrorOr TargetSubdir = findClangRelativeSysroot())
+  else if (llvm::ErrorOr TargetSubdir =
+   findClangRelativeSysroot(getDriver(), getTriple(), SubdirName))
 Base = 

[PATCH] D113254: [clang] Fix a misadjusted path style comparison in a unittest

[Martin Storsjö via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG120d44d1a00b: [clang] Fix a misadjusted path style 
comparison in a unittest (authored by mstorsjo).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113254/new/

https://reviews.llvm.org/D113254

Files:
  clang/unittests/Driver/ToolChainTest.cpp


Index: clang/unittests/Driver/ToolChainTest.cpp
===
--- clang/unittests/Driver/ToolChainTest.cpp
+++ clang/unittests/Driver/ToolChainTest.cpp
@@ -151,7 +151,7 @@
 llvm::raw_string_ostream OS(S);
 C->getDefaultToolChain().printVerboseInfo(OS);
   }
-  if (is_style_windows(llvm::sys::path::Style::windows))
+  if (is_style_windows(llvm::sys::path::Style::native))
 std::replace(S.begin(), S.end(), '\\', '/');
   EXPECT_EQ("Found candidate GCC installation: "
 "/home/test/bin/../lib/gcc/arm-linux-gnueabi/4.6.1\n"


Index: clang/unittests/Driver/ToolChainTest.cpp
===
--- clang/unittests/Driver/ToolChainTest.cpp
+++ clang/unittests/Driver/ToolChainTest.cpp
@@ -151,7 +151,7 @@
 llvm::raw_string_ostream OS(S);
 C->getDefaultToolChain().printVerboseInfo(OS);
   }
-  if (is_style_windows(llvm::sys::path::Style::windows))
+  if (is_style_windows(llvm::sys::path::Style::native))
 std::replace(S.begin(), S.end(), '\\', '/');
   EXPECT_EQ("Found candidate GCC installation: "
 "/home/test/bin/../lib/gcc/arm-linux-gnueabi/4.6.1\n"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D113254: [clang] Fix a misadjusted path style comparison in a unittest

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

Yeah I guess I should have just committed this as trivial. Anyway, it doesn't 
seem to have had any notable impact on anything - luckily.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113254/new/

https://reviews.llvm.org/D113254

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D115441: [X86][MS] Add 80bit long double support for Windows

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

In D115441#3187009 , @rnk wrote:

> I seem to recall assuming that Windows `long double` was 64-bits in many, 
> many places. Unfortunately, I have no idea where that could've happened.

Nothing comes to mind for me about that - in _most_ cases, Windows is kinda 
oblivious to `long double`, as nothing in Windows public API uses that type.

However outside of the core OS, any function in the CRT, that uses long 
doubles, is going to be wrong; in the C99 runtime, there's plenty of `long 
double` functions - a separate `-l` suffixed version of most math functions, 
but also more important conversion functions like `strtold`.

> Something that comes to mind immediately is the MSVC name mangler. I don't 
> think that's a blocking issue, but it's something you should be aware of if 
> you want to promote this flag's usage.

Oh, right, I have no familiarity with those aspects and what might break there.

> @mstorsjo, can you advise what GCC does here? I've forgotten how this is 
> supposed to work.

In GCC on Windows (and clang in mingw mode), `long double` is always 80 bit on 
x86. (On i386, `sizeof(long double) == 12`, while on x86_64 it's 16.)

Regarding the initial FPU state, I think the statically linked CRT startup bits 
differ from MSVC in this aspect, so the x87 state is initialized in 80 bit mode.

Then for runtime functions, mingw handles this by providing their own 
(statically linked) reimplementation of essentially all functions that touch 
long doubles. For math and similar, it's pretty straightforward, but for 
printf, it's a bit of a mess since we'd otherwise want to use UCRT's (otherwise 
standards compliant) printfs, but whenever long doubles are involved (very 
rarely in practice, but libc++'s testsuite do exercise them) the mingw provided 
version has to be used.

For mingw on arm (32 and 64) I haven't wanted to introduce any further deviance 
from MSVC, so there it's all identical to plain double.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115441/new/

https://reviews.llvm.org/D115441

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D115441: [X86][MS] Add 80bit long double support for Windows

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

In D115441#3188172 , @pengfei wrote:

>> In GCC on Windows (and clang in mingw mode), long double is always 80 bit on 
>> x86. (On i386, sizeof(long double) == 12, while on x86_64 it's 16.)
>
> How about the alignment? I can see on the i386 Linux case, the alignment is 
> 4, I assume it is also 4 for GCC on Windows, right?

Yes, it's 4 for i386 in GCC on Windows too (and Clang in mingw mode). For 
x86_64, both sizeof and alignof are 16.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115441/new/

https://reviews.llvm.org/D115441

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D115045: [Clang] Ignore CLANG_DEFAULT_LINKER for custom-linker toolchains

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

In D115045#3175648 , @phosek wrote:

> Rather, we shouldn't consider `-DCLANG_DEFAULT_LINKER=lld` as a valid value. 
> Instead AMDGPU bot should use `-DCLANG_DEFAULT_LINKER=ld.lld`.

I think this would be a kinda disruptive change; I (and others) specify it as 
`-DCLANG_DEFAULT_LINKER=lld` so far: 
https://github.com/mstorsjo/llvm-mingw/blob/master/build-llvm.sh#L162 And other 
projects too: 
https://github.com/msys2/MINGW-packages/blob/master/mingw-w64-clang/PKGBUILD#L204


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115045/new/

https://reviews.llvm.org/D115045

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D106721: [AArch64] Implemnt MSVC __mulh and __umulh builtins and corresponding IR level intrinsics

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo created this revision.
mstorsjo added reviewers: rnk, STL_MSFT, efriedma, DavidSpickett.
Herald added subscribers: danielkiss, hiraditya, kristof.beyls.
mstorsjo requested review of this revision.
Herald added projects: clang, LLVM.

This seems to work, but would it need more testing for anything else
than just the most trivial happy path cases, and any other tests than
these? (I tried to look for some of the existing tests for MSVC ARM64
intrinsics, but they're all very sparingly tested.)

This should fix PR51128.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D106721

Files:
  clang/include/clang/Basic/BuiltinsAArch64.def
  clang/lib/Headers/intrin.h
  clang/test/CodeGen/arm64-microsoft-intrinsics.c
  llvm/include/llvm/IR/IntrinsicsAArch64.td
  llvm/lib/Target/AArch64/AArch64InstrInfo.td
  llvm/test/CodeGen/AArch64/intrinsics-mulh.ll

Index: llvm/test/CodeGen/AArch64/intrinsics-mulh.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AArch64/intrinsics-mulh.ll
@@ -0,0 +1,21 @@
+; RUN: llc < %s -mtriple=aarch64-eabi -O=3 | FileCheck %s
+
+
+define i64 @check_smulh(i64 %a, i64 %b) {
+entry:
+  ; CHECK: smulh x0, x0, x1
+  %0 = tail call i64 @llvm.aarch64.smulh(i64 %a, i64 %b)
+  ret i64 %0
+} 
+
+define i64 @check_umulh(i64 %a, i64 %b) {
+entry:
+  ; CHECK: umulh x0, x0, x1
+  %0 = tail call i64 @llvm.aarch64.umulh(i64 %a, i64 %b)
+  ret i64 %0
+} 
+
+
+declare i64 @llvm.aarch64.smulh(i64, i64)
+declare i64 @llvm.aarch64.umulh(i64, i64)
+
Index: llvm/lib/Target/AArch64/AArch64InstrInfo.td
===
--- llvm/lib/Target/AArch64/AArch64InstrInfo.td
+++ llvm/lib/Target/AArch64/AArch64InstrInfo.td
@@ -1709,6 +1709,8 @@
 // Multiply-high
 def SMULHrr : MulHi<0b010, "smulh", mulhs>;
 def UMULHrr : MulHi<0b110, "umulh", mulhu>;
+def : Pat<(int_aarch64_smulh GPR64:$Rn, GPR64:$Rm), (SMULHrr GPR64:$Rn, GPR64:$Rm)>;
+def : Pat<(int_aarch64_umulh GPR64:$Rn, GPR64:$Rm), (UMULHrr GPR64:$Rn, GPR64:$Rm)>;
 
 // CRC32
 def CRC32Brr : BaseCRC32<0, 0b00, 0, GPR32, int_aarch64_crc32b, "crc32b">;
Index: llvm/include/llvm/IR/IntrinsicsAArch64.td
===
--- llvm/include/llvm/IR/IntrinsicsAArch64.td
+++ llvm/include/llvm/IR/IntrinsicsAArch64.td
@@ -34,6 +34,13 @@
 
 def int_aarch64_clrex : Intrinsic<[]>;
 
+def int_aarch64_smulh : MSBuiltin<"__mulh">,
+  Intrinsic<[llvm_i64_ty], [llvm_i64_ty, llvm_i64_ty],
+[IntrNoMem, IntrWillReturn]>;
+def int_aarch64_umulh : MSBuiltin<"__umulh">,
+  Intrinsic<[llvm_i64_ty], [llvm_i64_ty, llvm_i64_ty],
+[IntrNoMem, IntrWillReturn]>;
+
 def int_aarch64_sdiv : DefaultAttrsIntrinsic<[llvm_anyint_ty], [LLVMMatchType<0>,
 LLVMMatchType<0>], [IntrNoMem]>;
 def int_aarch64_udiv : DefaultAttrsIntrinsic<[llvm_anyint_ty], [LLVMMatchType<0>,
Index: clang/test/CodeGen/arm64-microsoft-intrinsics.c
===
--- clang/test/CodeGen/arm64-microsoft-intrinsics.c
+++ clang/test/CodeGen/arm64-microsoft-intrinsics.c
@@ -81,6 +81,20 @@
 // CHECK-MSVC: fence syncscope("singlethread")
 // CHECK-LINUX: error: implicit declaration of function '_ReadWriteBarrier'
 
+long long check_mulh(long long a, long long b) {
+  return __mulh(a, b);
+}
+
+// CHECK-MSVC: call i64 @llvm.aarch64.smulh(i64 %[[PARAM1:.*]], i64 %[[PARAM2:.*]])
+// CHECK-LINUX: error: implicit declaration of function '__mulh'
+
+unsigned long long check_umulh(unsigned long long a, unsigned long long b) {
+  return __umulh(a, b);
+}
+
+// CHECK-MSVC: call i64 @llvm.aarch64.umulh(i64 %[[PARAM3:.*]], i64 %[[PARAM4:.*]])
+// CHECK-LINUX: error: implicit declaration of function '__umulh'
+
 unsigned __int64 check__getReg() {
   unsigned volatile __int64 reg;
   reg = __getReg(18);
Index: clang/lib/Headers/intrin.h
===
--- clang/lib/Headers/intrin.h
+++ clang/lib/Headers/intrin.h
@@ -574,6 +574,9 @@
 unsigned short __cdecl _byteswap_ushort(unsigned short val);
 unsigned long __cdecl _byteswap_ulong (unsigned long val);
 unsigned __int64 __cdecl _byteswap_uint64(unsigned __int64 val);
+
+__int64 __mulh(__int64 __a, __int64 __b);
+unsigned __int64 __umulh(unsigned __int64 __a, unsigned __int64 __b);
 #endif
 
 /**\
Index: clang/include/clang/Basic/BuiltinsAArch64.def
===
--- clang/include/clang/Basic/BuiltinsAArch64.def
+++ clang/include/clang/Basic/BuiltinsAArch64.def
@@ -243,6 +243,9 @@
 TARGET_HEADER_BUILTIN(_WriteStatusReg, "viLLi", "nh", "intrin.h", ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(_AddressOfReturnAddress, "v*", "nh", "intrin.h", ALL_MS_LANGUAGES, "")
 

[PATCH] D104601: [Preprocessor] Implement -fminimize-whitespace.

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

This broke some cases for me, where the output is missing some newlines, 
breaking the output badly. I’ll try to provide a repro…


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104601/new/

https://reviews.llvm.org/D104601

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D104601: [Preprocessor] Implement -fminimize-whitespace.

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

In D104601#2906588 , @mstorsjo wrote:

> This broke some cases for me, where the output is missing some newlines, 
> breaking the output badly. I’ll try to provide a repro…

https://martin.st/temp/repro.tar.xz

clang -target x86_64-windows-gnu -E -P _mkerrcodes.h -Iinclude

The output (at the end) has missing newlines (e.g. around GPG_ERR_ETXTBSY).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104601/new/

https://reviews.llvm.org/D104601

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D106924: [Preprocessor] -E -P: Ensure newline after 8 skipped lines.

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

Thanks! I can confirm that this fixes the issue I ran into.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D106924/new/

https://reviews.llvm.org/D106924

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116503: [clang] Add --start-no-unused-arguments/--end-no-unused-arguments to silence some unused argument warnings

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo updated this revision to Diff 397719.
mstorsjo marked 2 inline comments as done.
mstorsjo added a comment.

Add testcases for using the new option with clang-cl, use `--target=` 
everywhere.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116503/new/

https://reviews.llvm.org/D116503

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/diagnostics.c

Index: clang/test/Driver/diagnostics.c
===
--- clang/test/Driver/diagnostics.c
+++ clang/test/Driver/diagnostics.c
@@ -1,9 +1,53 @@
 // Parse diagnostic arguments in the driver
-// PR12181
 
-// RUN: not %clang -target x86_64-apple-darwin10 \
-// RUN:   -fsyntax-only -fzyzzybalubah \
-// RUN:   -Werror=unused-command-line-argument %s
+// Exactly which arguments are warned about and which aren't differ based
+// on what target is selected. -stdlib= and -fuse-ld= emit diagnostics when
+// compiling C code, for e.g. *-linux-gnu. Linker inputs, like -lfoo, emit
+// diagnostics when only compiling for all targets.
 
-// RUN: not %clang -target x86_64-apple-darwin10 \
-// RUN:   -fsyntax-only -fzyzzybalubah -Werror %s
+// This is normally a non-fatal warning:
+// RUN: %clang --target=x86_64-apple-darwin10 \
+// RUN:   -fsyntax-only -lfoo %s 2>&1 | FileCheck %s
+
+// Either with a specific -Werror=unused.. or a blanket -Werror, this
+// causes the command to fail.
+// RUN: not %clang --target=x86_64-apple-darwin10 \
+// RUN:   -fsyntax-only -lfoo \
+// RUN:   -Werror=unused-command-line-argument %s 2>&1 | FileCheck %s
+
+// RUN: not %clang --target=x86_64-apple-darwin10 \
+// RUN:   -fsyntax-only -lfoo -Werror %s 2>&1 | FileCheck %s
+
+// With a specific -Wno-..., no diagnostic should be printed.
+// RUN: %clang --target=x86_64-apple-darwin10 \
+// RUN:   -fsyntax-only -lfoo -Werror \
+// RUN:   -Wno-unused-command-line-argument %s 2>&1 | count 0
+
+// With -Qunused-arguments, no diagnostic should be printed.
+// RUN: %clang --target=x86_64-apple-darwin10 \
+// RUN:   -fsyntax-only -lfoo -Werror \
+// RUN:   -Qunused-arguments %s 2>&1 | count 0
+
+// With the argument enclosed in --{start,end}-no-unused-arguments,
+// there's no diagnostic.
+// RUN: %clang --target=x86_64-apple-darwin10 -fsyntax-only \
+// RUN:   --start-no-unused-arguments -lfoo --end-no-unused-arguments \
+// RUN:   -Werror %s 2>&1 | count 0
+
+// With --{start,end}-no-unused-argument around a different argument, it
+// still warns about the unused argument.
+// RUN: not %clang --target=x86_64-apple-darwin10 \
+// RUN:   --start-no-unused-arguments -fsyntax-only --end-no-unused-arguments \
+// RUN:   -lfoo -Werror %s 2>&1 | FileCheck %s
+
+// Test clang-cl warning about unused linker options.
+// RUN: not %clang_cl -fsyntax-only /WX %s \
+// RUN:   -link 2>&1 | FileCheck %s --check-prefix=CL-WARNING
+
+// Test clang-cl ignoring the warning with --start-no-unused-arguments.
+// RUN: %clang_cl -fsyntax-only /WX %s \
+// RUN:   --start-no-unused-arguments -link --end-no-unused-arguments 2>&1 | count 0
+
+// CHECK: -lfoo: 'linker' input unused
+
+// CL-WARNING: argument unused during compilation: '-link'
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -367,7 +367,20 @@
   bool HasNostdlib = Args.hasArg(options::OPT_nostdlib);
   bool HasNostdlibxx = Args.hasArg(options::OPT_nostdlibxx);
   bool HasNodefaultlib = Args.hasArg(options::OPT_nodefaultlibs);
+  bool IgnoreUnused = false;
   for (Arg *A : Args) {
+if (IgnoreUnused)
+  A->claim();
+
+if (A->getOption().matches(options::OPT_start_no_unused_arguments)) {
+  IgnoreUnused = true;
+  continue;
+}
+if (A->getOption().matches(options::OPT_end_no_unused_arguments)) {
+  IgnoreUnused = false;
+  continue;
+}
+
 // Unfortunately, we have to parse some forwarding options (-Xassembler,
 // -Xlinker, -Xpreprocessor) because we either integrate their functionality
 // (assembler and preprocessor), or bypass a previous driver ('collect2').
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -1076,6 +1076,8 @@
 def emit_merged_ifs : Flag<["-"], "emit-merged-ifs">,
   Flags<[CC1Option]>, Group,
   HelpText<"Generate Interface Stub Files, emit merged text not binary.">;
+def end_no_unused_arguments : Flag<["--"], "end-no-unused-arguments">, Flags<[CoreOption]>,
+  HelpText<"Start emitting warnings for unused driver arguments">;
 def interface_stub_version_EQ : JoinedOrSeparate<["-"], "interface-stub-version=">, Flags<[CC1Option]>;
 def exported__symbols__list : Separate<["-"], "exported_symbols_list">;
 def e : JoinedOrSeparate<["-"], "e">, 

[PATCH] D116503: [clang] Add --start-no-unused-arguments/--end-no-unused-arguments to silence some unused argument warnings

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

In D116503#3223094 , @MaskRay wrote:

> But make sure to wait a bit to see what others think.

Do you think it's ok to go ahead and land this change now, or should I wait a 
bit more? (I'd like to have it landed with enough margin before the 14.x branch 
so that it has time to settle before that.)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116503/new/

https://reviews.llvm.org/D116503

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116919: [AST] Add RParen loc for decltype AutoTypeloc.

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

Ok, now I managed to make a standalone reproducer: 
https://martin.st/temp/pch-preproc.hxx and 
https://martin.st/temp/main-preproc.cpp

  $ bin/clang -target x86_64-w64-mingw32 -w -x c++-header -c pch-preproc.hxx -o 
pch-preproc.hxx.pch -std=c++17 -O3
  $ bin/clang -target x86_64-w64-mingw32 -w -Xclang -include-pch -Xclang 
pch-preproc.hxx.pch -c main-preproc.cpp -std=c++17 -O3


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116919/new/

https://reviews.llvm.org/D116919

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116919: [AST] Add RParen loc for decltype AutoTypeloc.

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

The relanded version in rG41fbdfa4d5601cccbcdc0ded8ef35190d502f7f3 
 seems to 
be breaking some builds with PCH for me, failing asserts like this:

  clang: ../tools/clang/include/clang/Basic/SourceLocation.h:135: 
clang::SourceLocation 
clang::SourceLocation::getLocWithOffset(clang::SourceLocation::IntTy) const: 
Assertion `((getOffset()+Offset) & MacroIDBit) == 0 && "offset overflow"' 
failed.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116919/new/

https://reviews.llvm.org/D116919

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116503: [clang] Add --start-no-unused-arguments/--end-no-unused-arguments to silence some unused argument warnings

[Martin Storsjö via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG50ec1306d060: [clang] Add 
--start-no-unused-arguments/--end-no-unused-arguments to silence… (authored by 
mstorsjo).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116503/new/

https://reviews.llvm.org/D116503

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/diagnostics.c

Index: clang/test/Driver/diagnostics.c
===
--- clang/test/Driver/diagnostics.c
+++ clang/test/Driver/diagnostics.c
@@ -1,9 +1,53 @@
 // Parse diagnostic arguments in the driver
-// PR12181
 
-// RUN: not %clang -target x86_64-apple-darwin10 \
-// RUN:   -fsyntax-only -fzyzzybalubah \
-// RUN:   -Werror=unused-command-line-argument %s
+// Exactly which arguments are warned about and which aren't differ based
+// on what target is selected. -stdlib= and -fuse-ld= emit diagnostics when
+// compiling C code, for e.g. *-linux-gnu. Linker inputs, like -lfoo, emit
+// diagnostics when only compiling for all targets.
 
-// RUN: not %clang -target x86_64-apple-darwin10 \
-// RUN:   -fsyntax-only -fzyzzybalubah -Werror %s
+// This is normally a non-fatal warning:
+// RUN: %clang --target=x86_64-apple-darwin10 \
+// RUN:   -fsyntax-only -lfoo %s 2>&1 | FileCheck %s
+
+// Either with a specific -Werror=unused.. or a blanket -Werror, this
+// causes the command to fail.
+// RUN: not %clang --target=x86_64-apple-darwin10 \
+// RUN:   -fsyntax-only -lfoo \
+// RUN:   -Werror=unused-command-line-argument %s 2>&1 | FileCheck %s
+
+// RUN: not %clang --target=x86_64-apple-darwin10 \
+// RUN:   -fsyntax-only -lfoo -Werror %s 2>&1 | FileCheck %s
+
+// With a specific -Wno-..., no diagnostic should be printed.
+// RUN: %clang --target=x86_64-apple-darwin10 \
+// RUN:   -fsyntax-only -lfoo -Werror \
+// RUN:   -Wno-unused-command-line-argument %s 2>&1 | count 0
+
+// With -Qunused-arguments, no diagnostic should be printed.
+// RUN: %clang --target=x86_64-apple-darwin10 \
+// RUN:   -fsyntax-only -lfoo -Werror \
+// RUN:   -Qunused-arguments %s 2>&1 | count 0
+
+// With the argument enclosed in --{start,end}-no-unused-arguments,
+// there's no diagnostic.
+// RUN: %clang --target=x86_64-apple-darwin10 -fsyntax-only \
+// RUN:   --start-no-unused-arguments -lfoo --end-no-unused-arguments \
+// RUN:   -Werror %s 2>&1 | count 0
+
+// With --{start,end}-no-unused-argument around a different argument, it
+// still warns about the unused argument.
+// RUN: not %clang --target=x86_64-apple-darwin10 \
+// RUN:   --start-no-unused-arguments -fsyntax-only --end-no-unused-arguments \
+// RUN:   -lfoo -Werror %s 2>&1 | FileCheck %s
+
+// Test clang-cl warning about unused linker options.
+// RUN: not %clang_cl -fsyntax-only /WX %s \
+// RUN:   -link 2>&1 | FileCheck %s --check-prefix=CL-WARNING
+
+// Test clang-cl ignoring the warning with --start-no-unused-arguments.
+// RUN: %clang_cl -fsyntax-only /WX %s \
+// RUN:   --start-no-unused-arguments -link --end-no-unused-arguments 2>&1 | count 0
+
+// CHECK: -lfoo: 'linker' input unused
+
+// CL-WARNING: argument unused during compilation: '-link'
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -367,7 +367,20 @@
   bool HasNostdlib = Args.hasArg(options::OPT_nostdlib);
   bool HasNostdlibxx = Args.hasArg(options::OPT_nostdlibxx);
   bool HasNodefaultlib = Args.hasArg(options::OPT_nodefaultlibs);
+  bool IgnoreUnused = false;
   for (Arg *A : Args) {
+if (IgnoreUnused)
+  A->claim();
+
+if (A->getOption().matches(options::OPT_start_no_unused_arguments)) {
+  IgnoreUnused = true;
+  continue;
+}
+if (A->getOption().matches(options::OPT_end_no_unused_arguments)) {
+  IgnoreUnused = false;
+  continue;
+}
+
 // Unfortunately, we have to parse some forwarding options (-Xassembler,
 // -Xlinker, -Xpreprocessor) because we either integrate their functionality
 // (assembler and preprocessor), or bypass a previous driver ('collect2').
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -1076,6 +1076,8 @@
 def emit_merged_ifs : Flag<["-"], "emit-merged-ifs">,
   Flags<[CC1Option]>, Group,
   HelpText<"Generate Interface Stub Files, emit merged text not binary.">;
+def end_no_unused_arguments : Flag<["--"], "end-no-unused-arguments">, Flags<[CoreOption]>,
+  HelpText<"Start emitting warnings for unused driver arguments">;
 def interface_stub_version_EQ : JoinedOrSeparate<["-"], "interface-stub-version=">, Flags<[CC1Option]>;
 def exported__symbols__list : 

[PATCH] D116919: [AST] Add RParen loc for decltype AutoTypeloc.

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

In D116919#3247887 , @hokein wrote:

> Thanks, everyone!
>
> I manage to figure out the cause, the crash was caused by an arbitrary 
> RPareLoc -- we missed to set the RPareLoc in 
> `TreeTransform::TransformAutoType`. I reland the patch in 
> ab3f100bec03d72ecee947a323c51698d4b95207 
> .

I rebuilt my full, non-reduced testcase now, and it seems like it built fine 
this time. Thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116919/new/

https://reviews.llvm.org/D116919

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D121687: [clang-tidy] Don't try to build CTTestTidyModule for Windows with dylibs

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo created this revision.
mstorsjo added a reviewer: rnk.
Herald added subscribers: xazax.hun, mgorny.
Herald added a project: All.
mstorsjo requested review of this revision.
Herald added a project: clang-tools-extra.

In MinGW mode, it's possible to build LLVM/Clang with
LLVM_LINK_LLVM_DYLIB (which implicitly enables plugins too). Other
existing ways of building plugins on Windows is to build with
LLVM_EXPORT_SYMBOLS_FOR_PLUGINS, where each executable exports its
symbols.

With LLVM_LINK_LLVM_DYLIB, we can't generally skip building plugins
even if they are set up with PLUGIN_TOOL, as some plugins (e.g.
under clang/examples) set up that way do build properly (as
they manually call clang_target_link_libraries, which links in the
libclang-cpp.dll dylib).

For CTTestTidyModule, there's no corresponding dylib that would
provide the same exports.

Alternatively, we could make the condition
`if (NOT WIN32 OR LLVM_EXPORT_SYMBOLS_FOR_PLUGINS)`, as that's the
only way this plugin would be possible to link on Windows.

(Currently, building this plugin with `LLVM_EXPORT_SYMBOLS_FOR_PLUGINS`
fails to link one symbol though, but in principle, it could
work.)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D121687

Files:
  clang-tools-extra/test/CMakeLists.txt


Index: clang-tools-extra/test/CMakeLists.txt
===
--- clang-tools-extra/test/CMakeLists.txt
+++ clang-tools-extra/test/CMakeLists.txt
@@ -81,11 +81,13 @@
 endforeach()
 
 if (NOT LLVM_INSTALL_TOOLCHAIN_ONLY)
-  llvm_add_library(
-  CTTestTidyModule
-  MODULE clang-tidy/CTTestTidyModule.cpp
-  PLUGIN_TOOL clang-tidy
-  DEPENDS clang-tidy-headers)
+  if (NOT WIN32 AND NOT LLVM_LINK_LLVM_DYLIB)
+llvm_add_library(
+CTTestTidyModule
+MODULE clang-tidy/CTTestTidyModule.cpp
+PLUGIN_TOOL clang-tidy
+DEPENDS clang-tidy-headers)
+  endif()
 
   if(CLANG_BUILT_STANDALONE)
 # LLVMHello library is needed below


Index: clang-tools-extra/test/CMakeLists.txt
===
--- clang-tools-extra/test/CMakeLists.txt
+++ clang-tools-extra/test/CMakeLists.txt
@@ -81,11 +81,13 @@
 endforeach()
 
 if (NOT LLVM_INSTALL_TOOLCHAIN_ONLY)
-  llvm_add_library(
-  CTTestTidyModule
-  MODULE clang-tidy/CTTestTidyModule.cpp
-  PLUGIN_TOOL clang-tidy
-  DEPENDS clang-tidy-headers)
+  if (NOT WIN32 AND NOT LLVM_LINK_LLVM_DYLIB)
+llvm_add_library(
+CTTestTidyModule
+MODULE clang-tidy/CTTestTidyModule.cpp
+PLUGIN_TOOL clang-tidy
+DEPENDS clang-tidy-headers)
+  endif()
 
   if(CLANG_BUILT_STANDALONE)
 # LLVMHello library is needed below
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D120727: [libc++] Overhaul how we select the ABI library

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added subscribers: phosek, mstorsjo.
mstorsjo added a comment.

FWIW I think D116689  interacts with this 
somewhat. I think D116689  is useful for the 
fixes I want to do wrt libcxxabi integration on Windows (but I haven't studied 
it, nor this one, closely yet though). @phosek Do you see anything in this one 
that isn't compatible with D116689 ?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120727/new/

https://reviews.llvm.org/D120727

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D122298: [clang-cl] Ignore /Wv and /Wv:17 flags

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo accepted this revision.
mstorsjo added a comment.
This revision is now accepted and ready to land.

The patch itself seems sensible to me.




Comment at: clang/include/clang/Driver/Options.td:2496
   Group, Flags<[CC1Option, NoArgumentUnused, HelpHidden]>;
-def fopenmp_assume_no_thread_state : Flag<["-"], 
"fopenmp-assume-no-thread-state">, Group, 
-  Flags<[CC1Option, NoArgumentUnused, HelpHidden]>, 
+def fopenmp_assume_no_thread_state : Flag<["-"], 
"fopenmp-assume-no-thread-state">, Group,
+  Flags<[CC1Option, NoArgumentUnused, HelpHidden]>,

thieta wrote:
> Do we mind removing these trailing whitespaces in the same commit? I can fix 
> that - but they shouldn't be there anyway.
I think it'd be fine to just push such a commit without taking it through 
review, marked as NFC (no functional changes). Or keep it here but split it out 
into a separate commit before pushing.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122298/new/

https://reviews.llvm.org/D122298

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D121873: [clang][extract-api] Add enum support

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

This (or some closely related commit?) causes a huge amount of warnings when 
building with GCC:

  warning: ‘clang::extractapi::EnumRecord’ has a field 
‘clang::extractapi::EnumRecord::Constants’ whose type uses the anonymous 
namespace [-Wsubobject-linkage]
   167 | struct EnumRecord : APIRecord {
   |^~


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D121873/new/

https://reviews.llvm.org/D121873

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D121687: [clang-tidy] Don't try to build CTTestTidyModule for Windows with dylibs

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added inline comments.



Comment at: clang-tools-extra/test/CMakeLists.txt:84
 if (NOT LLVM_INSTALL_TOOLCHAIN_ONLY)
-  llvm_add_library(
-  CTTestTidyModule
-  MODULE clang-tidy/CTTestTidyModule.cpp
-  PLUGIN_TOOL clang-tidy
-  DEPENDS clang-tidy-headers)
+  if (NOT WIN32 AND NOT LLVM_LINK_LLVM_DYLIB)
+llvm_add_library(

jeremyd2019 wrote:
> I only just noticed this patch, but it feels to me like this was intended to 
> be OR here, because the condition to be avoided seems to be `WIN32 AND 
> LLVM_LINK_LLVM_DYLIB`, which inverted is `NOT WIN32 OR NOT 
> LLVM_LINK_LLVM_DYLIB`
Oh indeed, thanks, good catch! I'll push a patch fixing that.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D121687/new/

https://reviews.llvm.org/D121687

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D122766: [clang] Use forward slash as the path separator for Windows in __FILE__, __builtin_FILE(), and std::source_location

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

> When targeting Windows, the path separator used when targeting Windows 
> depends on the build environment when Clang _itself_ is built. This leads to 
> inconsistencies in Chrome builds where Clang running on non-Windows 
> environments uses the forward slash (/) path separator while Clang running on 
> Windows builds uses the backslash (\) path separator. To fix this, we make 
> Clang use forward slashes whenever it is targeting Windows.

IMO, this problem formulation is a bit too narrow/specific - you'd have the 
same phenomenon if you'd cross compile things on Windows, targeting e.g. Linux. 
So I think the condition for normalizing paths would be if the build host OS is 
Windows, not based on the target OS. (I.e. in the current patch form, the 
normalization that is chosen, when targeting Windows from Unix, is a no-op 
anyway.)

Then secondly - if the source paths are relative paths, making them OS-agnostic 
in this way does make sense. But if they are absolute, they are pretty much by 
definition specific to the build host, and can't be expected to make sense on a 
different platform. So for such a case, there's less reason to try to normalize 
the paths. But as Windows path handling in most cases does tolerate paths with 
forward slashes, I guess it's ok to do anyway.

Overall, I think this is a sensible thing to do. I'm not entirely sure if an 
option for the behaviour is needed or not though - I guess that's the main 
question.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122766/new/

https://reviews.llvm.org/D122766

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D122766: [clang] Use forward slash as the path separator for Windows in __FILE__, __builtin_FILE(), and std::source_location

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

In D122766#3420925 , @thakis wrote:

> Windows can handle slashes, but several tools can't. I worry that if we do 
> something different than cl, some random corner case might break (dbghelp, or 
> some source server thing or some custom debug info processor somewhere).

Yup, that's a valid concern. Especially PDB handling is pretty particular about 
requiring backslashes, as far as I've seen.

>> Then secondly - if the source paths are relative paths, making them 
>> OS-agnostic in this way does make sense. But if they are absolute, they are 
>> pretty much by definition specific to the build host,
>
> We use `/pdbsourcepath:X:\fake\prefix` to write a deterministic absolute path 
> to the output file at link time (and `-fdebug-compilation-dir .` to get 
> deterministic compiler output – see 
> https://blog.llvm.org/2019/11/deterministic-builds-with-clang-and-lld.html). 
> The motivation is to get exactly the same output when building on linux and 
> windows hosts.

Ok, nice.

Do you agree that if we go down this path doing this, we should also do it 
whenever the host OS is windows, i.e. also for windows->linux cross 
compilation, for consistency?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122766/new/

https://reviews.llvm.org/D122766

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D121687: [clang-tidy] Don't try to build CTTestTidyModule for Windows with dylibs

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

@rnk Can you have a look at this?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D121687/new/

https://reviews.llvm.org/D121687

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D121687: [clang-tidy] Don't try to build CTTestTidyModule for Windows with dylibs

[Martin Storsjö via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG3e7a8aab759a: [clang-tidy] Dont try to build 
CTTestTidyModule for Windows with dylibs (authored by mstorsjo).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D121687/new/

https://reviews.llvm.org/D121687

Files:
  clang-tools-extra/test/CMakeLists.txt


Index: clang-tools-extra/test/CMakeLists.txt
===
--- clang-tools-extra/test/CMakeLists.txt
+++ clang-tools-extra/test/CMakeLists.txt
@@ -81,11 +81,13 @@
 endforeach()
 
 if (NOT LLVM_INSTALL_TOOLCHAIN_ONLY)
-  llvm_add_library(
-  CTTestTidyModule
-  MODULE clang-tidy/CTTestTidyModule.cpp
-  PLUGIN_TOOL clang-tidy
-  DEPENDS clang-tidy-headers)
+  if (NOT WIN32 AND NOT LLVM_LINK_LLVM_DYLIB)
+llvm_add_library(
+CTTestTidyModule
+MODULE clang-tidy/CTTestTidyModule.cpp
+PLUGIN_TOOL clang-tidy
+DEPENDS clang-tidy-headers)
+  endif()
 
   if(CLANG_BUILT_STANDALONE)
 # LLVMHello library is needed below


Index: clang-tools-extra/test/CMakeLists.txt
===
--- clang-tools-extra/test/CMakeLists.txt
+++ clang-tools-extra/test/CMakeLists.txt
@@ -81,11 +81,13 @@
 endforeach()
 
 if (NOT LLVM_INSTALL_TOOLCHAIN_ONLY)
-  llvm_add_library(
-  CTTestTidyModule
-  MODULE clang-tidy/CTTestTidyModule.cpp
-  PLUGIN_TOOL clang-tidy
-  DEPENDS clang-tidy-headers)
+  if (NOT WIN32 AND NOT LLVM_LINK_LLVM_DYLIB)
+llvm_add_library(
+CTTestTidyModule
+MODULE clang-tidy/CTTestTidyModule.cpp
+PLUGIN_TOOL clang-tidy
+DEPENDS clang-tidy-headers)
+  endif()
 
   if(CLANG_BUILT_STANDALONE)
 # LLVMHello library is needed below
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D118980: Don't dllexport reference temporaries

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo accepted this revision.
mstorsjo added a comment.
This revision is now accepted and ready to land.

LGTM, seems reasonable!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D118980/new/

https://reviews.llvm.org/D118980

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D106674: Runtime for Interop directive

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added inline comments.



Comment at: openmp/runtime/src/dllexports:553
+omp_get_interop_ptr 761
+omp_get_interop_str 762
 

jdoerfert wrote:
> Those values are taken by now, you need new values that are not taken yet.
This broke building for Windows:
```
generate-def.pl: (x) Error parsing file 
"llvm-project/openmp/runtime/src/dllexports" line 556:
generate-def.pl: (x) omp_get_interop_int 2514
generate-def.pl: (x) Ordinal of user-callable entry must be < 1000
```



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D106674/new/

https://reviews.llvm.org/D106674

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D106674: Runtime for Interop directive

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added inline comments.



Comment at: openmp/runtime/src/dllexports:553
+omp_get_interop_ptr 761
+omp_get_interop_str 762
 

mstorsjo wrote:
> jdoerfert wrote:
> > Those values are taken by now, you need new values that are not taken yet.
> This broke building for Windows:
> ```
> generate-def.pl: (x) Error parsing file 
> "llvm-project/openmp/runtime/src/dllexports" line 556:
> generate-def.pl: (x) omp_get_interop_int 2514
> generate-def.pl: (x) Ordinal of user-callable entry must be < 1000
> ```
> 
... and even with those ordinals changed to something in the right range, the 
build later fails with linker errors:
```
ld.lld: error: : undefined symbol: OMP_GET_INTEROP_INT
ld.lld: error: : undefined symbol: OMP_GET_INTEROP_PTR
ld.lld: error: : undefined symbol: OMP_GET_INTEROP_STR
```
So please fix the Windows build of OpenMP, before the 14.x branch is made early 
next week.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D106674/new/

https://reviews.llvm.org/D106674

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D119234: [clang] [MinGW] Recognize -lcrtdll as a library replacing -lmsvcrt

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo created this revision.
mstorsjo added reviewers: mati865, rnk.
mstorsjo requested review of this revision.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D119234

Files:
  clang/lib/Driver/ToolChains/MinGW.cpp
  clang/test/Driver/mingw-msvcrt.c


Index: clang/test/Driver/mingw-msvcrt.c
===
--- clang/test/Driver/mingw-msvcrt.c
+++ clang/test/Driver/mingw-msvcrt.c
@@ -2,6 +2,7 @@
 // RUN: %clang -v -target i686-pc-windows-gnu -lmsvcr120 -### %s 2>&1 | 
FileCheck -check-prefix=CHECK_MSVCR120 %s
 // RUN: %clang -v -target i686-pc-windows-gnu -lucrtbase -### %s 2>&1 | 
FileCheck -check-prefix=CHECK_UCRTBASE %s
 // RUN: %clang -v -target i686-pc-windows-gnu -lucrt -### %s 2>&1 | FileCheck 
-check-prefix=CHECK_UCRT %s
+// RUN: %clang -v -target i686-pc-windows-gnu -lcrtdll -### %s 2>&1 | 
FileCheck -check-prefix=CHECK_CRTDLL %s
 
 // CHECK_DEFAULT: "-lmingwex" "-lmsvcrt" "-ladvapi32"
 // CHECK_DEFAULT-SAME: "-lmsvcrt" "-lkernel32" "{{.*}}crtend.o"
@@ -11,3 +12,5 @@
 // CHECK_UCRTBASE-SAME: "-lmingwex" "-ladvapi32"
 // CHECK_UCRT: "-lucrt"
 // CHECK_UCRT-SAME: "-lmingwex" "-ladvapi32"
+// CHECK_CRTDLL: "-lcrtdll"
+// CHECK_CRTDLL-SAME: "-lmingwex" "-ladvapi32"
Index: clang/lib/Driver/ToolChains/MinGW.cpp
===
--- clang/lib/Driver/ToolChains/MinGW.cpp
+++ clang/lib/Driver/ToolChains/MinGW.cpp
@@ -86,7 +86,9 @@
   CmdArgs.push_back("-lmoldname");
   CmdArgs.push_back("-lmingwex");
   for (auto Lib : Args.getAllArgValues(options::OPT_l))
-if (StringRef(Lib).startswith("msvcr") || 
StringRef(Lib).startswith("ucrt"))
+if (StringRef(Lib).startswith("msvcr") ||
+StringRef(Lib).startswith("ucrt") ||
+StringRef(Lib).startswith("crtdll"))
   return;
   CmdArgs.push_back("-lmsvcrt");
 }


Index: clang/test/Driver/mingw-msvcrt.c
===
--- clang/test/Driver/mingw-msvcrt.c
+++ clang/test/Driver/mingw-msvcrt.c
@@ -2,6 +2,7 @@
 // RUN: %clang -v -target i686-pc-windows-gnu -lmsvcr120 -### %s 2>&1 | FileCheck -check-prefix=CHECK_MSVCR120 %s
 // RUN: %clang -v -target i686-pc-windows-gnu -lucrtbase -### %s 2>&1 | FileCheck -check-prefix=CHECK_UCRTBASE %s
 // RUN: %clang -v -target i686-pc-windows-gnu -lucrt -### %s 2>&1 | FileCheck -check-prefix=CHECK_UCRT %s
+// RUN: %clang -v -target i686-pc-windows-gnu -lcrtdll -### %s 2>&1 | FileCheck -check-prefix=CHECK_CRTDLL %s
 
 // CHECK_DEFAULT: "-lmingwex" "-lmsvcrt" "-ladvapi32"
 // CHECK_DEFAULT-SAME: "-lmsvcrt" "-lkernel32" "{{.*}}crtend.o"
@@ -11,3 +12,5 @@
 // CHECK_UCRTBASE-SAME: "-lmingwex" "-ladvapi32"
 // CHECK_UCRT: "-lucrt"
 // CHECK_UCRT-SAME: "-lmingwex" "-ladvapi32"
+// CHECK_CRTDLL: "-lcrtdll"
+// CHECK_CRTDLL-SAME: "-lmingwex" "-ladvapi32"
Index: clang/lib/Driver/ToolChains/MinGW.cpp
===
--- clang/lib/Driver/ToolChains/MinGW.cpp
+++ clang/lib/Driver/ToolChains/MinGW.cpp
@@ -86,7 +86,9 @@
   CmdArgs.push_back("-lmoldname");
   CmdArgs.push_back("-lmingwex");
   for (auto Lib : Args.getAllArgValues(options::OPT_l))
-if (StringRef(Lib).startswith("msvcr") || StringRef(Lib).startswith("ucrt"))
+if (StringRef(Lib).startswith("msvcr") ||
+StringRef(Lib).startswith("ucrt") ||
+StringRef(Lib).startswith("crtdll"))
   return;
   CmdArgs.push_back("-lmsvcrt");
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D119234: [clang] [MinGW] Recognize -lcrtdll as a library replacing -lmsvcrt

[Martin Storsjö via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG079b6d02d1f5: [clang] [MinGW] Recognize -lcrtdll as a 
library replacing -lmsvcrt (authored by mstorsjo).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119234/new/

https://reviews.llvm.org/D119234

Files:
  clang/lib/Driver/ToolChains/MinGW.cpp
  clang/test/Driver/mingw-msvcrt.c


Index: clang/test/Driver/mingw-msvcrt.c
===
--- clang/test/Driver/mingw-msvcrt.c
+++ clang/test/Driver/mingw-msvcrt.c
@@ -2,6 +2,7 @@
 // RUN: %clang -v -target i686-pc-windows-gnu -lmsvcr120 -### %s 2>&1 | 
FileCheck -check-prefix=CHECK_MSVCR120 %s
 // RUN: %clang -v -target i686-pc-windows-gnu -lucrtbase -### %s 2>&1 | 
FileCheck -check-prefix=CHECK_UCRTBASE %s
 // RUN: %clang -v -target i686-pc-windows-gnu -lucrt -### %s 2>&1 | FileCheck 
-check-prefix=CHECK_UCRT %s
+// RUN: %clang -v -target i686-pc-windows-gnu -lcrtdll -### %s 2>&1 | 
FileCheck -check-prefix=CHECK_CRTDLL %s
 
 // CHECK_DEFAULT: "-lmingwex" "-lmsvcrt" "-ladvapi32"
 // CHECK_DEFAULT-SAME: "-lmsvcrt" "-lkernel32" "{{.*}}crtend.o"
@@ -11,3 +12,5 @@
 // CHECK_UCRTBASE-SAME: "-lmingwex" "-ladvapi32"
 // CHECK_UCRT: "-lucrt"
 // CHECK_UCRT-SAME: "-lmingwex" "-ladvapi32"
+// CHECK_CRTDLL: "-lcrtdll"
+// CHECK_CRTDLL-SAME: "-lmingwex" "-ladvapi32"
Index: clang/lib/Driver/ToolChains/MinGW.cpp
===
--- clang/lib/Driver/ToolChains/MinGW.cpp
+++ clang/lib/Driver/ToolChains/MinGW.cpp
@@ -86,7 +86,9 @@
   CmdArgs.push_back("-lmoldname");
   CmdArgs.push_back("-lmingwex");
   for (auto Lib : Args.getAllArgValues(options::OPT_l))
-if (StringRef(Lib).startswith("msvcr") || 
StringRef(Lib).startswith("ucrt"))
+if (StringRef(Lib).startswith("msvcr") ||
+StringRef(Lib).startswith("ucrt") ||
+StringRef(Lib).startswith("crtdll"))
   return;
   CmdArgs.push_back("-lmsvcrt");
 }


Index: clang/test/Driver/mingw-msvcrt.c
===
--- clang/test/Driver/mingw-msvcrt.c
+++ clang/test/Driver/mingw-msvcrt.c
@@ -2,6 +2,7 @@
 // RUN: %clang -v -target i686-pc-windows-gnu -lmsvcr120 -### %s 2>&1 | FileCheck -check-prefix=CHECK_MSVCR120 %s
 // RUN: %clang -v -target i686-pc-windows-gnu -lucrtbase -### %s 2>&1 | FileCheck -check-prefix=CHECK_UCRTBASE %s
 // RUN: %clang -v -target i686-pc-windows-gnu -lucrt -### %s 2>&1 | FileCheck -check-prefix=CHECK_UCRT %s
+// RUN: %clang -v -target i686-pc-windows-gnu -lcrtdll -### %s 2>&1 | FileCheck -check-prefix=CHECK_CRTDLL %s
 
 // CHECK_DEFAULT: "-lmingwex" "-lmsvcrt" "-ladvapi32"
 // CHECK_DEFAULT-SAME: "-lmsvcrt" "-lkernel32" "{{.*}}crtend.o"
@@ -11,3 +12,5 @@
 // CHECK_UCRTBASE-SAME: "-lmingwex" "-ladvapi32"
 // CHECK_UCRT: "-lucrt"
 // CHECK_UCRT-SAME: "-lmingwex" "-ladvapi32"
+// CHECK_CRTDLL: "-lcrtdll"
+// CHECK_CRTDLL-SAME: "-lmingwex" "-ladvapi32"
Index: clang/lib/Driver/ToolChains/MinGW.cpp
===
--- clang/lib/Driver/ToolChains/MinGW.cpp
+++ clang/lib/Driver/ToolChains/MinGW.cpp
@@ -86,7 +86,9 @@
   CmdArgs.push_back("-lmoldname");
   CmdArgs.push_back("-lmingwex");
   for (auto Lib : Args.getAllArgValues(options::OPT_l))
-if (StringRef(Lib).startswith("msvcr") || StringRef(Lib).startswith("ucrt"))
+if (StringRef(Lib).startswith("msvcr") ||
+StringRef(Lib).startswith("ucrt") ||
+StringRef(Lib).startswith("crtdll"))
   return;
   CmdArgs.push_back("-lmsvcrt");
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D119326: [clang] [MinGW] Default to DWARF 4

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo created this revision.
mstorsjo added reviewers: dblaikie, rnk, mati865.
mstorsjo requested review of this revision.
Herald added a project: clang.

Neither LLDB nor GDB seem to work with DWARF 5 debug information on
Windows right now.

This applies the same change as in
9c6272861032f511a23784ce0c5cc8f6ac2f625b 
 (Default 
to DWARFv4 on Windows)
to the MinGW driver too.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D119326

Files:
  clang/lib/Driver/ToolChains/MinGW.h
  clang/test/CodeGen/dwarf-version.c


Index: clang/test/CodeGen/dwarf-version.c
===
--- clang/test/CodeGen/dwarf-version.c
+++ clang/test/CodeGen/dwarf-version.c
@@ -32,6 +32,11 @@
 // Explicitly request both.
 // RUN: %clang -target i686-pc-windows-msvc -gdwarf -gcodeview -S -emit-llvm 
-o - %s \
 // RUN: | FileCheck %s --check-prefixes=VER4,CODEVIEW
+
+// Check what version of dwarf is used for MinGW targets.
+// RUN: %clang -target i686-pc-windows-gnu -g -S -emit-llvm -o - %s | \
+// RUN:   FileCheck %s --check-prefixes=VER4
+
 // RUN: %clang -target powerpc-ibm-aix-xcoff -g -S -emit-llvm -o - %s | \
 // RUN:   FileCheck %s --check-prefix=VER3
 // RUN: %clang -target powerpc-ibm-aix-xcoff -gdwarf-2 -S -emit-llvm -o - %s | 
\
Index: clang/lib/Driver/ToolChains/MinGW.h
===
--- clang/lib/Driver/ToolChains/MinGW.h
+++ clang/lib/Driver/ToolChains/MinGW.h
@@ -90,6 +90,8 @@
 
   void printVerboseInfo(raw_ostream ) const override;
 
+  unsigned GetDefaultDwarfVersion() const override { return 4; }
+
 protected:
   Tool *getTool(Action::ActionClass AC) const override;
   Tool *buildLinker() const override;


Index: clang/test/CodeGen/dwarf-version.c
===
--- clang/test/CodeGen/dwarf-version.c
+++ clang/test/CodeGen/dwarf-version.c
@@ -32,6 +32,11 @@
 // Explicitly request both.
 // RUN: %clang -target i686-pc-windows-msvc -gdwarf -gcodeview -S -emit-llvm -o - %s \
 // RUN: | FileCheck %s --check-prefixes=VER4,CODEVIEW
+
+// Check what version of dwarf is used for MinGW targets.
+// RUN: %clang -target i686-pc-windows-gnu -g -S -emit-llvm -o - %s | \
+// RUN:   FileCheck %s --check-prefixes=VER4
+
 // RUN: %clang -target powerpc-ibm-aix-xcoff -g -S -emit-llvm -o - %s | \
 // RUN:   FileCheck %s --check-prefix=VER3
 // RUN: %clang -target powerpc-ibm-aix-xcoff -gdwarf-2 -S -emit-llvm -o - %s | \
Index: clang/lib/Driver/ToolChains/MinGW.h
===
--- clang/lib/Driver/ToolChains/MinGW.h
+++ clang/lib/Driver/ToolChains/MinGW.h
@@ -90,6 +90,8 @@
 
   void printVerboseInfo(raw_ostream ) const override;
 
+  unsigned GetDefaultDwarfVersion() const override { return 4; }
+
 protected:
   Tool *getTool(Action::ActionClass AC) const override;
   Tool *buildLinker() const override;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D110485: Support [[no_unique_address]] for all targets.

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

To pick up the thread here again, `[[no_unique_address]]` is done and settled 
in MSVC, with the slightly surprising semantics: `[[no_unique_address]]` is 
accepted, without any warning (in C++20 mode), but it has no effect. (This, not 
related to LLVM, but because they had shipped it in earlier versions without 
having an effect, and changing that later would break things.) 
`[[msvc::no_unique_address]]` does have an effect though. See 
https://github.com/microsoft/STL/issues/1364#issuecomment-1034167093 for a more 
authoritative source on that.

So, separately from implementing `[[msvc::no_unique_address]]`, I think we also 
should also silence the current warning about unknown attribute for the 
standard `[[no_unique_address]]`, to match MSVC.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110485/new/

https://reviews.llvm.org/D110485

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D110485: Support [[no_unique_address]] for all targets.

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

In D110485#3308854 , @mstorsjo wrote:

> To pick up the thread here again, `[[no_unique_address]]` is done and settled 
> in MSVC, with the slightly surprising semantics: `[[no_unique_address]]` is 
> accepted, without any warning (in C++20 mode), but it has no effect. (This, 
> not related to LLVM, but because they had shipped it in earlier versions 
> without having an effect, and changing that later would break things.) 
> `[[msvc::no_unique_address]]` does have an effect though. See 
> https://github.com/microsoft/STL/issues/1364#issuecomment-1034167093 for a 
> more authoritative source on that.
>
> So, separately from implementing `[[msvc::no_unique_address]]`, I think we 
> also should also silence the current warning about unknown attribute for the 
> standard `[[no_unique_address]]`, to match MSVC.

Oh, also, according to 
https://devblogs.microsoft.com/cppblog/msvc-cpp20-and-the-std-cpp20-switch/, 
the plan is to change `[[no_unique_address]]` to actually have an effect the 
next time the compiler breaks its C++ ABI at an unknown point in the future. 
(This shouldn't be an issue for Clang, as we'd have to make a conscious effort 
to implement the new ABI whenever that happens anyway.)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110485/new/

https://reviews.llvm.org/D110485

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D119326: [clang] [MinGW] Default to DWARF 4

[Martin Storsjö via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG6cf64b2d2858: [clang] [MinGW] Default to DWARF 4 (authored 
by mstorsjo).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119326/new/

https://reviews.llvm.org/D119326

Files:
  clang/lib/Driver/ToolChains/MinGW.h
  clang/test/CodeGen/dwarf-version.c


Index: clang/test/CodeGen/dwarf-version.c
===
--- clang/test/CodeGen/dwarf-version.c
+++ clang/test/CodeGen/dwarf-version.c
@@ -32,6 +32,11 @@
 // Explicitly request both.
 // RUN: %clang -target i686-pc-windows-msvc -gdwarf -gcodeview -S -emit-llvm 
-o - %s \
 // RUN: | FileCheck %s --check-prefixes=VER4,CODEVIEW
+
+// Check what version of dwarf is used for MinGW targets.
+// RUN: %clang -target i686-pc-windows-gnu -g -S -emit-llvm -o - %s | \
+// RUN:   FileCheck %s --check-prefixes=VER4
+
 // RUN: %clang -target powerpc-ibm-aix-xcoff -g -S -emit-llvm -o - %s | \
 // RUN:   FileCheck %s --check-prefix=VER3
 // RUN: %clang -target powerpc-ibm-aix-xcoff -gdwarf-2 -S -emit-llvm -o - %s | 
\
Index: clang/lib/Driver/ToolChains/MinGW.h
===
--- clang/lib/Driver/ToolChains/MinGW.h
+++ clang/lib/Driver/ToolChains/MinGW.h
@@ -90,6 +90,8 @@
 
   void printVerboseInfo(raw_ostream ) const override;
 
+  unsigned GetDefaultDwarfVersion() const override { return 4; }
+
 protected:
   Tool *getTool(Action::ActionClass AC) const override;
   Tool *buildLinker() const override;


Index: clang/test/CodeGen/dwarf-version.c
===
--- clang/test/CodeGen/dwarf-version.c
+++ clang/test/CodeGen/dwarf-version.c
@@ -32,6 +32,11 @@
 // Explicitly request both.
 // RUN: %clang -target i686-pc-windows-msvc -gdwarf -gcodeview -S -emit-llvm -o - %s \
 // RUN: | FileCheck %s --check-prefixes=VER4,CODEVIEW
+
+// Check what version of dwarf is used for MinGW targets.
+// RUN: %clang -target i686-pc-windows-gnu -g -S -emit-llvm -o - %s | \
+// RUN:   FileCheck %s --check-prefixes=VER4
+
 // RUN: %clang -target powerpc-ibm-aix-xcoff -g -S -emit-llvm -o - %s | \
 // RUN:   FileCheck %s --check-prefix=VER3
 // RUN: %clang -target powerpc-ibm-aix-xcoff -gdwarf-2 -S -emit-llvm -o - %s | \
Index: clang/lib/Driver/ToolChains/MinGW.h
===
--- clang/lib/Driver/ToolChains/MinGW.h
+++ clang/lib/Driver/ToolChains/MinGW.h
@@ -90,6 +90,8 @@
 
   void printVerboseInfo(raw_ostream ) const override;
 
+  unsigned GetDefaultDwarfVersion() const override { return 4; }
+
 protected:
   Tool *getTool(Action::ActionClass AC) const override;
   Tool *buildLinker() const override;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits