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

[Aiden Grossman via Phabricator via cfe-commits]

aidengrossman added reviewers: aaron.ballman, glandium, mstorsjo, craig.topper.
aidengrossman added a comment.

This is an updated version of https://reviews.llvm.org/D150646 that uses 
`__has_builtin` instead of checking for a definition of `_MSC_EXTENSIONS`. This 
fixes the cases that I have received so far and shouldn't cause any other 
issues.

There is one thing that needs some discussion: include order relating to 
`intrin.h`. When using a MSVC target triple with `-fms-extensions` (and maybe 
in other cases), `intrin.h` needs to be included to get the declaration of the 
built-in (or otherwise `__has_builtin` won't detect it and it will error out 
later). This means that the way the patch is setup currently `intrin.h` needs 
to be included before `cpuid.h`. The only way I see to fix this is to use the 
MSVC `__cpuidex` signature which drops the `static` keyword. This makes the 
implementation slightly different from the GCC implementation (where they 
include `static`), but it would allow us to declare the function outside of the 
preprocessor conditional in `cpuid.h` which would make `cpuid.h` 
include-order-agnostic again with respect to `intrin.h`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158348

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

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

[Aiden Grossman via Phabricator via cfe-commits]

aidengrossman created this revision.
Herald added a project: All.
aidengrossman requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added subscribers: cfe-commits, jplehr, sstefan1.
Herald added a project: clang.

MSVC has a __cpuidex function implemented to call the underlying cpuid
instruction which accepts a leaf, subleaf, and data array that the output
data is written into. This patch adds this functionality into clang
under the cpuid.h header. This also makes clang match GCC's behavior.
GCC has had __cpuidex in its cpuid.h since 2020.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D158348

Files:
  clang/lib/Headers/cpuid.h
  clang/test/Headers/__cpuidex_conflict.c
  clang/test/Headers/cpuid.c


Index: clang/test/Headers/cpuid.c
===
--- clang/test/Headers/cpuid.c
+++ clang/test/Headers/cpuid.c
@@ -6,14 +6,19 @@
 
 // CHECK-64: {{.*}} call { i32, i32, i32, i32 } asm "  xchgq  %rbx,${1:q}\0A 
cpuid\0A xchgq %rbx,${1:q}", 
"={ax},=r,={cx},={dx},0,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}})
 // CHECK-64: {{.*}} call { i32, i32, i32, i32 } asm "  xchgq  %rbx,${1:q}\0A  
cpuid\0A  xchgq  %rbx,${1:q}", 
"={ax},=r,={cx},={dx},0,2,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}}, i32 
%{{[a-z0-9]+}})
+// CHECK-64: {{.*}} call { i32, i32, i32, i32 } asm "  xchgq  %rbx,${1:q}\0A  
cpuid\0A  xchgq  %rbx,${1:q}", 
"={ax},=r,={cx},={dx},0,2,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}}, i32 
%{{[a-z0-9]+}})
 
 // CHECK-32: {{.*}} call { i32, i32, i32, i32 } asm "cpuid", 
"={ax},={bx},={cx},={dx},0,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}})
 // CHECK-32: {{.*}} call { i32, i32, i32, i32 } asm "cpuid", 
"={ax},={bx},={cx},={dx},0,2,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}}, 
i32 %{{[a-z0-9]+}})
+// CHECK-32: {{.*}} call { i32, i32, i32, i32 } asm "cpuid", 
"={ax},={bx},={cx},={dx},0,2,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}}, 
i32 %{{[a-z0-9]+}})
 
 unsigned eax0, ebx0, ecx0, edx0;
 unsigned eax1, ebx1, ecx1, edx1;
 
+int cpuid_info[4];
+
 void test_cpuid(unsigned level, unsigned count) {
   __cpuid(level, eax1, ebx1, ecx1, edx1);
   __cpuid_count(level, count, eax0, ebx0, ecx0, edx0);
+  __cpuidex(cpuid_info, level, count);
 }
Index: clang/test/Headers/__cpuidex_conflict.c
===
--- /dev/null
+++ clang/test/Headers/__cpuidex_conflict.c
@@ -0,0 +1,22 @@
+// Make sure that __cpuidex in cpuid.h doesn't conflict with the MS
+// extensions built in by ensuring compilation succeeds:
+// RUN: %clang_cc1 %s -ffreestanding -fms-extensions -fms-compatibility \
+// RUN:  -fms-compatibility-version=19.00 -triple x86_64-pc-windows-msvc 
-emit-llvm -o -
+// %clang_cc1 %s -ffreestanding -triple x86_64-w64-windows-gnu -fms-extensions 
-emit-llvm -o -
+// RUN: %clang_cc1 %s -ffreestanding -fopenmp -fopenmp-is-target-device 
-aux-triple x86_64-unknown-linux-gnu
+
+typedef __SIZE_TYPE__ size_t;
+
+// We declare __cpuidex here as where the buitlin should be exposed (MSVC), the
+// declaration is in , but  is not available from all the
+// targets that are being tested here.
+void __cpuidex (int[4], int, int);
+
+#include 
+
+int cpuid_info[4];
+
+void test_cpuidex(unsigned level, unsigned count) {
+  __cpuidex(cpuid_info, level, count);
+}
+
Index: clang/lib/Headers/cpuid.h
===
--- clang/lib/Headers/cpuid.h
+++ clang/lib/Headers/cpuid.h
@@ -328,4 +328,14 @@
 return 1;
 }
 
+// In some configurations, __cpuidex is defined as a builtin (primarily
+// -fms-extensions) which will conflict with the __cpuidex definition below.
+#if !(__has_builtin(__cpuidex))
+static __inline void __cpuidex (int __cpu_info[4], int __leaf, int __subleaf)
+{
+  __cpuid_count(__leaf, __subleaf, __cpu_info[0], __cpu_info[1], __cpu_info[2],
+__cpu_info[3]);
+}
+#endif
+
 #endif /* __CPUID_H */


Index: clang/test/Headers/cpuid.c
===
--- clang/test/Headers/cpuid.c
+++ clang/test/Headers/cpuid.c
@@ -6,14 +6,19 @@
 
 // CHECK-64: {{.*}} call { i32, i32, i32, i32 } asm "  xchgq  %rbx,${1:q}\0A cpuid\0A xchgq %rbx,${1:q}", "={ax},=r,={cx},={dx},0,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}})
 // CHECK-64: {{.*}} call { i32, i32, i32, i32 } asm "  xchgq  %rbx,${1:q}\0A  cpuid\0A  xchgq  %rbx,${1:q}", "={ax},=r,={cx},={dx},0,2,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}}, i32 %{{[a-z0-9]+}})
+// CHECK-64: {{.*}} call { i32, i32, i32, i32 } asm "  xchgq  %rbx,${1:q}\0A  cpuid\0A  xchgq  %rbx,${1:q}", "={ax},=r,={cx},={dx},0,2,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}}, i32 %{{[a-z0-9]+}})
 
 // CHECK-32: {{.*}} call { i32, i32, i32, i32 } asm "cpuid", "={ax},={bx},={cx},={dx},0,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}})
 // CHECK-32: {{.*}} call { i32, i32, i32, i32 } asm "cpuid", "={ax},={bx},={cx},={dx},0,2,~{dirfl

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

[Aiden Grossman via Phabricator via cfe-commits]

aidengrossman added a comment.

> I believe that /Ze has more in common with `-fms-compatibility` than 
> `-fms-extensions`, but I could be wrong. Also, they may just be completely 
> different things at this point. `/permissive` is closer in spirit to 
> `-fms-compatibility`.

Better documentation at some point in the future would be helpful.

> In any case, if the has_builtin check works, I'd rather leave the macros 
> alone. There could be unintended consequences, and I'm not fully convinced 
> this is the right change.

Sounds good! That should also fix the aux triple issue with the `__cpuidex` 
patch as well (but not the underlying issue). Thank you for your insight and 
patience!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157334

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

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

[Aiden Grossman via Phabricator via cfe-commits]

aidengrossman added a comment.

In D157334#4596328 , @rnk wrote:

> I don't think this is the right thing to do, I think I recall saying as much 
> on some other review. The MSVC docs 
> 
>  say that `/Za`/`/Ze` control `_MSC_EXTENSION`, and as I understand it, `/Za` 
> is more like our `-fms-compatibility` flag, so it makes sense to control this 
> macro with `-fms-compatibility`.

From what I can understand from this page 
,
 `/Ze` enables the MSVC extensions (which is set by default) and `/Za` disables 
the MSVC extensions. From what I can gather reading the Clang options docs 
, 
`-fms-compatibility` is intended to be a superset of `-fms-extensions`. 
Assuming those two things, I think it's logical to set `_MSC_EXTENSIONS` with 
`-fms-extensions` rather than `-fms-compatibility`. (Also assuming that `/Ze` 
is similar to our `-fms-extensions` which I believe is the case, but not 
completely sure).

> Even though the name of the macro and the flag correspond, they aren't 
> covering the same thing.
>
> Let's try to focus on the use case: We need a feature flag or macro to 
> communicate that `-fms-extensions` is enabled on mingw. `_MSC_VER` is out 
> because we're doing mingw. Is there something else we could check for like 
> `__has_declspec_attribute` or `__has_builtin`?

`__has_builtin` works (no idea why I didn't think of this, thank you very much 
for the suggestion) for that case.

Like you mentioned in the https://reviews.llvm.org/D150646, exposing the 
builtins with `-fms-extensions` doesn't make sense, and that should probably be 
fixed at some point. That should probably be left to another patch though, 
along with other issues related to when builtins get exposed (like the 
previously mentioned aux triple issue).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157334

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

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

[Aiden Grossman via Phabricator via cfe-commits]

aidengrossman updated this revision to Diff 550529.
aidengrossman added a comment.

Hoist _MSC_EXTENSIONS macro logic even further and add release note on chages.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157334

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Basic/Targets/OSTargets.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/test/Preprocessor/predefined-win-macros.c


Index: clang/test/Preprocessor/predefined-win-macros.c
===
--- clang/test/Preprocessor/predefined-win-macros.c
+++ clang/test/Preprocessor/predefined-win-macros.c
@@ -120,6 +120,11 @@
 // CHECK-AMD64-MINGW: #define _WIN32 1
 // CHECK-AMD64-MINGW: #define _WIN64 1
 
+// RUN: %clang_cc1 -triple x86_64-windows-gnu %s -E -dM -o - -fms-extensions \
+// RUN:   | FileCheck -match-full-lines %s --check-prefix=CHECK-AMD64-MINGW-MSE
+
+// CHECK-AMD64-MINGW-MSE: #define _MSC_EXTENSIONS 1
+
 // RUN: %clang_cc1 -triple aarch64-windows-gnu %s -E -dM -o - \
 // RUN:   | FileCheck -match-full-lines %s --check-prefix=CHECK-ARM64-MINGW
 
Index: clang/lib/Frontend/InitPreprocessor.cpp
===
--- clang/lib/Frontend/InitPreprocessor.cpp
+++ clang/lib/Frontend/InitPreprocessor.cpp
@@ -926,11 +926,19 @@
 Builder.defineMacro("__private_extern__", "extern");
 
   if (LangOpts.MicrosoftExt) {
+Builder.defineMacro("_MSC_EXTENSIONS");
+
 if (LangOpts.WChar) {
   // wchar_t supported as a keyword.
   Builder.defineMacro("_WCHAR_T_DEFINED");
   Builder.defineMacro("_NATIVE_WCHAR_T_DEFINED");
 }
+
+if (LangOpts.CPlusPlus11) {
+  Builder.defineMacro("_RVALUE_REFERENCES_V2_SUPPORTED");
+  Builder.defineMacro("_RVALUE_REFERENCES_SUPPORTED");
+  Builder.defineMacro("_NATIVE_NULLPTR_SUPPORTED");
+}
   }
 
   // Macros to help identify the narrow and wide character sets
Index: clang/lib/Basic/Targets/OSTargets.cpp
===
--- clang/lib/Basic/Targets/OSTargets.cpp
+++ clang/lib/Basic/Targets/OSTargets.cpp
@@ -226,16 +226,6 @@
 }
   }
 
-  if (Opts.MicrosoftExt) {
-Builder.defineMacro("_MSC_EXTENSIONS");
-
-if (Opts.CPlusPlus11) {
-  Builder.defineMacro("_RVALUE_REFERENCES_V2_SUPPORTED");
-  Builder.defineMacro("_RVALUE_REFERENCES_SUPPORTED");
-  Builder.defineMacro("_NATIVE_NULLPTR_SUPPORTED");
-}
-  }
-
   if (!Opts.MSVolatile)
 Builder.defineMacro("_ISO_VOLATILE");
 
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -110,6 +110,9 @@
 
 Modified Compiler Flags
 ---
+- ``-fms-extensions`` now defines the ``_MSC_EXTENSIONS`` macro regardless of
+  whether or not the ``-fms-compatibility`` flag is set and regardless of the
+  target triple.
 
 Removed Compiler Flags
 -


Index: clang/test/Preprocessor/predefined-win-macros.c
===
--- clang/test/Preprocessor/predefined-win-macros.c
+++ clang/test/Preprocessor/predefined-win-macros.c
@@ -120,6 +120,11 @@
 // CHECK-AMD64-MINGW: #define _WIN32 1
 // CHECK-AMD64-MINGW: #define _WIN64 1
 
+// RUN: %clang_cc1 -triple x86_64-windows-gnu %s -E -dM -o - -fms-extensions \
+// RUN:   | FileCheck -match-full-lines %s --check-prefix=CHECK-AMD64-MINGW-MSE
+
+// CHECK-AMD64-MINGW-MSE: #define _MSC_EXTENSIONS 1
+
 // RUN: %clang_cc1 -triple aarch64-windows-gnu %s -E -dM -o - \
 // RUN:   | FileCheck -match-full-lines %s --check-prefix=CHECK-ARM64-MINGW
 
Index: clang/lib/Frontend/InitPreprocessor.cpp
===
--- clang/lib/Frontend/InitPreprocessor.cpp
+++ clang/lib/Frontend/InitPreprocessor.cpp
@@ -926,11 +926,19 @@
 Builder.defineMacro("__private_extern__", "extern");
 
   if (LangOpts.MicrosoftExt) {
+Builder.defineMacro("_MSC_EXTENSIONS");
+
 if (LangOpts.WChar) {
   // wchar_t supported as a keyword.
   Builder.defineMacro("_WCHAR_T_DEFINED");
   Builder.defineMacro("_NATIVE_WCHAR_T_DEFINED");
 }
+
+if (LangOpts.CPlusPlus11) {
+  Builder.defineMacro("_RVALUE_REFERENCES_V2_SUPPORTED");
+  Builder.defineMacro("_RVALUE_REFERENCES_SUPPORTED");
+  Builder.defineMacro("_NATIVE_NULLPTR_SUPPORTED");
+}
   }
 
   // Macros to help identify the narrow and wide character sets
Index: clang/lib/Basic/Targets/OSTargets.cpp
===
--- clang/lib/Basic/Targets/OSTargets.cpp
+++ clang/lib/Basic/Targets/OSTargets.cpp
@@ -226,16 +226,6 @@
 }
   }
 
-  if (Opts.MicrosoftExt) {
-Builder.defineMacro("_MSC_EXTENSIONS");
-
-if (Opts.CPlusPlus11) {
-  Builder.defineMacro("_RVALUE_REFERENCES_V2_SUPPORTED

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

[Aiden Grossman via Phabricator via cfe-commits]

aidengrossman added a comment.

The main issue here is that there's no way to reliably detect whether the built 
in is defined (can't check if a function is defined in the preprocessor, 
preprocessor macro isn't exposed correctly in all configurations), which ends 
up breaking some configurations. I wouldn't be opposed to exposing things more 
often, but I'm fine with the builtins being exposed under `-fms-extensions`, we 
just need to expose the preprocessor macro when we expose the builtins.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150646

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

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

[Aiden Grossman via Phabricator via cfe-commits]

aidengrossman added a comment.

It's something set on by default in MSVC 
(https://learn.microsoft.com/en-us/cpp/preprocessor/predefined-macros?view=msvc-170).
 It's interesting that GCC doesn't set the `_MSC_EXTENSIONS` macro with 
`-fms-extensions`. It should if it wants to match the behavior of MSVC. It 
might cause an issue somewhere, but I think it'll probably be exceedingly rare 
and I think would be resulting from incorrect user code (assuming all the 
extensions enabled by MSVC with the flag they use are present in clang). I 
would've thought not having the macro defined would cause more issues, but 
given this part of the code hasn't been touched in six years (and that was a 
refactoring that was presumably NFC), it doesn't seem like people rely on this 
macro too much. Thanks for running the tests though! Having concrete data to 
back up hypotheses is always great to have.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157334

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

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

[Aiden Grossman via Phabricator via cfe-commits]

aidengrossman added a comment.

Since `-fms-extensions` can be enabled on Linux as well, we should probably 
hoist this further since this patch only accounts for the windows case as I 
just hoist the conditional to be in `addWindowsDefines`. I'll work on getting 
another patch version up in a bit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157334

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

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

[Aiden Grossman via Phabricator via cfe-commits]

aidengrossman updated this revision to Diff 548473.
aidengrossman added a comment.

Reformat using arc as manually uploading patch messes up formatting.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157334

Files:
  clang/lib/Basic/Targets/OSTargets.cpp
  clang/test/Preprocessor/predefined-win-macros.c


Index: clang/test/Preprocessor/predefined-win-macros.c
===
--- clang/test/Preprocessor/predefined-win-macros.c
+++ clang/test/Preprocessor/predefined-win-macros.c
@@ -120,6 +120,11 @@
 // CHECK-AMD64-MINGW: #define _WIN32 1
 // CHECK-AMD64-MINGW: #define _WIN64 1
 
+// RUN: %clang_cc1 -triple x86_64-windows-gnu %s -E -dM -o - -fms-extensions \
+// RUN:   | FileCheck -match-full-lines %s --check-prefix=CHECK-AMD64-MINGW-MSE
+
+// CHECK-AMD64-MINGW-MSE: #define _MSC_EXTENSIONS 1
+
 // RUN: %clang_cc1 -triple aarch64-windows-gnu %s -E -dM -o - \
 // RUN:   | FileCheck -match-full-lines %s --check-prefix=CHECK-ARM64-MINGW
 
Index: clang/lib/Basic/Targets/OSTargets.cpp
===
--- clang/lib/Basic/Targets/OSTargets.cpp
+++ clang/lib/Basic/Targets/OSTargets.cpp
@@ -226,16 +226,6 @@
 }
   }
 
-  if (Opts.MicrosoftExt) {
-Builder.defineMacro("_MSC_EXTENSIONS");
-
-if (Opts.CPlusPlus11) {
-  Builder.defineMacro("_RVALUE_REFERENCES_V2_SUPPORTED");
-  Builder.defineMacro("_RVALUE_REFERENCES_SUPPORTED");
-  Builder.defineMacro("_NATIVE_NULLPTR_SUPPORTED");
-}
-  }
-
   if (!Opts.MSVolatile)
 Builder.defineMacro("_ISO_VOLATILE");
 
@@ -264,6 +254,16 @@
   else if (Triple.isKnownWindowsMSVCEnvironment() ||
(Triple.isWindowsItaniumEnvironment() && Opts.MSVCCompat))
 addVisualCDefines(Opts, Builder);
+
+  if (Opts.MicrosoftExt) {
+Builder.defineMacro("_MSC_EXTENSIONS");
+
+if (Opts.CPlusPlus11) {
+  Builder.defineMacro("_RVALUE_REFERENCES_V2_SUPPORTED");
+  Builder.defineMacro("_RVALUE_REFERENCES_SUPPORTED");
+  Builder.defineMacro("_NATIVE_NULLPTR_SUPPORTED");
+}
+  }
 }
 
 } // namespace targets


Index: clang/test/Preprocessor/predefined-win-macros.c
===
--- clang/test/Preprocessor/predefined-win-macros.c
+++ clang/test/Preprocessor/predefined-win-macros.c
@@ -120,6 +120,11 @@
 // CHECK-AMD64-MINGW: #define _WIN32 1
 // CHECK-AMD64-MINGW: #define _WIN64 1
 
+// RUN: %clang_cc1 -triple x86_64-windows-gnu %s -E -dM -o - -fms-extensions \
+// RUN:   | FileCheck -match-full-lines %s --check-prefix=CHECK-AMD64-MINGW-MSE
+
+// CHECK-AMD64-MINGW-MSE: #define _MSC_EXTENSIONS 1
+
 // RUN: %clang_cc1 -triple aarch64-windows-gnu %s -E -dM -o - \
 // RUN:   | FileCheck -match-full-lines %s --check-prefix=CHECK-ARM64-MINGW
 
Index: clang/lib/Basic/Targets/OSTargets.cpp
===
--- clang/lib/Basic/Targets/OSTargets.cpp
+++ clang/lib/Basic/Targets/OSTargets.cpp
@@ -226,16 +226,6 @@
 }
   }
 
-  if (Opts.MicrosoftExt) {
-Builder.defineMacro("_MSC_EXTENSIONS");
-
-if (Opts.CPlusPlus11) {
-  Builder.defineMacro("_RVALUE_REFERENCES_V2_SUPPORTED");
-  Builder.defineMacro("_RVALUE_REFERENCES_SUPPORTED");
-  Builder.defineMacro("_NATIVE_NULLPTR_SUPPORTED");
-}
-  }
-
   if (!Opts.MSVolatile)
 Builder.defineMacro("_ISO_VOLATILE");
 
@@ -264,6 +254,16 @@
   else if (Triple.isKnownWindowsMSVCEnvironment() ||
(Triple.isWindowsItaniumEnvironment() && Opts.MSVCCompat))
 addVisualCDefines(Opts, Builder);
+
+  if (Opts.MicrosoftExt) {
+Builder.defineMacro("_MSC_EXTENSIONS");
+
+if (Opts.CPlusPlus11) {
+  Builder.defineMacro("_RVALUE_REFERENCES_V2_SUPPORTED");
+  Builder.defineMacro("_RVALUE_REFERENCES_SUPPORTED");
+  Builder.defineMacro("_NATIVE_NULLPTR_SUPPORTED");
+}
+  }
 }
 
 } // namespace targets
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Aiden Grossman via Phabricator via cfe-commits]

aidengrossman added reviewers: aaron.ballman, hans, mstorsjo, rnk, glandium.
aidengrossman added a comment.

This should fix the issues seen in https://reviews.llvm.org/D150646 regarding 
`-fms-extensions` being set on MinGW and the builtins being defined but no 
`_MSC_EXTENSIONS` macro being set which caused a redefinition of the 
`__cpuidex` function. I'm not sure if this is 100% correct, but at least 
something needs to be done when `-fms-extensions` is passed and the target 
triple includes `-gnu`. Also not sure what happened with the patch formatting 
in Phabricator. The git diff is much cleaner.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157334

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

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

[Aiden Grossman via Phabricator via cfe-commits]

aidengrossman created this revision.
Herald added subscribers: pengfei, mstorsjo.
Herald added a project: All.
aidengrossman requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This patch makes sure _MSC_EXTENSIONS is defined if -fms-extensions is set by 
hosting the logic for setting the -fms-extensions related macros out of some 
conditional checks related to the target triple. Otherwise we would end up in 
situations where Windows builtins added by -fms-extensions would be defined but 
_MSC_EXTENSIONS wouldn't be defined like on MinGW with the triple 
x86_64-windows-gnu.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157334

Files:
  clang/lib/Basic/Targets/OSTargets.cpp
  clang/test/Preprocessor/predefined-win-macros.c


Index: clang/test/Preprocessor/predefined-win-macros.c
===
--- clang/test/Preprocessor/predefined-win-macros.c
+++ clang/test/Preprocessor/predefined-win-macros.c
@@ -120,9 +120,14 @@
 // CHECK-AMD64-MINGW: #define _WIN32 1
 // CHECK-AMD64-MINGW: #define _WIN64 1

+// RUN: %clang_cc1 -triple x86_64-windows-gnu %s -E -dM -o - -fms-extensions \
+// RUN:   | FileCheck -match-full-lines %s --check-prefix=CHECK-AMD64-MINGW-MSE
+
+// CHECK-AMD64-MINGW-MSE: #define _MSC_EXTENSIONS 1
+
 // RUN: %clang_cc1 -triple aarch64-windows-gnu %s -E -dM -o - \
 // RUN:   | FileCheck -match-full-lines %s --check-prefix=CHECK-ARM64-MINGW

 // CHECK-ARM64-MINGW-NOT: #define _M_ARM64 1
 // CHECK-ARM64-MINGW: #define WIN32 1
 // CHECK-ARM64-MINGW: #define WIN64 1
Index: clang/lib/Basic/Targets/OSTargets.cpp
===
--- clang/lib/Basic/Targets/OSTargets.cpp
+++ clang/lib/Basic/Targets/OSTargets.cpp
@@ -226,25 +226,15 @@
 }
   }

-  if (Opts.MicrosoftExt) {
-Builder.defineMacro("_MSC_EXTENSIONS");
-
-if (Opts.CPlusPlus11) {
-  Builder.defineMacro("_RVALUE_REFERENCES_V2_SUPPORTED");
-  Builder.defineMacro("_RVALUE_REFERENCES_SUPPORTED");
-  Builder.defineMacro("_NATIVE_NULLPTR_SUPPORTED");
-}
-  }
-
   if (!Opts.MSVolatile)
 Builder.defineMacro("_ISO_VOLATILE");

   if (Opts.Kernel)
 Builder.defineMacro("_KERNEL_MODE");

   Builder.defineMacro("_INTEGRAL_MAX_BITS", "64");
   Builder.defineMacro("__STDC_NO_THREADS__");

   // Starting with VS 2022 17.1, MSVC predefines the below macro to inform
   // users of the execution character set defined at compile time.
   // The value given is the Windows Code Page Identifier:
@@ -264,7 +254,17 @@
   else if (Triple.isKnownWindowsMSVCEnvironment() ||
(Triple.isWindowsItaniumEnvironment() && Opts.MSVCCompat))
 addVisualCDefines(Opts, Builder);
+
+  if (Opts.MicrosoftExt) {
+Builder.defineMacro("_MSC_EXTENSIONS");
+
+if (Opts.CPlusPlus11) {
+  Builder.defineMacro("_RVALUE_REFERENCES_V2_SUPPORTED");
+  Builder.defineMacro("_RVALUE_REFERENCES_SUPPORTED");
+  Builder.defineMacro("_NATIVE_NULLPTR_SUPPORTED");
+}
+  }
 }

 } // namespace targets
 } // namespace clang


Index: clang/test/Preprocessor/predefined-win-macros.c
===
--- clang/test/Preprocessor/predefined-win-macros.c
+++ clang/test/Preprocessor/predefined-win-macros.c
@@ -120,9 +120,14 @@
 // CHECK-AMD64-MINGW: #define _WIN32 1
 // CHECK-AMD64-MINGW: #define _WIN64 1

+// RUN: %clang_cc1 -triple x86_64-windows-gnu %s -E -dM -o - -fms-extensions \
+// RUN:   | FileCheck -match-full-lines %s --check-prefix=CHECK-AMD64-MINGW-MSE
+
+// CHECK-AMD64-MINGW-MSE: #define _MSC_EXTENSIONS 1
+
 // RUN: %clang_cc1 -triple aarch64-windows-gnu %s -E -dM -o - \
 // RUN:   | FileCheck -match-full-lines %s --check-prefix=CHECK-ARM64-MINGW

 // CHECK-ARM64-MINGW-NOT: #define _M_ARM64 1
 // CHECK-ARM64-MINGW: #define WIN32 1
 // CHECK-ARM64-MINGW: #define WIN64 1
Index: clang/lib/Basic/Targets/OSTargets.cpp
===
--- clang/lib/Basic/Targets/OSTargets.cpp
+++ clang/lib/Basic/Targets/OSTargets.cpp
@@ -226,25 +226,15 @@
 }
   }

-  if (Opts.MicrosoftExt) {
-Builder.defineMacro("_MSC_EXTENSIONS");
-
-if (Opts.CPlusPlus11) {
-  Builder.defineMacro("_RVALUE_REFERENCES_V2_SUPPORTED");
-  Builder.defineMacro("_RVALUE_REFERENCES_SUPPORTED");
-  Builder.defineMacro("_NATIVE_NULLPTR_SUPPORTED");
-}
-  }
-
   if (!Opts.MSVolatile)
 Builder.defineMacro("_ISO_VOLATILE");

   if (Opts.Kernel)
 Builder.defineMacro("_KERNEL_MODE");

   Builder.defineMacro("_INTEGRAL_MAX_BITS", "64");
   Builder.defineMacro("__STDC_NO_THREADS__");

   // Starting with VS 2022 17.1, MSVC predefines the below macro to inform
   // users of the execution character set defined at compile time.
   // The value given is the Windows Code Page Identifier:
@@ -264,7 +254,17 @@
   else if (Triple.isKnownWindowsMSVCEnvironment() ||
(Triple.i

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

[Aiden Grossman via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rGf3baf63d9a1b: Revert "[clang][X86] Add __cpuidex 
function to cpuid.h" (authored by aidengrossman).

Changed prior to commit:
  https://reviews.llvm.org/D157115?vs=547247&id=547329#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157115

Files:
  clang/lib/Headers/cpuid.h
  clang/test/Headers/__cpuidex_conflict.c
  clang/test/Headers/cpuid.c


Index: clang/test/Headers/cpuid.c
===
--- clang/test/Headers/cpuid.c
+++ clang/test/Headers/cpuid.c
@@ -6,19 +6,14 @@
 
 // CHECK-64: {{.*}} call { i32, i32, i32, i32 } asm "  xchgq  %rbx,${1:q}\0A 
cpuid\0A xchgq %rbx,${1:q}", 
"={ax},=r,={cx},={dx},0,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}})
 // CHECK-64: {{.*}} call { i32, i32, i32, i32 } asm "  xchgq  %rbx,${1:q}\0A  
cpuid\0A  xchgq  %rbx,${1:q}", 
"={ax},=r,={cx},={dx},0,2,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}}, i32 
%{{[a-z0-9]+}})
-// CHECK-64: {{.*}} call { i32, i32, i32, i32 } asm "  xchgq  %rbx,${1:q}\0A  
cpuid\0A  xchgq  %rbx,${1:q}", 
"={ax},=r,={cx},={dx},0,2,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}}, i32 
%{{[a-z0-9]+}})
 
 // CHECK-32: {{.*}} call { i32, i32, i32, i32 } asm "cpuid", 
"={ax},={bx},={cx},={dx},0,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}})
 // CHECK-32: {{.*}} call { i32, i32, i32, i32 } asm "cpuid", 
"={ax},={bx},={cx},={dx},0,2,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}}, 
i32 %{{[a-z0-9]+}})
-// CHECK-32: {{.*}} call { i32, i32, i32, i32 } asm "cpuid", 
"={ax},={bx},={cx},={dx},0,2,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}}, 
i32 %{{[a-z0-9]+}})
 
 unsigned eax0, ebx0, ecx0, edx0;
 unsigned eax1, ebx1, ecx1, edx1;
 
-int cpuid_info[4];
-
 void test_cpuid(unsigned level, unsigned count) {
   __cpuid(level, eax1, ebx1, ecx1, edx1);
   __cpuid_count(level, count, eax0, ebx0, ecx0, edx0);
-  __cpuidex(cpuid_info, level, count);
 }
Index: clang/test/Headers/__cpuidex_conflict.c
===
--- clang/test/Headers/__cpuidex_conflict.c
+++ /dev/null
@@ -1,15 +0,0 @@
-// Make sure that __cpuidex in cpuid.h doesn't conflict with the MS
-// compatibility built in by ensuring compilation succeeds:
-// RUN: %clang_cc1 %s -ffreestanding -fms-extensions -fms-compatibility \
-// RUN:  -fms-compatibility-version=19.00 -triple x86_64-pc-windows-msvc 
-emit-llvm -o -
-
-typedef __SIZE_TYPE__ size_t;
-
-#include 
-#include 
-
-int cpuid_info[4];
-
-void test_cpuidex(unsigned level, unsigned count) {
-  __cpuidex(cpuid_info, level, count);
-}
Index: clang/lib/Headers/cpuid.h
===
--- clang/lib/Headers/cpuid.h
+++ clang/lib/Headers/cpuid.h
@@ -328,14 +328,4 @@
 return 1;
 }
 
-// If MS extensions are enabled, __cpuidex is defined as a builtin which will
-// conflict with the __cpuidex definition below.
-#ifndef _MSC_EXTENSIONS
-static __inline void __cpuidex (int __cpu_info[4], int __leaf, int __subleaf)
-{
-  __cpuid_count(__leaf, __subleaf, __cpu_info[0], __cpu_info[1], __cpu_info[2],
-__cpu_info[3]);
-}
-#endif
-
 #endif /* __CPUID_H */


Index: clang/test/Headers/cpuid.c
===
--- clang/test/Headers/cpuid.c
+++ clang/test/Headers/cpuid.c
@@ -6,19 +6,14 @@
 
 // CHECK-64: {{.*}} call { i32, i32, i32, i32 } asm "  xchgq  %rbx,${1:q}\0A cpuid\0A xchgq %rbx,${1:q}", "={ax},=r,={cx},={dx},0,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}})
 // CHECK-64: {{.*}} call { i32, i32, i32, i32 } asm "  xchgq  %rbx,${1:q}\0A  cpuid\0A  xchgq  %rbx,${1:q}", "={ax},=r,={cx},={dx},0,2,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}}, i32 %{{[a-z0-9]+}})
-// CHECK-64: {{.*}} call { i32, i32, i32, i32 } asm "  xchgq  %rbx,${1:q}\0A  cpuid\0A  xchgq  %rbx,${1:q}", "={ax},=r,={cx},={dx},0,2,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}}, i32 %{{[a-z0-9]+}})
 
 // CHECK-32: {{.*}} call { i32, i32, i32, i32 } asm "cpuid", "={ax},={bx},={cx},={dx},0,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}})
 // CHECK-32: {{.*}} call { i32, i32, i32, i32 } asm "cpuid", "={ax},={bx},={cx},={dx},0,2,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}}, i32 %{{[a-z0-9]+}})
-// CHECK-32: {{.*}} call { i32, i32, i32, i32 } asm "cpuid", "={ax},={bx},={cx},={dx},0,2,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}}, i32 %{{[a-z0-9]+}})
 
 unsigned eax0, ebx0, ecx0, edx0;
 unsigned eax1, ebx1, ecx1, edx1;
 
-int cpuid_info[4];
-
 void test_cpuid(unsigned level, unsigned count) {
   __cpuid(level, eax1, ebx1, ecx1, edx1);
   __cpuid_count(level, count, eax0, ebx0, ecx0, edx0);
-  __cpuidex(cpuid_info, level, count);
 }
Index: clang/test/Headers/__cpuidex_conflict.c
=

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

[Aiden Grossman via Phabricator via cfe-commits]

aidengrossman added a comment.

Thanks for the quick response!

Sorry for all the churn with this patch. I'm fine leaving this out of LLVM 17, 
so I'll land this patch, backport the commit to the 17 release branch, and then 
once we have the aux triple and MS extensions flag fixes I'll reland in main 
and won't backport it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157115

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

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

[Aiden Grossman via Phabricator via cfe-commits]

aidengrossman added a comment.

Assuming everyone agrees to this going in, I'll land it and then open a 
backport for the 17 release and reland once the auxiliary triple issues are 
resolved.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157115

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

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

[Aiden Grossman via Phabricator via cfe-commits]

aidengrossman created this revision.
Herald added a project: All.
aidengrossman requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This reverts commit 2df77ac20a1ed996706b164b0c4ed5ad140f635f 
.

This has been causing some issues with some windows builds as `_MSC_EXTENSIONS` 
isn't defined when only `-fms-extensions` is set, but the builtin that 
conflicts with __cpuidex is. This was also causing problems as it exposed some 
latent issues with how auxiliary triples are handled in clang.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157115

Files:
  clang/lib/Headers/cpuid.h
  clang/test/Headers/__cpuidex_conflict.c
  clang/test/Headers/cpuid.c


Index: clang/test/Headers/cpuid.c
===
--- clang/test/Headers/cpuid.c
+++ clang/test/Headers/cpuid.c
@@ -6,19 +6,14 @@

 // CHECK-64: {{.*}} call { i32, i32, i32, i32 } asm "  xchgq  %rbx,${1:q}\0A 
cpuid\0A xchgq %rbx,${1:q}", 
"={ax},=r,={cx},={dx},0,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}})
 // CHECK-64: {{.*}} call { i32, i32, i32, i32 } asm "  xchgq  %rbx,${1:q}\0A  
cpuid\0A  xchgq  %rbx,${1:q}", 
"={ax},=r,={cx},={dx},0,2,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}}, i32 
%{{[a-z0-9]+}})
-// CHECK-64: {{.*}} call { i32, i32, i32, i32 } asm "  xchgq  %rbx,${1:q}\0A  
cpuid\0A  xchgq  %rbx,${1:q}", 
"={ax},=r,={cx},={dx},0,2,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}}, i32 
%{{[a-z0-9]+}})

 // CHECK-32: {{.*}} call { i32, i32, i32, i32 } asm "cpuid", 
"={ax},={bx},={cx},={dx},0,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}})
 // CHECK-32: {{.*}} call { i32, i32, i32, i32 } asm "cpuid", 
"={ax},={bx},={cx},={dx},0,2,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}}, 
i32 %{{[a-z0-9]+}})
-// CHECK-32: {{.*}} call { i32, i32, i32, i32 } asm "cpuid", 
"={ax},={bx},={cx},={dx},0,2,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}}, 
i32 %{{[a-z0-9]+}})

 unsigned eax0, ebx0, ecx0, edx0;
 unsigned eax1, ebx1, ecx1, edx1;

-int cpuid_info[4];
-
 void test_cpuid(unsigned level, unsigned count) {
   __cpuid(level, eax1, ebx1, ecx1, edx1);
   __cpuid_count(level, count, eax0, ebx0, ecx0, edx0);
-  __cpuidex(cpuid_info, level, count);
 }
Index: clang/test/Headers/__cpuidex_conflict.c
===
--- clang/test/Headers/__cpuidex_conflict.c
+++ /dev/null
@@ -1,15 +0,0 @@
-// Make sure that __cpuidex in cpuid.h doesn't conflict with the MS
-// compatibility built in by ensuring compilation succeeds:
-// RUN: %clang_cc1 %s -ffreestanding -fms-extensions -fms-compatibility \
-// RUN:  -fms-compatibility-version=19.00 -triple x86_64-pc-windows-msvc 
-emit-llvm -o -
-
-typedef __SIZE_TYPE__ size_t;
-
-#include 
-#include 
-
-int cpuid_info[4];
-
-void test_cpuidex(unsigned level, unsigned count) {
-  __cpuidex(cpuid_info, level, count);
-}
Index: clang/lib/Headers/cpuid.h
===
--- clang/lib/Headers/cpuid.h
+++ clang/lib/Headers/cpuid.h
@@ -328,14 +328,4 @@
 return 1;
 }

-// If MS extensions are enabled, __cpuidex is defined as a builtin which will
-// conflict with the __cpuidex definition below.
-#ifndef _MSC_EXTENSIONS
-static __inline void __cpuidex (int __cpu_info[4], int __leaf, int __subleaf)
-{
-  __cpuid_count(__leaf, __subleaf, __cpu_info[0], __cpu_info[1], __cpu_info[2],
-__cpu_info[3]);
-}
-#endif
-
 #endif /* __CPUID_H */


Index: clang/test/Headers/cpuid.c
===
--- clang/test/Headers/cpuid.c
+++ clang/test/Headers/cpuid.c
@@ -6,19 +6,14 @@

 // CHECK-64: {{.*}} call { i32, i32, i32, i32 } asm "  xchgq  %rbx,${1:q}\0A cpuid\0A xchgq %rbx,${1:q}", "={ax},=r,={cx},={dx},0,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}})
 // CHECK-64: {{.*}} call { i32, i32, i32, i32 } asm "  xchgq  %rbx,${1:q}\0A  cpuid\0A  xchgq  %rbx,${1:q}", "={ax},=r,={cx},={dx},0,2,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}}, i32 %{{[a-z0-9]+}})
-// CHECK-64: {{.*}} call { i32, i32, i32, i32 } asm "  xchgq  %rbx,${1:q}\0A  cpuid\0A  xchgq  %rbx,${1:q}", "={ax},=r,={cx},={dx},0,2,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}}, i32 %{{[a-z0-9]+}})

 // CHECK-32: {{.*}} call { i32, i32, i32, i32 } asm "cpuid", "={ax},={bx},={cx},={dx},0,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}})
 // CHECK-32: {{.*}} call { i32, i32, i32, i32 } asm "cpuid", "={ax},={bx},={cx},={dx},0,2,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}}, i32 %{{[a-z0-9]+}})
-// CHECK-32: {{.*}} call { i32, i32, i32, i32 } asm "cpuid", "={ax},={bx},={cx},={dx},0,2,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}}, i32 %{{[a-z0-9]+}})

 unsigned eax0, ebx0, ecx0, edx0;
 unsigned eax1, ebx1, ecx1, edx1;

-int cpuid_info[4];
-
 void test_cpuid(unsigned level, unsigned count) {
   __cpuid(level, eax1, ebx1, ecx1, edx1)

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

[Aiden Grossman via Phabricator via cfe-commits]

aidengrossman added a comment.

I'm not opposed to a revert, but I know some downstream users like MinGW have 
already adapted to this change so I'm not sure how much headache it would cause 
them to do a revert.

Maybe I'm not understanding things correctly, but I originally 
`_MSC_EXTENSIONS` patch as people were enabling the microsoft extensions (thus 
enabling the builtin) and still including `cpuid.h` which caused an error due 
to redefining a builtin function. There seems to be another issue there where 
multiple flags need to be set (`-fms-extensions`, `-fms-compatibility`, and 
`fms-compatibility-version`) in order to get `_MSC_EXTENSIONS` defined while 
only passing `fms-extensions` gets the builtin defined, but not the macro. I'm 
not sure if just passing `-fms-extensions` is even a valid configuration, but 
it does error out then. I believe that's a separate issue to the auxiliary 
triple issue you mentioned.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150646

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

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

[Aiden Grossman via Phabricator via cfe-commits]

aidengrossman added a comment.

In D150646#4450551 , @glandium wrote:

> Did you find something?

Not yet. I just finished finals a week ago so I haven't had as much time as I 
would've liked to get through this. I'll contact some people tonight in regards 
to the Windows flag situation and ask for their thoughts on this and hopefully 
have some sort of solution at least by next Monday.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150646

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

[PATCH] D152057: [CMake][Fuchsia] Add LLVM_ENABLE_HTTPLIB to Stage 2 build

[Aiden Grossman via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG2ff0aa207fd5: [CMake][Fuchsia] Add LLVM_ENABLE_HTTPLIB to 
Stage 2 build (authored by aidengrossman).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152057

Files:
  clang/cmake/caches/Fuchsia-stage2.cmake


Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -11,6 +11,7 @@
 
 set(LLVM_ENABLE_BACKTRACES OFF CACHE BOOL "")
 set(LLVM_ENABLE_DIA_SDK OFF CACHE BOOL "")
+set(LLVM_ENABLE_HTTPLIB ON CACHE BOOL "")
 set(LLVM_ENABLE_LIBCXX ON CACHE BOOL "")
 set(LLVM_ENABLE_LIBEDIT OFF CACHE BOOL "")
 set(LLVM_ENABLE_LLD ON CACHE BOOL "")


Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -11,6 +11,7 @@
 
 set(LLVM_ENABLE_BACKTRACES OFF CACHE BOOL "")
 set(LLVM_ENABLE_DIA_SDK OFF CACHE BOOL "")
+set(LLVM_ENABLE_HTTPLIB ON CACHE BOOL "")
 set(LLVM_ENABLE_LIBCXX ON CACHE BOOL "")
 set(LLVM_ENABLE_LIBEDIT OFF CACHE BOOL "")
 set(LLVM_ENABLE_LLD ON CACHE BOOL "")
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D152057: [CMake][Fuchsia] Add LLVM_ENABLE_HTTPLIB to Stage 2 build

[Aiden Grossman via Phabricator via cfe-commits]

aidengrossman created this revision.
Herald added subscribers: ekilmer, abrachet.
Herald added a project: All.
aidengrossman requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This patch sets the LLVM_ENABLE_HTTPLIB flag to ON in the stage 2 build
similar to how many of the other dependency flags are already specified.
This is necessary to configure the stage 2 build by itself, otherwise
the CMake configuration crashes.

This is currently causing the MLGO demo to fail since we're only using
stage 2 to avoid having to build stage 1 to save some compile time.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D152057

Files:
  clang/cmake/caches/Fuchsia-stage2.cmake


Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -11,6 +11,7 @@
 
 set(LLVM_ENABLE_BACKTRACES OFF CACHE BOOL "")
 set(LLVM_ENABLE_DIA_SDK OFF CACHE BOOL "")
+set(LLVM_ENABLE_HTTPLIB ON CACHE BOOL "")
 set(LLVM_ENABLE_LIBCXX ON CACHE BOOL "")
 set(LLVM_ENABLE_LIBEDIT OFF CACHE BOOL "")
 set(LLVM_ENABLE_LLD ON CACHE BOOL "")


Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -11,6 +11,7 @@
 
 set(LLVM_ENABLE_BACKTRACES OFF CACHE BOOL "")
 set(LLVM_ENABLE_DIA_SDK OFF CACHE BOOL "")
+set(LLVM_ENABLE_HTTPLIB ON CACHE BOOL "")
 set(LLVM_ENABLE_LIBCXX ON CACHE BOOL "")
 set(LLVM_ENABLE_LIBEDIT OFF CACHE BOOL "")
 set(LLVM_ENABLE_LLD ON CACHE BOOL "")
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Aiden Grossman via Phabricator via cfe-commits]

aidengrossman added a comment.

In D150646#4379543 , @glandium wrote:

> There seem to still be two problems with this change, with mingw:
>
> - with `-fms-extensions`:
>
>   echo '#include "cpuid.h"' | ./clang/bin/clang++ 
> --target=x86_64-w64-windows-gnu -xc++ - -fms-extensions
>   In file included from :1:
>   /tmp/clang/lib/clang/17/include/cpuid.h:334:22: error: definition of 
> builtin function '__cpuidex'
>   static __inline void __cpuidex (int __cpu_info[4], int __leaf, int 
> __subleaf)
>^
>   1 error generated.

Interesting. I would've thought `-fms-extensions` by itself would've declared 
that macro (which is what I gathered reading what caused the MSVC compatible 
builtins to get included in clang), but apparently not. For whatever reason, 
`LangOpts.MicrosoftExt` only seems to be set to true if 
`-fms-compaitilibty-version` (maybe on top of `-fms-compatibility`) are also 
passed (not 100% on this though). I'll have to do some more digging soon.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150646

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

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

[Aiden Grossman via Phabricator via cfe-commits]

aidengrossman added a comment.

Do you have any more information? Do you have your own implementation of 
`__cpuidex` somewhere else before including `cpuid.h`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150646

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

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

[Aiden Grossman 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 rG2df77ac20a1e: [clang][X86] Add __cpuidex function to cpuid.h 
(authored by aidengrossman).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150646

Files:
  clang/lib/Headers/cpuid.h
  clang/test/Headers/__cpuidex_conflict.c
  clang/test/Headers/cpuid.c


Index: clang/test/Headers/cpuid.c
===
--- clang/test/Headers/cpuid.c
+++ clang/test/Headers/cpuid.c
@@ -6,14 +6,19 @@
 
 // CHECK-64: {{.*}} call { i32, i32, i32, i32 } asm "  xchgq  %rbx,${1:q}\0A 
cpuid\0A xchgq %rbx,${1:q}", 
"={ax},=r,={cx},={dx},0,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}})
 // CHECK-64: {{.*}} call { i32, i32, i32, i32 } asm "  xchgq  %rbx,${1:q}\0A  
cpuid\0A  xchgq  %rbx,${1:q}", 
"={ax},=r,={cx},={dx},0,2,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}}, i32 
%{{[a-z0-9]+}})
+// CHECK-64: {{.*}} call { i32, i32, i32, i32 } asm "  xchgq  %rbx,${1:q}\0A  
cpuid\0A  xchgq  %rbx,${1:q}", 
"={ax},=r,={cx},={dx},0,2,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}}, i32 
%{{[a-z0-9]+}})
 
 // CHECK-32: {{.*}} call { i32, i32, i32, i32 } asm "cpuid", 
"={ax},={bx},={cx},={dx},0,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}})
 // CHECK-32: {{.*}} call { i32, i32, i32, i32 } asm "cpuid", 
"={ax},={bx},={cx},={dx},0,2,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}}, 
i32 %{{[a-z0-9]+}})
+// CHECK-32: {{.*}} call { i32, i32, i32, i32 } asm "cpuid", 
"={ax},={bx},={cx},={dx},0,2,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}}, 
i32 %{{[a-z0-9]+}})
 
 unsigned eax0, ebx0, ecx0, edx0;
 unsigned eax1, ebx1, ecx1, edx1;
 
+int cpuid_info[4];
+
 void test_cpuid(unsigned level, unsigned count) {
   __cpuid(level, eax1, ebx1, ecx1, edx1);
   __cpuid_count(level, count, eax0, ebx0, ecx0, edx0);
+  __cpuidex(cpuid_info, level, count);
 }
Index: clang/test/Headers/__cpuidex_conflict.c
===
--- /dev/null
+++ clang/test/Headers/__cpuidex_conflict.c
@@ -0,0 +1,15 @@
+// Make sure that __cpuidex in cpuid.h doesn't conflict with the MS
+// compatibility built in by ensuring compilation succeeds:
+// RUN: %clang_cc1 %s -ffreestanding -fms-extensions -fms-compatibility \
+// RUN:  -fms-compatibility-version=19.00 -triple x86_64-pc-windows-msvc 
-emit-llvm -o -
+
+typedef __SIZE_TYPE__ size_t;
+
+#include 
+#include 
+
+int cpuid_info[4];
+
+void test_cpuidex(unsigned level, unsigned count) {
+  __cpuidex(cpuid_info, level, count);
+}
Index: clang/lib/Headers/cpuid.h
===
--- clang/lib/Headers/cpuid.h
+++ clang/lib/Headers/cpuid.h
@@ -328,4 +328,14 @@
 return 1;
 }
 
+// If MS extensions are enabled, __cpuidex is defined as a builtin which will
+// conflict with the __cpuidex definition below.
+#ifndef _MSC_EXTENSIONS
+static __inline void __cpuidex (int __cpu_info[4], int __leaf, int __subleaf)
+{
+  __cpuid_count(__leaf, __subleaf, __cpu_info[0], __cpu_info[1], __cpu_info[2],
+__cpu_info[3]);
+}
+#endif
+
 #endif /* __CPUID_H */


Index: clang/test/Headers/cpuid.c
===
--- clang/test/Headers/cpuid.c
+++ clang/test/Headers/cpuid.c
@@ -6,14 +6,19 @@
 
 // CHECK-64: {{.*}} call { i32, i32, i32, i32 } asm "  xchgq  %rbx,${1:q}\0A cpuid\0A xchgq %rbx,${1:q}", "={ax},=r,={cx},={dx},0,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}})
 // CHECK-64: {{.*}} call { i32, i32, i32, i32 } asm "  xchgq  %rbx,${1:q}\0A  cpuid\0A  xchgq  %rbx,${1:q}", "={ax},=r,={cx},={dx},0,2,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}}, i32 %{{[a-z0-9]+}})
+// CHECK-64: {{.*}} call { i32, i32, i32, i32 } asm "  xchgq  %rbx,${1:q}\0A  cpuid\0A  xchgq  %rbx,${1:q}", "={ax},=r,={cx},={dx},0,2,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}}, i32 %{{[a-z0-9]+}})
 
 // CHECK-32: {{.*}} call { i32, i32, i32, i32 } asm "cpuid", "={ax},={bx},={cx},={dx},0,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}})
 // CHECK-32: {{.*}} call { i32, i32, i32, i32 } asm "cpuid", "={ax},={bx},={cx},={dx},0,2,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}}, i32 %{{[a-z0-9]+}})
+// CHECK-32: {{.*}} call { i32, i32, i32, i32 } asm "cpuid", "={ax},={bx},={cx},={dx},0,2,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}}, i32 %{{[a-z0-9]+}})
 
 unsigned eax0, ebx0, ecx0, edx0;
 unsigned eax1, ebx1, ecx1, edx1;
 
+int cpuid_info[4];
+
 void test_cpuid(unsigned level, unsigned count) {
   __cpuid(level, eax1, ebx1, ecx1, edx1);
   __cpuid_count(level, count, eax0, ebx0, ecx0, edx0);
+  __cpuidex(cpuid_info, level, count);
 }
Index: clang/test/Headers/__cpuidex_conflict.c
===
--- /dev/null
+++ clang/test/Headers/__cpuidex_conflict.c
@@ -0,0 +1,15 @@
+// Make su

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

[Aiden Grossman via Phabricator via cfe-commits]

aidengrossman updated this revision to Diff 523686.
aidengrossman added a comment.

Fix conflict with MS compatibility __cpuidex builtin. This update adds a
preprocessor guard around the __cpuidex definition in cpuid.h that checks
if _MSC_EXTENSIONS is defined. If _MSC_EXTENSIONS is defined, we can know
that the MS builtins are also included (see 
./clang/lib/Basic/Targets/OSTargets.cpp:229
and ./clang/lib/Basic/Builtins.cpp:91), so we shouldn't define the
function. Also adds in a test to make sure that everything works as expected
when MS extensions/compatibility is enabled.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150646

Files:
  clang/lib/Headers/cpuid.h
  clang/test/Headers/__cpuidex_conflict.c
  clang/test/Headers/cpuid.c


Index: clang/test/Headers/cpuid.c
===
--- clang/test/Headers/cpuid.c
+++ clang/test/Headers/cpuid.c
@@ -6,14 +6,19 @@
 
 // CHECK-64: {{.*}} call { i32, i32, i32, i32 } asm "  xchgq  %rbx,${1:q}\0A 
cpuid\0A xchgq %rbx,${1:q}", 
"={ax},=r,={cx},={dx},0,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}})
 // CHECK-64: {{.*}} call { i32, i32, i32, i32 } asm "  xchgq  %rbx,${1:q}\0A  
cpuid\0A  xchgq  %rbx,${1:q}", 
"={ax},=r,={cx},={dx},0,2,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}}, i32 
%{{[a-z0-9]+}})
+// CHECK-64: {{.*}} call { i32, i32, i32, i32 } asm "  xchgq  %rbx,${1:q}\0A  
cpuid\0A  xchgq  %rbx,${1:q}", 
"={ax},=r,={cx},={dx},0,2,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}}, i32 
%{{[a-z0-9]+}})
 
 // CHECK-32: {{.*}} call { i32, i32, i32, i32 } asm "cpuid", 
"={ax},={bx},={cx},={dx},0,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}})
 // CHECK-32: {{.*}} call { i32, i32, i32, i32 } asm "cpuid", 
"={ax},={bx},={cx},={dx},0,2,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}}, 
i32 %{{[a-z0-9]+}})
+// CHECK-32: {{.*}} call { i32, i32, i32, i32 } asm "cpuid", 
"={ax},={bx},={cx},={dx},0,2,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}}, 
i32 %{{[a-z0-9]+}})
 
 unsigned eax0, ebx0, ecx0, edx0;
 unsigned eax1, ebx1, ecx1, edx1;
 
+int cpuid_info[4];
+
 void test_cpuid(unsigned level, unsigned count) {
   __cpuid(level, eax1, ebx1, ecx1, edx1);
   __cpuid_count(level, count, eax0, ebx0, ecx0, edx0);
+  __cpuidex(cpuid_info, level, count);
 }
Index: clang/test/Headers/__cpuidex_conflict.c
===
--- /dev/null
+++ clang/test/Headers/__cpuidex_conflict.c
@@ -0,0 +1,15 @@
+// Make sure that __cpuidex in cpuid.h doesn't conflict with the MS
+// compatibility built in by ensuring compilation succeeds:
+// RUN: %clang_cc1 %s -ffreestanding -fms-extensions -fms-compatibility \
+// RUN:  -fms-compatibility-version=19.00 -triple x86_64-pc-windows-msvc 
-emit-llvm -o -
+
+typedef __SIZE_TYPE__ size_t;
+
+#include 
+#include 
+
+int cpuid_info[4];
+
+void test_cpuidex(unsigned level, unsigned count) {
+  __cpuidex(cpuid_info, level, count);
+}
Index: clang/lib/Headers/cpuid.h
===
--- clang/lib/Headers/cpuid.h
+++ clang/lib/Headers/cpuid.h
@@ -328,4 +328,14 @@
 return 1;
 }
 
+// If MS extensions are enabled, __cpuidex is defined as a builtin which will
+// conflict with the __cpuidex definition below.
+#ifndef _MSC_EXTENSIONS
+static __inline void __cpuidex (int __cpu_info[4], int __leaf, int __subleaf)
+{
+  __cpuid_count(__leaf, __subleaf, __cpu_info[0], __cpu_info[1], __cpu_info[2],
+__cpu_info[3]);
+}
+#endif
+
 #endif /* __CPUID_H */


Index: clang/test/Headers/cpuid.c
===
--- clang/test/Headers/cpuid.c
+++ clang/test/Headers/cpuid.c
@@ -6,14 +6,19 @@
 
 // CHECK-64: {{.*}} call { i32, i32, i32, i32 } asm "  xchgq  %rbx,${1:q}\0A cpuid\0A xchgq %rbx,${1:q}", "={ax},=r,={cx},={dx},0,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}})
 // CHECK-64: {{.*}} call { i32, i32, i32, i32 } asm "  xchgq  %rbx,${1:q}\0A  cpuid\0A  xchgq  %rbx,${1:q}", "={ax},=r,={cx},={dx},0,2,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}}, i32 %{{[a-z0-9]+}})
+// CHECK-64: {{.*}} call { i32, i32, i32, i32 } asm "  xchgq  %rbx,${1:q}\0A  cpuid\0A  xchgq  %rbx,${1:q}", "={ax},=r,={cx},={dx},0,2,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}}, i32 %{{[a-z0-9]+}})
 
 // CHECK-32: {{.*}} call { i32, i32, i32, i32 } asm "cpuid", "={ax},={bx},={cx},={dx},0,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}})
 // CHECK-32: {{.*}} call { i32, i32, i32, i32 } asm "cpuid", "={ax},={bx},={cx},={dx},0,2,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}}, i32 %{{[a-z0-9]+}})
+// CHECK-32: {{.*}} call { i32, i32, i32, i32 } asm "cpuid", "={ax},={bx},={cx},={dx},0,2,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}}, i32 %{{[a-z0-9]+}})
 
 unsigned eax0, ebx0, ecx0, edx0;
 unsigned eax1, ebx1, ecx1, edx1;
 
+int cpuid_info[4];
+
 void test_cpuid(unsigned level, unsigned count) {

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

[Aiden Grossman via Phabricator via cfe-commits]

aidengrossman added a comment.

Thanks for notifying me of this. I've reverted it as I don't have too much time 
currently to thoroughly go through things and I don't want to keep you blocked. 
Essentially, the built-in for `__cpuidex` on Windows platforms landed in 
https://reviews.llvm.org/D121653 which is conflicting with this new 
implementation (from what I can gather). If that's the issue though, it's 
interesting that it doesn't seem like there are any conflicts between the 
`__cpuid` builtin and the one defined in `cpuid.h`. Hoping to get a fix up 
(relatively) soon but not sure currently exactly what it will look like. 
@glandium Do you have a link to a source file that ends up triggering this 
error in Firefox? I'd be interested in seeing the context.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150646

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

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

[Aiden Grossman 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 rG286cefcf35d0: [clang][X86] Add __cpuidex function to cpuid.h 
(authored by aidengrossman).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150646

Files:
  clang/lib/Headers/cpuid.h
  clang/test/Headers/cpuid.c


Index: clang/test/Headers/cpuid.c
===
--- clang/test/Headers/cpuid.c
+++ clang/test/Headers/cpuid.c
@@ -6,14 +6,19 @@
 
 // CHECK-64: {{.*}} call { i32, i32, i32, i32 } asm "  xchgq  %rbx,${1:q}\0A 
cpuid\0A xchgq %rbx,${1:q}", 
"={ax},=r,={cx},={dx},0,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}})
 // CHECK-64: {{.*}} call { i32, i32, i32, i32 } asm "  xchgq  %rbx,${1:q}\0A  
cpuid\0A  xchgq  %rbx,${1:q}", 
"={ax},=r,={cx},={dx},0,2,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}}, i32 
%{{[a-z0-9]+}})
+// CHECK-64: {{.*}} call { i32, i32, i32, i32 } asm "  xchgq  %rbx,${1:q}\0A  
cpuid\0A  xchgq  %rbx,${1:q}", 
"={ax},=r,={cx},={dx},0,2,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}}, i32 
%{{[a-z0-9]+}})
 
 // CHECK-32: {{.*}} call { i32, i32, i32, i32 } asm "cpuid", 
"={ax},={bx},={cx},={dx},0,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}})
 // CHECK-32: {{.*}} call { i32, i32, i32, i32 } asm "cpuid", 
"={ax},={bx},={cx},={dx},0,2,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}}, 
i32 %{{[a-z0-9]+}})
+// CHECK-32: {{.*}} call { i32, i32, i32, i32 } asm "cpuid", 
"={ax},={bx},={cx},={dx},0,2,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}}, 
i32 %{{[a-z0-9]+}})
 
 unsigned eax0, ebx0, ecx0, edx0;
 unsigned eax1, ebx1, ecx1, edx1;
 
+int cpuid_info[4];
+
 void test_cpuid(unsigned level, unsigned count) {
   __cpuid(level, eax1, ebx1, ecx1, edx1);
   __cpuid_count(level, count, eax0, ebx0, ecx0, edx0);
+  __cpuidex(cpuid_info, level, count);
 }
Index: clang/lib/Headers/cpuid.h
===
--- clang/lib/Headers/cpuid.h
+++ clang/lib/Headers/cpuid.h
@@ -328,4 +328,10 @@
 return 1;
 }
 
+static __inline void __cpuidex (int __cpu_info[4], int __leaf, int __subleaf)
+{
+  __cpuid_count(__leaf, __subleaf, __cpu_info[0], __cpu_info[1], __cpu_info[2],
+__cpu_info[3]);
+}
+
 #endif /* __CPUID_H */


Index: clang/test/Headers/cpuid.c
===
--- clang/test/Headers/cpuid.c
+++ clang/test/Headers/cpuid.c
@@ -6,14 +6,19 @@
 
 // CHECK-64: {{.*}} call { i32, i32, i32, i32 } asm "  xchgq  %rbx,${1:q}\0A cpuid\0A xchgq %rbx,${1:q}", "={ax},=r,={cx},={dx},0,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}})
 // CHECK-64: {{.*}} call { i32, i32, i32, i32 } asm "  xchgq  %rbx,${1:q}\0A  cpuid\0A  xchgq  %rbx,${1:q}", "={ax},=r,={cx},={dx},0,2,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}}, i32 %{{[a-z0-9]+}})
+// CHECK-64: {{.*}} call { i32, i32, i32, i32 } asm "  xchgq  %rbx,${1:q}\0A  cpuid\0A  xchgq  %rbx,${1:q}", "={ax},=r,={cx},={dx},0,2,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}}, i32 %{{[a-z0-9]+}})
 
 // CHECK-32: {{.*}} call { i32, i32, i32, i32 } asm "cpuid", "={ax},={bx},={cx},={dx},0,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}})
 // CHECK-32: {{.*}} call { i32, i32, i32, i32 } asm "cpuid", "={ax},={bx},={cx},={dx},0,2,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}}, i32 %{{[a-z0-9]+}})
+// CHECK-32: {{.*}} call { i32, i32, i32, i32 } asm "cpuid", "={ax},={bx},={cx},={dx},0,2,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}}, i32 %{{[a-z0-9]+}})
 
 unsigned eax0, ebx0, ecx0, edx0;
 unsigned eax1, ebx1, ecx1, edx1;
 
+int cpuid_info[4];
+
 void test_cpuid(unsigned level, unsigned count) {
   __cpuid(level, eax1, ebx1, ecx1, edx1);
   __cpuid_count(level, count, eax0, ebx0, ecx0, edx0);
+  __cpuidex(cpuid_info, level, count);
 }
Index: clang/lib/Headers/cpuid.h
===
--- clang/lib/Headers/cpuid.h
+++ clang/lib/Headers/cpuid.h
@@ -328,4 +328,10 @@
 return 1;
 }
 
+static __inline void __cpuidex (int __cpu_info[4], int __leaf, int __subleaf)
+{
+  __cpuid_count(__leaf, __subleaf, __cpu_info[0], __cpu_info[1], __cpu_info[2],
+__cpu_info[3]);
+}
+
 #endif /* __CPUID_H */
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Aiden Grossman via Phabricator via cfe-commits]

aidengrossman added a comment.

Thanks for the review! Definitely a little bit odd and it probably would've 
been better if originally replicated in some other way, but definitely agree on 
the matching behavior.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150646

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

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

[Aiden Grossman via Phabricator via cfe-commits]

aidengrossman created this revision.
Herald added a project: All.
aidengrossman requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

MSVC has a __cpuidex function implemented to call the underlying cpuid
instruction which accepts a leaf, subleaf, and data array that the output
data is written into. This patch adds this functionality into clang
under the cpuid.h header. This also makes clang match GCC's behavior.
GCC has had __cpuidex in its cpuid.h since 2020.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D150646

Files:
  clang/lib/Headers/cpuid.h
  clang/test/Headers/cpuid.c


Index: clang/test/Headers/cpuid.c
===
--- clang/test/Headers/cpuid.c
+++ clang/test/Headers/cpuid.c
@@ -6,14 +6,19 @@
 
 // CHECK-64: {{.*}} call { i32, i32, i32, i32 } asm "  xchgq  %rbx,${1:q}\0A 
cpuid\0A xchgq %rbx,${1:q}", 
"={ax},=r,={cx},={dx},0,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}})
 // CHECK-64: {{.*}} call { i32, i32, i32, i32 } asm "  xchgq  %rbx,${1:q}\0A  
cpuid\0A  xchgq  %rbx,${1:q}", 
"={ax},=r,={cx},={dx},0,2,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}}, i32 
%{{[a-z0-9]+}})
+// CHECK-64: {{.*}} call { i32, i32, i32, i32 } asm "  xchgq  %rbx,${1:q}\0A  
cpuid\0A  xchgq  %rbx,${1:q}", 
"={ax},=r,={cx},={dx},0,2,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}}, i32 
%{{[a-z0-9]+}})
 
 // CHECK-32: {{.*}} call { i32, i32, i32, i32 } asm "cpuid", 
"={ax},={bx},={cx},={dx},0,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}})
 // CHECK-32: {{.*}} call { i32, i32, i32, i32 } asm "cpuid", 
"={ax},={bx},={cx},={dx},0,2,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}}, 
i32 %{{[a-z0-9]+}})
+// CHECK-32: {{.*}} call { i32, i32, i32, i32 } asm "cpuid", 
"={ax},={bx},={cx},={dx},0,2,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}}, 
i32 %{{[a-z0-9]+}})
 
 unsigned eax0, ebx0, ecx0, edx0;
 unsigned eax1, ebx1, ecx1, edx1;
 
+int cpuid_info[4];
+
 void test_cpuid(unsigned level, unsigned count) {
   __cpuid(level, eax1, ebx1, ecx1, edx1);
   __cpuid_count(level, count, eax0, ebx0, ecx0, edx0);
+  __cpuidex(cpuid_info, level, count);
 }
Index: clang/lib/Headers/cpuid.h
===
--- clang/lib/Headers/cpuid.h
+++ clang/lib/Headers/cpuid.h
@@ -328,4 +328,10 @@
 return 1;
 }
 
+static __inline void __cpuidex (int __cpu_info[4], int __leaf, int __subleaf)
+{
+  __cpuid_count(__leaf, __subleaf, __cpu_info[0], __cpu_info[1], __cpu_info[2],
+__cpu_info[3]);
+}
+
 #endif /* __CPUID_H */


Index: clang/test/Headers/cpuid.c
===
--- clang/test/Headers/cpuid.c
+++ clang/test/Headers/cpuid.c
@@ -6,14 +6,19 @@
 
 // CHECK-64: {{.*}} call { i32, i32, i32, i32 } asm "  xchgq  %rbx,${1:q}\0A cpuid\0A xchgq %rbx,${1:q}", "={ax},=r,={cx},={dx},0,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}})
 // CHECK-64: {{.*}} call { i32, i32, i32, i32 } asm "  xchgq  %rbx,${1:q}\0A  cpuid\0A  xchgq  %rbx,${1:q}", "={ax},=r,={cx},={dx},0,2,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}}, i32 %{{[a-z0-9]+}})
+// CHECK-64: {{.*}} call { i32, i32, i32, i32 } asm "  xchgq  %rbx,${1:q}\0A  cpuid\0A  xchgq  %rbx,${1:q}", "={ax},=r,={cx},={dx},0,2,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}}, i32 %{{[a-z0-9]+}})
 
 // CHECK-32: {{.*}} call { i32, i32, i32, i32 } asm "cpuid", "={ax},={bx},={cx},={dx},0,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}})
 // CHECK-32: {{.*}} call { i32, i32, i32, i32 } asm "cpuid", "={ax},={bx},={cx},={dx},0,2,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}}, i32 %{{[a-z0-9]+}})
+// CHECK-32: {{.*}} call { i32, i32, i32, i32 } asm "cpuid", "={ax},={bx},={cx},={dx},0,2,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}}, i32 %{{[a-z0-9]+}})
 
 unsigned eax0, ebx0, ecx0, edx0;
 unsigned eax1, ebx1, ecx1, edx1;
 
+int cpuid_info[4];
+
 void test_cpuid(unsigned level, unsigned count) {
   __cpuid(level, eax1, ebx1, ecx1, edx1);
   __cpuid_count(level, count, eax0, ebx0, ecx0, edx0);
+  __cpuidex(cpuid_info, level, count);
 }
Index: clang/lib/Headers/cpuid.h
===
--- clang/lib/Headers/cpuid.h
+++ clang/lib/Headers/cpuid.h
@@ -328,4 +328,10 @@
 return 1;
 }
 
+static __inline void __cpuidex (int __cpu_info[4], int __leaf, int __subleaf)
+{
+  __cpuid_count(__leaf, __subleaf, __cpu_info[0], __cpu_info[1], __cpu_info[2],
+__cpu_info[3]);
+}
+
 #endif /* __CPUID_H */
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D149809: [Clang][Docs] Fix man page build

[Aiden Grossman via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGdb63fb5d45e0: [Clang][Docs] Fix man page build (authored by 
aidengrossman).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149809

Files:
  clang/docs/CMakeLists.txt


Index: clang/docs/CMakeLists.txt
===
--- clang/docs/CMakeLists.txt
+++ clang/docs/CMakeLists.txt
@@ -90,50 +90,60 @@
 endif()
 endif()
 
-function (gen_rst_file_from_td output_file td_option source docs_target)
+function (gen_rst_file_from_td output_file td_option source docs_targets)
   if (NOT EXISTS "${CMAKE_CURRENT_SOURCE_DIR}/${source}")
 message(FATAL_ERROR "Cannot find source file: ${source} in 
${CMAKE_CURRENT_SOURCE_DIR}")
   endif()
   get_filename_component(TABLEGEN_INCLUDE_DIR 
"${CMAKE_CURRENT_SOURCE_DIR}/${source}" DIRECTORY)
   list(APPEND LLVM_TABLEGEN_FLAGS "-I${TABLEGEN_INCLUDE_DIR}")
   clang_tablegen(${output_file} ${td_option} SOURCE ${source} TARGET 
"gen-${output_file}")
-  add_dependencies(${docs_target} "gen-${output_file}")
+  foreach(target ${docs_targets})
+add_dependencies(${target} gen-${output_file})
+  endforeach()
 endfunction()
 
 if (LLVM_ENABLE_SPHINX)
   include(AddSphinxTarget)
-  if (SPHINX_FOUND)
+  if (SPHINX_FOUND AND (${SPHINX_OUTPUT_HTML} OR ${SPHINX_OUTPUT_MAN}))
+# Copy rst files to build directory before generating the html
+# documentation.  Some of the rst files are generated, so they
+# only exist in the build directory.  Sphinx needs all files in
+# the same directory in order to generate the html, so we need to
+# copy all the non-gnerated rst files from the source to the build
+# directory before we run sphinx.
+add_custom_target(copy-clang-rst-docs
+  COMMAND "${CMAKE_COMMAND}" -E copy_directory
+  "${CMAKE_CURRENT_SOURCE_DIR}" "${CMAKE_CURRENT_BINARY_DIR}"
+
+  COMMAND "${CMAKE_COMMAND}" -E copy_if_different
+  "${CMAKE_CURRENT_SOURCE_DIR}/../CodeOwners.rst"
+  "${CMAKE_CURRENT_BINARY_DIR}"
+)
+
+set(docs_targets "")
+
 if (${SPHINX_OUTPUT_HTML})
   add_sphinx_target(html clang SOURCE_DIR "${CMAKE_CURRENT_BINARY_DIR}")
 
-  # Copy rst files to build directory before generating the html
-  # documentation.  Some of the rst files are generated, so they
-  # only exist in the build directory.  Sphinx needs all files in
-  # the same directory in order to generate the html, so we need to
-  # copy all the non-gnerated rst files from the source to the build
-  # directory before we run sphinx.
-  add_custom_target(copy-clang-rst-docs
-COMMAND "${CMAKE_COMMAND}" -E copy_directory
-"${CMAKE_CURRENT_SOURCE_DIR}" "${CMAKE_CURRENT_BINARY_DIR}"
-
-COMMAND "${CMAKE_COMMAND}" -E copy_if_different
-"${CMAKE_CURRENT_SOURCE_DIR}/../CodeOwners.rst"
-"${CMAKE_CURRENT_BINARY_DIR}"
-  )
-  add_dependencies(docs-clang-html copy-clang-rst-docs)
-
   add_custom_command(TARGET docs-clang-html POST_BUILD
 COMMAND "${CMAKE_COMMAND}" -E copy
 "${CMAKE_CURRENT_SOURCE_DIR}/LibASTMatchersReference.html"
 "${CMAKE_CURRENT_BINARY_DIR}/html/LibASTMatchersReference.html")
 
-  # Generated files
-  gen_rst_file_from_td(AttributeReference.rst -gen-attr-docs 
../include/clang/Basic/Attr.td docs-clang-html)
-  gen_rst_file_from_td(DiagnosticsReference.rst -gen-diag-docs 
../include/clang/Basic/Diagnostic.td docs-clang-html)
-  gen_rst_file_from_td(ClangCommandLineReference.rst -gen-opt-docs 
../include/clang/Driver/ClangOptionDocs.td docs-clang-html)
+  list(APPEND docs_targets "docs-clang-html")
 endif()
 if (${SPHINX_OUTPUT_MAN})
-  add_sphinx_target(man clang)
+  add_sphinx_target(man clang SOURCE_DIR "${CMAKE_CURRENT_BINARY_DIR}")
+  list(APPEND docs_targets "docs-clang-man")
 endif()
+
+# Generated files
+gen_rst_file_from_td(AttributeReference.rst -gen-attr-docs 
../include/clang/Basic/Attr.td "${docs_targets}")
+gen_rst_file_from_td(DiagnosticsReference.rst -gen-diag-docs 
../include/clang/Basic/Diagnostic.td "${docs_targets}")
+gen_rst_file_from_td(ClangCommandLineReference.rst -gen-opt-docs 
../include/clang/Driver/ClangOptionDocs.td "${docs_targets}")
+
+foreach(target ${docs_targets})
+  add_dependencies(${target} copy-clang-rst-docs)
+endforeach()
   endif()
 endif()


Index: clang/docs/CMakeLists.txt
===
--- clang/docs/CMakeLists.txt
+++ clang/docs/CMakeLists.txt
@@ -90,50 +90,60 @@
 endif()
 endif()
 
-function (gen_rst_file_from_td output_file td_option source docs_target)
+function (gen_rst_file_from_td output_file td_option source docs_targets)
   if (NOT EXISTS "${CMAKE_CURRENT_SOURCE_DIR}/${source}")
 message(FATAL_ERROR "Cannot find source file: ${source} 

[PATCH] D149809: [Clang][Docs] Fix man page build

[Aiden Grossman via Phabricator via cfe-commits]

aidengrossman added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149809

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

[PATCH] D149809: [Clang][Docs] Fix man page build

[Aiden Grossman via Phabricator via cfe-commits]

aidengrossman added a comment.

Thank you very much for your patience and writing everything out. That makes a 
lot more sense. The resulting code definitely has a lot more desirable 
properties. I've updated the diff in accordance with your suggestions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149809

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

[PATCH] D149809: [Clang][Docs] Fix man page build

[Aiden Grossman via Phabricator via cfe-commits]

aidengrossman updated this revision to Diff 519622.
aidengrossman marked 6 inline comments as done.
aidengrossman added a comment.

Adjust based on reviewer suggestions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149809

Files:
  clang/docs/CMakeLists.txt


Index: clang/docs/CMakeLists.txt
===
--- clang/docs/CMakeLists.txt
+++ clang/docs/CMakeLists.txt
@@ -90,50 +90,60 @@
 endif()
 endif()
 
-function (gen_rst_file_from_td output_file td_option source docs_target)
+function (gen_rst_file_from_td output_file td_option source docs_targets)
   if (NOT EXISTS "${CMAKE_CURRENT_SOURCE_DIR}/${source}")
 message(FATAL_ERROR "Cannot find source file: ${source} in 
${CMAKE_CURRENT_SOURCE_DIR}")
   endif()
   get_filename_component(TABLEGEN_INCLUDE_DIR 
"${CMAKE_CURRENT_SOURCE_DIR}/${source}" DIRECTORY)
   list(APPEND LLVM_TABLEGEN_FLAGS "-I${TABLEGEN_INCLUDE_DIR}")
   clang_tablegen(${output_file} ${td_option} SOURCE ${source} TARGET 
"gen-${output_file}")
-  add_dependencies(${docs_target} "gen-${output_file}")
+  foreach(target ${docs_targets})
+add_dependencies(${target} gen-${output_file})
+  endforeach()
 endfunction()
 
 if (LLVM_ENABLE_SPHINX)
   include(AddSphinxTarget)
-  if (SPHINX_FOUND)
+  if (SPHINX_FOUND AND (${SPHINX_OUTPUT_HTML} OR ${SPHINX_OUTPUT_MAN}))
+# Copy rst files to build directory before generating the html
+# documentation.  Some of the rst files are generated, so they
+# only exist in the build directory.  Sphinx needs all files in
+# the same directory in order to generate the html, so we need to
+# copy all the non-gnerated rst files from the source to the build
+# directory before we run sphinx.
+add_custom_target(copy-clang-rst-docs
+  COMMAND "${CMAKE_COMMAND}" -E copy_directory
+  "${CMAKE_CURRENT_SOURCE_DIR}" "${CMAKE_CURRENT_BINARY_DIR}"
+
+  COMMAND "${CMAKE_COMMAND}" -E copy_if_different
+  "${CMAKE_CURRENT_SOURCE_DIR}/../CodeOwners.rst"
+  "${CMAKE_CURRENT_BINARY_DIR}"
+)
+
+set(docs_targets "")
+
 if (${SPHINX_OUTPUT_HTML})
   add_sphinx_target(html clang SOURCE_DIR "${CMAKE_CURRENT_BINARY_DIR}")
 
-  # Copy rst files to build directory before generating the html
-  # documentation.  Some of the rst files are generated, so they
-  # only exist in the build directory.  Sphinx needs all files in
-  # the same directory in order to generate the html, so we need to
-  # copy all the non-gnerated rst files from the source to the build
-  # directory before we run sphinx.
-  add_custom_target(copy-clang-rst-docs
-COMMAND "${CMAKE_COMMAND}" -E copy_directory
-"${CMAKE_CURRENT_SOURCE_DIR}" "${CMAKE_CURRENT_BINARY_DIR}"
-
-COMMAND "${CMAKE_COMMAND}" -E copy_if_different
-"${CMAKE_CURRENT_SOURCE_DIR}/../CodeOwners.rst"
-"${CMAKE_CURRENT_BINARY_DIR}"
-  )
-  add_dependencies(docs-clang-html copy-clang-rst-docs)
-
   add_custom_command(TARGET docs-clang-html POST_BUILD
 COMMAND "${CMAKE_COMMAND}" -E copy
 "${CMAKE_CURRENT_SOURCE_DIR}/LibASTMatchersReference.html"
 "${CMAKE_CURRENT_BINARY_DIR}/html/LibASTMatchersReference.html")
 
-  # Generated files
-  gen_rst_file_from_td(AttributeReference.rst -gen-attr-docs 
../include/clang/Basic/Attr.td docs-clang-html)
-  gen_rst_file_from_td(DiagnosticsReference.rst -gen-diag-docs 
../include/clang/Basic/Diagnostic.td docs-clang-html)
-  gen_rst_file_from_td(ClangCommandLineReference.rst -gen-opt-docs 
../include/clang/Driver/ClangOptionDocs.td docs-clang-html)
+  list(APPEND docs_targets "docs-clang-html")
 endif()
 if (${SPHINX_OUTPUT_MAN})
-  add_sphinx_target(man clang)
+  add_sphinx_target(man clang SOURCE_DIR "${CMAKE_CURRENT_BINARY_DIR}")
+  list(APPEND docs_targets "docs-clang-man")
 endif()
+
+# Generated files
+gen_rst_file_from_td(AttributeReference.rst -gen-attr-docs 
../include/clang/Basic/Attr.td "${docs_targets}")
+gen_rst_file_from_td(DiagnosticsReference.rst -gen-diag-docs 
../include/clang/Basic/Diagnostic.td "${docs_targets}")
+gen_rst_file_from_td(ClangCommandLineReference.rst -gen-opt-docs 
../include/clang/Driver/ClangOptionDocs.td "${docs_targets}")
+
+foreach(target ${docs_targets})
+  add_dependencies(${target} copy-clang-rst-docs)
+endforeach()
   endif()
 endif()


Index: clang/docs/CMakeLists.txt
===
--- clang/docs/CMakeLists.txt
+++ clang/docs/CMakeLists.txt
@@ -90,50 +90,60 @@
 endif()
 endif()
 
-function (gen_rst_file_from_td output_file td_option source docs_target)
+function (gen_rst_file_from_td output_file td_option source docs_targets)
   if (NOT EXISTS "${CMAKE_CURRENT_SOURCE_DIR}/${source}")
 message(FATAL_ERROR "Cannot find source file: ${source}

[PATCH] D149809: [Clang][Docs] Fix man page build

[Aiden Grossman via Phabricator via cfe-commits]

aidengrossman added a comment.

In D149809#4317610 , @tstellar wrote:

> I think we could remove some of the duplication by making the docs_target 
> parameter into a list and passing both docs-clang-html and docs-clang-man to 
> it.

If I'm understanding things correctly we wouldn't be able to do this for either 
the `gen_docs_depends` or `gen_rst_file_from_td` functions because someone 
might have the HTML docs build turned on and the man pages build turned off or 
vice versa. We can only add dependencies to the target after it's created as 
far as I'm aware.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149809

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

[PATCH] D149809: [Clang][Docs] Fix man page build

[Aiden Grossman via Phabricator via cfe-commits]

aidengrossman added reviewers: aaron.ballman, tstellar, rnk.
aidengrossman added a comment.

Posting this as when I was setting up a new dev environment with 
`-DLLVM_ENABLE_SPHINX`, it defaults to having `SPHINX_OUTPUT_HTML` and 
`SPHINX_OUTPUT_MAN` on, and the man build was broken so when I tried to build 
the default target, it failed. This patch fixes the clang man page build.

Not sure this is the ideal approach. I'd really like to remove the redundancy 
between the generated targets reference in `gen_docs_depends` and their values 
as specified in the calls to `gen_rst_files_from_td`, but CMake doesn't really 
seem to have appropriate data structures (a list of tuples or something 
similar) to accomplish this. Very open to suggestions here though since I'm not 
that familiar with CMake.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149809

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

[PATCH] D149809: [Clang][Docs] Fix man page build

[Aiden Grossman via Phabricator via cfe-commits]

aidengrossman created this revision.
Herald added a project: All.
aidengrossman requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This patch fixes the man page build. It currently doesn't work as
SOURCE_DIR isn't set correctly (just undefined) within the
add_sphinx_target function. This patch also moves around the creation of
targets for autogenerated rst files so that both the man page and html
build can depend upon them as before only the html build depended on
them.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D149809

Files:
  clang/docs/CMakeLists.txt


Index: clang/docs/CMakeLists.txt
===
--- clang/docs/CMakeLists.txt
+++ clang/docs/CMakeLists.txt
@@ -90,50 +90,59 @@
 endif()
 endif()
 
-function (gen_rst_file_from_td output_file td_option source docs_target)
+function (gen_rst_file_from_td output_file td_option source)
   if (NOT EXISTS "${CMAKE_CURRENT_SOURCE_DIR}/${source}")
 message(FATAL_ERROR "Cannot find source file: ${source} in 
${CMAKE_CURRENT_SOURCE_DIR}")
   endif()
   get_filename_component(TABLEGEN_INCLUDE_DIR 
"${CMAKE_CURRENT_SOURCE_DIR}/${source}" DIRECTORY)
   list(APPEND LLVM_TABLEGEN_FLAGS "-I${TABLEGEN_INCLUDE_DIR}")
   clang_tablegen(${output_file} ${td_option} SOURCE ${source} TARGET 
"gen-${output_file}")
-  add_dependencies(${docs_target} "gen-${output_file}")
+endfunction()
+
+function (gen_docs_depends docs_target)
+  add_dependencies(${docs_target} copy-clang-rst-docs)
+
+  add_dependencies(${docs_target} "gen-AttributeReference.rst")
+  add_dependencies(${docs_target} "gen-DiagnosticsReference.rst")
+  add_dependencies(${docs_target} "gen-ClangCommandLineReference.rst")
 endfunction()
 
 if (LLVM_ENABLE_SPHINX)
   include(AddSphinxTarget)
-  if (SPHINX_FOUND)
+  if (SPHINX_FOUND AND (${SPHINX_OUTPUT_HTML} OR ${SPHINX_OUTPUT_MAN}))
+# Copy rst files to build directory before generating the html
+# documentation.  Some of the rst files are generated, so they
+# only exist in the build directory.  Sphinx needs all files in
+# the same directory in order to generate the html, so we need to
+# copy all the non-gnerated rst files from the source to the build
+# directory before we run sphinx.
+add_custom_target(copy-clang-rst-docs
+  COMMAND "${CMAKE_COMMAND}" -E copy_directory
+  "${CMAKE_CURRENT_SOURCE_DIR}" "${CMAKE_CURRENT_BINARY_DIR}"
+
+  COMMAND "${CMAKE_COMMAND}" -E copy_if_different
+  "${CMAKE_CURRENT_SOURCE_DIR}/../CodeOwners.rst"
+  "${CMAKE_CURRENT_BINARY_DIR}"
+)
+
+# Generated files
+gen_rst_file_from_td(AttributeReference.rst -gen-attr-docs 
../include/clang/Basic/Attr.td)
+gen_rst_file_from_td(DiagnosticsReference.rst -gen-diag-docs 
../include/clang/Basic/Diagnostic.td)
+gen_rst_file_from_td(ClangCommandLineReference.rst -gen-opt-docs 
../include/clang/Driver/ClangOptionDocs.td)
+
 if (${SPHINX_OUTPUT_HTML})
   add_sphinx_target(html clang SOURCE_DIR "${CMAKE_CURRENT_BINARY_DIR}")
 
-  # Copy rst files to build directory before generating the html
-  # documentation.  Some of the rst files are generated, so they
-  # only exist in the build directory.  Sphinx needs all files in
-  # the same directory in order to generate the html, so we need to
-  # copy all the non-gnerated rst files from the source to the build
-  # directory before we run sphinx.
-  add_custom_target(copy-clang-rst-docs
-COMMAND "${CMAKE_COMMAND}" -E copy_directory
-"${CMAKE_CURRENT_SOURCE_DIR}" "${CMAKE_CURRENT_BINARY_DIR}"
-
-COMMAND "${CMAKE_COMMAND}" -E copy_if_different
-"${CMAKE_CURRENT_SOURCE_DIR}/../CodeOwners.rst"
-"${CMAKE_CURRENT_BINARY_DIR}"
-  )
-  add_dependencies(docs-clang-html copy-clang-rst-docs)
-
   add_custom_command(TARGET docs-clang-html POST_BUILD
 COMMAND "${CMAKE_COMMAND}" -E copy
 "${CMAKE_CURRENT_SOURCE_DIR}/LibASTMatchersReference.html"
 "${CMAKE_CURRENT_BINARY_DIR}/html/LibASTMatchersReference.html")
 
-  # Generated files
-  gen_rst_file_from_td(AttributeReference.rst -gen-attr-docs 
../include/clang/Basic/Attr.td docs-clang-html)
-  gen_rst_file_from_td(DiagnosticsReference.rst -gen-diag-docs 
../include/clang/Basic/Diagnostic.td docs-clang-html)
-  gen_rst_file_from_td(ClangCommandLineReference.rst -gen-opt-docs 
../include/clang/Driver/ClangOptionDocs.td docs-clang-html)
+  gen_docs_depends(docs-clang-html)
 endif()
 if (${SPHINX_OUTPUT_MAN})
-  add_sphinx_target(man clang)
+  add_sphinx_target(man clang SOURCE_DIR "${CMAKE_CURRENT_BINARY_DIR}")
+  gen_docs_depends(docs-clang-man)
 endif()
   endif()
 endif()


Index: clang/docs/CMakeLists.txt
===
--- clang/docs/CMakeLists.txt
+++ clang/docs/CMakeLists.txt
@@ -90,50 +90,59 @@
 end

[PATCH] D132991: [Clang] Give error message for invalid profile path when compiling IR

[Aiden Grossman via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGc0bc461999fd: [Clang] Give error message for invalid profile 
path when compiling IR (authored by aidengrossman).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132991

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Profile/profile-does-not-exist-ir.c
  clang/test/Profile/profile-does-not-exist.c


Index: clang/test/Profile/profile-does-not-exist.c
===
--- clang/test/Profile/profile-does-not-exist.c
+++ clang/test/Profile/profile-does-not-exist.c
@@ -1,4 +1,4 @@
 // RUN: not %clang_cc1 -emit-llvm %s -o - 
-fprofile-instrument-use-path=%t.nonexistent.profdata 2>&1 | FileCheck %s
 
-// CHECK: error: Could not read profile {{.*}}.nonexistent.profdata:
+// CHECK: error: Error in reading profile {{.*}}.nonexistent.profdata:
 // CHECK-NOT: Assertion failed
Index: clang/test/Profile/profile-does-not-exist-ir.c
===
--- /dev/null
+++ clang/test/Profile/profile-does-not-exist-ir.c
@@ -0,0 +1,4 @@
+// RUN: not %clang_cc1 -emit-llvm -x ir %s -o - 
-fprofile-instrument-use-path=%t.nonexistent.profdata 2>&1 | FileCheck %s
+
+// CHECK: error: Error in reading profile {{.*}}.nonexistent.profdata:
+// CHECK-NOT: Assertion failed
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -1293,12 +1293,15 @@
 
 // Set the profile kind using fprofile-instrument-use-path.
 static void setPGOUseInstrumentor(CodeGenOptions &Opts,
-  const Twine &ProfileName) {
+  const Twine &ProfileName,
+  DiagnosticsEngine &Diags) {
   auto ReaderOrErr = llvm::IndexedInstrProfReader::create(ProfileName);
-  // In error, return silently and let Clang PGOUse report the error message.
   if (auto E = ReaderOrErr.takeError()) {
-llvm::consumeError(std::move(E));
-Opts.setProfileUse(CodeGenOptions::ProfileClangInstr);
+unsigned DiagID = Diags.getCustomDiagID(DiagnosticsEngine::Error,
+"Error in reading profile %0: %1");
+llvm::handleAllErrors(std::move(E), [&](const llvm::ErrorInfoBase &EI) {
+  Diags.Report(DiagID) << ProfileName.str() << EI.message();
+});
 return;
   }
   std::unique_ptr PGOReader =
@@ -1712,7 +1715,7 @@
   }
 
   if (!Opts.ProfileInstrumentUsePath.empty())
-setPGOUseInstrumentor(Opts, Opts.ProfileInstrumentUsePath);
+setPGOUseInstrumentor(Opts, Opts.ProfileInstrumentUsePath, Diags);
 
   if (const Arg *A = Args.getLastArg(OPT_ftime_report, OPT_ftime_report_EQ)) {
 Opts.TimePasses = true;
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -182,15 +182,11 @@
   if (CodeGenOpts.hasProfileClangUse()) {
 auto ReaderOrErr = llvm::IndexedInstrProfReader::create(
 CodeGenOpts.ProfileInstrumentUsePath, 
CodeGenOpts.ProfileRemappingFile);
-if (auto E = ReaderOrErr.takeError()) {
-  unsigned DiagID = Diags.getCustomDiagID(DiagnosticsEngine::Error,
-  "Could not read profile %0: %1");
-  llvm::handleAllErrors(std::move(E), [&](const llvm::ErrorInfoBase &EI) {
-getDiags().Report(DiagID) << CodeGenOpts.ProfileInstrumentUsePath
-  << EI.message();
-  });
-} else
-  PGOReader = std::move(ReaderOrErr.get());
+// We're checking for profile read errors in CompilerInvocation, so if
+// there was an error it should've already been caught. If it hasn't been
+// somehow, trip an assertion.
+assert(ReaderOrErr);
+PGOReader = std::move(ReaderOrErr.get());
   }
 
   // If coverage mapping generation is enabled, create the


Index: clang/test/Profile/profile-does-not-exist.c
===
--- clang/test/Profile/profile-does-not-exist.c
+++ clang/test/Profile/profile-does-not-exist.c
@@ -1,4 +1,4 @@
 // RUN: not %clang_cc1 -emit-llvm %s -o - -fprofile-instrument-use-path=%t.nonexistent.profdata 2>&1 | FileCheck %s
 
-// CHECK: error: Could not read profile {{.*}}.nonexistent.profdata:
+// CHECK: error: Error in reading profile {{.*}}.nonexistent.profdata:
 // CHECK-NOT: Assertion failed
Index: clang/test/Profile/profile-does-not-exist-ir.c
===
--- /dev/null
+++ clang/test/Profile/profile-does-not-exist-ir.c
@@ -0,0 +1,4 @@
+// RUN: not %clang_cc1 -emit-llvm -x ir %s -o - -fprofi

[PATCH] D132991: [Clang] Give error message for invalid profile path when compiling IR

[Aiden Grossman via Phabricator via cfe-commits]

aidengrossman added a comment.

@xur Gentle reminder. Just want to make sure I'm not missing anything obvious 
since I've made some somewhat substantial changes since your sign off.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132991

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

[PATCH] D132991: [Clang] Give error message for invalid profile path when compiling IR

[Aiden Grossman via Phabricator via cfe-commits]

aidengrossman added a comment.

In regards to the review, I have checked most cases, and I'm pretty confident 
this patch should cover all cases (given that `CompilerInvocation` is used 
directly from clang's `main()`), but if there are any edge cases that somehow 
pop up due to this patch, it would be due to switching to an `assert` within 
`CodeGenModule`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132991

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

[PATCH] D132991: [Clang] Give error message for invalid profile path when compiling IR

[Aiden Grossman via Phabricator via cfe-commits]

aidengrossman added a comment.

`CodeGenModule` doesn't get invoked when compiling IR. The error would trip 
when compiling other languages (eg c), but when passing IR to clang, the 
function doing error checking would never get called so no error was ever 
thrown.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132991

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

[PATCH] D132991: [Clang] Give error message for invalid profile path when compiling IR

[Aiden Grossman via Phabricator via cfe-commits]

aidengrossman added a comment.

@xur I've modified the patch slightly (mainly fixing tests and changing the 
error message printing in `CodeGenModule` to an assert as we should be 
capturing everything in `CompilerInvocation`. Do you mind looking over these 
changes, specifically making sure that going through CompilerInvocation is 
always guaranteed if we end up hitting `CodeGenModule`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132991

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

[PATCH] D132991: [Clang] Give error message for invalid profile path when compiling IR

[Aiden Grossman via Phabricator via cfe-commits]

aidengrossman updated this revision to Diff 457179.
aidengrossman added a comment.

Fix tests and replace error message in CodeGenModule with assertion since we're
now capturing the error message in CompileInvocation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132991

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Profile/profile-does-not-exist-ir.c
  clang/test/Profile/profile-does-not-exist.c


Index: clang/test/Profile/profile-does-not-exist.c
===
--- clang/test/Profile/profile-does-not-exist.c
+++ clang/test/Profile/profile-does-not-exist.c
@@ -1,4 +1,4 @@
 // RUN: not %clang_cc1 -emit-llvm %s -o - 
-fprofile-instrument-use-path=%t.nonexistent.profdata 2>&1 | FileCheck %s
 
-// CHECK: error: Could not read profile {{.*}}.nonexistent.profdata:
+// CHECK: error: Error in reading profile {{.*}}.nonexistent.profdata:
 // CHECK-NOT: Assertion failed
Index: clang/test/Profile/profile-does-not-exist-ir.c
===
--- /dev/null
+++ clang/test/Profile/profile-does-not-exist-ir.c
@@ -0,0 +1,4 @@
+// RUN: not %clang_cc1 -emit-llvm -x ir %s -o - 
-fprofile-instrument-use-path=%t.nonexistent.profdata 2>&1 | FileCheck %s
+
+// CHECK: error: Error in reading profile {{.*}}.nonexistent.profdata:
+// CHECK-NOT: Assertion failed
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -1293,12 +1293,15 @@
 
 // Set the profile kind using fprofile-instrument-use-path.
 static void setPGOUseInstrumentor(CodeGenOptions &Opts,
-  const Twine &ProfileName) {
+  const Twine &ProfileName,
+  DiagnosticsEngine &Diags) {
   auto ReaderOrErr = llvm::IndexedInstrProfReader::create(ProfileName);
-  // In error, return silently and let Clang PGOUse report the error message.
   if (auto E = ReaderOrErr.takeError()) {
-llvm::consumeError(std::move(E));
-Opts.setProfileUse(CodeGenOptions::ProfileClangInstr);
+unsigned DiagID = Diags.getCustomDiagID(DiagnosticsEngine::Error,
+"Error in reading profile %0: %1");
+llvm::handleAllErrors(std::move(E), [&](const llvm::ErrorInfoBase &EI) {
+  Diags.Report(DiagID) << ProfileName.str() << EI.message();
+});
 return;
   }
   std::unique_ptr PGOReader =
@@ -1717,7 +1720,7 @@
   }
 
   if (!Opts.ProfileInstrumentUsePath.empty())
-setPGOUseInstrumentor(Opts, Opts.ProfileInstrumentUsePath);
+setPGOUseInstrumentor(Opts, Opts.ProfileInstrumentUsePath, Diags);
 
   if (const Arg *A = Args.getLastArg(OPT_ftime_report, OPT_ftime_report_EQ)) {
 Opts.TimePasses = true;
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -182,15 +182,11 @@
   if (CodeGenOpts.hasProfileClangUse()) {
 auto ReaderOrErr = llvm::IndexedInstrProfReader::create(
 CodeGenOpts.ProfileInstrumentUsePath, 
CodeGenOpts.ProfileRemappingFile);
-if (auto E = ReaderOrErr.takeError()) {
-  unsigned DiagID = Diags.getCustomDiagID(DiagnosticsEngine::Error,
-  "Could not read profile %0: %1");
-  llvm::handleAllErrors(std::move(E), [&](const llvm::ErrorInfoBase &EI) {
-getDiags().Report(DiagID) << CodeGenOpts.ProfileInstrumentUsePath
-  << EI.message();
-  });
-} else
-  PGOReader = std::move(ReaderOrErr.get());
+// We're checking for profile read errors in CompilerInvocation, so if
+// there was an error it should've already been caught. If it hasn't been
+// somehow, trip an assertion.
+assert(ReaderOrErr);
+PGOReader = std::move(ReaderOrErr.get());
   }
 
   // If coverage mapping generation is enabled, create the


Index: clang/test/Profile/profile-does-not-exist.c
===
--- clang/test/Profile/profile-does-not-exist.c
+++ clang/test/Profile/profile-does-not-exist.c
@@ -1,4 +1,4 @@
 // RUN: not %clang_cc1 -emit-llvm %s -o - -fprofile-instrument-use-path=%t.nonexistent.profdata 2>&1 | FileCheck %s
 
-// CHECK: error: Could not read profile {{.*}}.nonexistent.profdata:
+// CHECK: error: Error in reading profile {{.*}}.nonexistent.profdata:
 // CHECK-NOT: Assertion failed
Index: clang/test/Profile/profile-does-not-exist-ir.c
===
--- /dev/null
+++ clang/test/Profile/profile-does-not-exist-ir.c
@@ -0,0 +1,4 @@
+// RUN: not %clang_cc1 -emit-llvm -x ir %s 

[PATCH] D132991: [Clang] Give error message for invalid profile path when compiling IR

[Aiden Grossman via Phabricator via cfe-commits]

aidengrossman updated this revision to Diff 457074.
aidengrossman added a comment.

Changed error message from "Could not read profile" to "Error in reading 
profile"
to better reflect the fact that profile reading can fail for multiple reasons, 
not just
when the file path cannot be read/does not exist.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132991

Files:
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Profile/profile-does-not-exist-ir.c


Index: clang/test/Profile/profile-does-not-exist-ir.c
===
--- /dev/null
+++ clang/test/Profile/profile-does-not-exist-ir.c
@@ -0,0 +1,4 @@
+// RUN: not %clang_cc1 -emit-llvm -x ir %s -o - 
-fprofile-instrument-use-path=%t.nonexistent.profdata 2>&1 | FileCheck %s
+
+// CHECK: error: Could not read profile {{.*}}.nonexistent.profdata:
+// CHECK-NOT: Assertion failed
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -1293,12 +1293,15 @@
 
 // Set the profile kind using fprofile-instrument-use-path.
 static void setPGOUseInstrumentor(CodeGenOptions &Opts,
-  const Twine &ProfileName) {
+  const Twine &ProfileName,
+  DiagnosticsEngine &Diags) {
   auto ReaderOrErr = llvm::IndexedInstrProfReader::create(ProfileName);
-  // In error, return silently and let Clang PGOUse report the error message.
   if (auto E = ReaderOrErr.takeError()) {
-llvm::consumeError(std::move(E));
-Opts.setProfileUse(CodeGenOptions::ProfileClangInstr);
+unsigned DiagID = Diags.getCustomDiagID(DiagnosticsEngine::Error,
+"Error in reading profile %0: %1");
+llvm::handleAllErrors(std::move(E), [&](const llvm::ErrorInfoBase &EI) {
+  Diags.Report(DiagID) << ProfileName.str() << EI.message();
+});
 return;
   }
   std::unique_ptr PGOReader =
@@ -1717,7 +1720,7 @@
   }
 
   if (!Opts.ProfileInstrumentUsePath.empty())
-setPGOUseInstrumentor(Opts, Opts.ProfileInstrumentUsePath);
+setPGOUseInstrumentor(Opts, Opts.ProfileInstrumentUsePath, Diags);
 
   if (const Arg *A = Args.getLastArg(OPT_ftime_report, OPT_ftime_report_EQ)) {
 Opts.TimePasses = true;


Index: clang/test/Profile/profile-does-not-exist-ir.c
===
--- /dev/null
+++ clang/test/Profile/profile-does-not-exist-ir.c
@@ -0,0 +1,4 @@
+// RUN: not %clang_cc1 -emit-llvm -x ir %s -o - -fprofile-instrument-use-path=%t.nonexistent.profdata 2>&1 | FileCheck %s
+
+// CHECK: error: Could not read profile {{.*}}.nonexistent.profdata:
+// CHECK-NOT: Assertion failed
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -1293,12 +1293,15 @@
 
 // Set the profile kind using fprofile-instrument-use-path.
 static void setPGOUseInstrumentor(CodeGenOptions &Opts,
-  const Twine &ProfileName) {
+  const Twine &ProfileName,
+  DiagnosticsEngine &Diags) {
   auto ReaderOrErr = llvm::IndexedInstrProfReader::create(ProfileName);
-  // In error, return silently and let Clang PGOUse report the error message.
   if (auto E = ReaderOrErr.takeError()) {
-llvm::consumeError(std::move(E));
-Opts.setProfileUse(CodeGenOptions::ProfileClangInstr);
+unsigned DiagID = Diags.getCustomDiagID(DiagnosticsEngine::Error,
+"Error in reading profile %0: %1");
+llvm::handleAllErrors(std::move(E), [&](const llvm::ErrorInfoBase &EI) {
+  Diags.Report(DiagID) << ProfileName.str() << EI.message();
+});
 return;
   }
   std::unique_ptr PGOReader =
@@ -1717,7 +1720,7 @@
   }
 
   if (!Opts.ProfileInstrumentUsePath.empty())
-setPGOUseInstrumentor(Opts, Opts.ProfileInstrumentUsePath);
+setPGOUseInstrumentor(Opts, Opts.ProfileInstrumentUsePath, Diags);
 
   if (const Arg *A = Args.getLastArg(OPT_ftime_report, OPT_ftime_report_EQ)) {
 Opts.TimePasses = true;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D132991: [Clang] Give error message for invalid profile path when compiling IR

[Aiden Grossman via Phabricator via cfe-commits]

aidengrossman added reviewers: xur, vsk.
aidengrossman added a subscriber: mtrofin.
aidengrossman added a comment.

Pinging reviewers based on previous commits in `setPGOUseInstrumentor`.

If there is an alternative method of writing this patch (eg placing the check 
in an equivalent of `CodeGenModule` for the IR side), let me know and I can 
adjust it. I'm not very familiar with the clang codebase, so there might be 
some inefficiencies in how I've implemented things.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132991

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

[PATCH] D132991: [Clang] Give error message for invalid profile path when compiling IR

[Aiden Grossman via Phabricator via cfe-commits]

aidengrossman created this revision.
Herald added a subscriber: wenlei.
Herald added a project: All.
aidengrossman requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Before this patch, when compiling an IR file (eg the .llvmbc section
from an object file compiled with -Xclang -fembed-bitcode=all) and
profile data was passed in using the -fprofile-instrument-use-path
flag, there would be no error printed (as the previous implementation
relied on the error getting caught again in the constructor of
CodeGenModule which isn't called when -x ir is set). This patch
moves the error checking directly to where the error is caught
originally rather than failing silently in setPGOUseInstrumentor and
waiting to catch it in CodeGenModule to print diagnostic information to
the user.

Regression test added.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D132991

Files:
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Profile/profile-does-not-exist-ir.c


Index: clang/test/Profile/profile-does-not-exist-ir.c
===
--- /dev/null
+++ clang/test/Profile/profile-does-not-exist-ir.c
@@ -0,0 +1,4 @@
+// RUN: not %clang_cc1 -emit-llvm -x ir %s -o - 
-fprofile-instrument-use-path=%t.nonexistent.profdata 2>&1 | FileCheck %s
+
+// CHECK: error: Could not read profile {{.*}}.nonexistent.profdata:
+// CHECK-NOT: Assertion failed
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -1293,12 +1293,15 @@
 
 // Set the profile kind using fprofile-instrument-use-path.
 static void setPGOUseInstrumentor(CodeGenOptions &Opts,
-  const Twine &ProfileName) {
+  const Twine &ProfileName,
+  DiagnosticsEngine &Diags) {
   auto ReaderOrErr = llvm::IndexedInstrProfReader::create(ProfileName);
-  // In error, return silently and let Clang PGOUse report the error message.
   if (auto E = ReaderOrErr.takeError()) {
-llvm::consumeError(std::move(E));
-Opts.setProfileUse(CodeGenOptions::ProfileClangInstr);
+unsigned DiagID = Diags.getCustomDiagID(DiagnosticsEngine::Error,
+"Could not read profile %0: %1");
+llvm::handleAllErrors(std::move(E), [&](const llvm::ErrorInfoBase &EI) {
+  Diags.Report(DiagID) << ProfileName.str() << EI.message();
+});
 return;
   }
   std::unique_ptr PGOReader =
@@ -1717,7 +1720,7 @@
   }
 
   if (!Opts.ProfileInstrumentUsePath.empty())
-setPGOUseInstrumentor(Opts, Opts.ProfileInstrumentUsePath);
+setPGOUseInstrumentor(Opts, Opts.ProfileInstrumentUsePath, Diags);
 
   if (const Arg *A = Args.getLastArg(OPT_ftime_report, OPT_ftime_report_EQ)) {
 Opts.TimePasses = true;


Index: clang/test/Profile/profile-does-not-exist-ir.c
===
--- /dev/null
+++ clang/test/Profile/profile-does-not-exist-ir.c
@@ -0,0 +1,4 @@
+// RUN: not %clang_cc1 -emit-llvm -x ir %s -o - -fprofile-instrument-use-path=%t.nonexistent.profdata 2>&1 | FileCheck %s
+
+// CHECK: error: Could not read profile {{.*}}.nonexistent.profdata:
+// CHECK-NOT: Assertion failed
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -1293,12 +1293,15 @@
 
 // Set the profile kind using fprofile-instrument-use-path.
 static void setPGOUseInstrumentor(CodeGenOptions &Opts,
-  const Twine &ProfileName) {
+  const Twine &ProfileName,
+  DiagnosticsEngine &Diags) {
   auto ReaderOrErr = llvm::IndexedInstrProfReader::create(ProfileName);
-  // In error, return silently and let Clang PGOUse report the error message.
   if (auto E = ReaderOrErr.takeError()) {
-llvm::consumeError(std::move(E));
-Opts.setProfileUse(CodeGenOptions::ProfileClangInstr);
+unsigned DiagID = Diags.getCustomDiagID(DiagnosticsEngine::Error,
+"Could not read profile %0: %1");
+llvm::handleAllErrors(std::move(E), [&](const llvm::ErrorInfoBase &EI) {
+  Diags.Report(DiagID) << ProfileName.str() << EI.message();
+});
 return;
   }
   std::unique_ptr PGOReader =
@@ -1717,7 +1720,7 @@
   }
 
   if (!Opts.ProfileInstrumentUsePath.empty())
-setPGOUseInstrumentor(Opts, Opts.ProfileInstrumentUsePath);
+setPGOUseInstrumentor(Opts, Opts.ProfileInstrumentUsePath, Diags);
 
   if (const Arg *A = Args.getLastArg(OPT_ftime_report, OPT_ftime_report_EQ)) {
 Opts.TimePasses = true;
___
cfe-commits mailing list
cfe-co

[PATCH] D130620: Fix lack of cc1 flag in llvmcmd sections when assertions are enabled

[Aiden Grossman via Phabricator via cfe-commits]

aidengrossman added a comment.

Thanks for the review and your suggestions. Do you mind pushing this commit? I 
don't currently have commit access to LLVM. Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130620

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

[PATCH] D130620: Fix lack of cc1 flag in llvmcmd sections when assertions are enabled

[Aiden Grossman via Phabricator via cfe-commits]

aidengrossman updated this revision to Diff 448477.
aidengrossman added a comment.

Rebased against main and adjusted unit tests to check directly
for the cc1 flag in CmdArgs which is directly consumed by the
bitcode writer while creating the .llvmcmd section. Also changed
the flags to better match what is already getting passed in.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130620

Files:
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/unittests/Frontend/CompilerInvocationTest.cpp


Index: clang/unittests/Frontend/CompilerInvocationTest.cpp
===
--- clang/unittests/Frontend/CompilerInvocationTest.cpp
+++ clang/unittests/Frontend/CompilerInvocationTest.cpp
@@ -23,6 +23,7 @@
 using ::testing::Contains;
 using ::testing::HasSubstr;
 using ::testing::StrEq;
+using ::testing::StartsWith;
 
 namespace {
 class CommandLineTest : public ::testing::Test {
@@ -145,6 +146,26 @@
   ASSERT_THAT(GeneratedArgs, Contains(StrEq("-fno-temp-file")));
 }
 
+TEST_F(CommandLineTest, CC1FlagPresentWhenDoingRoundTrip) {
+  const char *Args[] = {"-cc1", "-round-trip-args"};
+
+  ASSERT_TRUE(CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags));
+
+  ASSERT_THAT(std::string(Invocation.getCodeGenOpts().CmdArgs.begin(),
+  Invocation.getCodeGenOpts().CmdArgs.end()),
+  StartsWith("-cc1"));
+}
+
+TEST_F(CommandLineTest, CC1FlagPresentWhenNotDoingRoundTrip) {
+  const char *Args[] = {"-cc1", "-no-round-trip-args"};
+
+  ASSERT_TRUE(CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags));
+
+  ASSERT_THAT(std::string(Invocation.getCodeGenOpts().CmdArgs.begin(),
+  Invocation.getCodeGenOpts().CmdArgs.end()),
+  StartsWith("-cc1"));
+}
+
 TEST_F(CommandLineTest, BoolOptionDefaultTrueSingleFlagUnknownPresent) {
   const char *Args[] = {"-ftemp-file"};
 
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -4544,7 +4544,10 @@
 return CreateFromArgsImpl(Invocation, CommandLineArgs, Diags, Argv0);
   },
   [](CompilerInvocation &Invocation, SmallVectorImpl &Args,
- StringAllocator SA) { Invocation.generateCC1CommandLine(Args, SA); },
+ StringAllocator SA) {
+Args.push_back("-cc1");
+Invocation.generateCC1CommandLine(Args, SA);
+  },
   Invocation, DummyInvocation, CommandLineArgs, Diags, Argv0);
 }
 


Index: clang/unittests/Frontend/CompilerInvocationTest.cpp
===
--- clang/unittests/Frontend/CompilerInvocationTest.cpp
+++ clang/unittests/Frontend/CompilerInvocationTest.cpp
@@ -23,6 +23,7 @@
 using ::testing::Contains;
 using ::testing::HasSubstr;
 using ::testing::StrEq;
+using ::testing::StartsWith;
 
 namespace {
 class CommandLineTest : public ::testing::Test {
@@ -145,6 +146,26 @@
   ASSERT_THAT(GeneratedArgs, Contains(StrEq("-fno-temp-file")));
 }
 
+TEST_F(CommandLineTest, CC1FlagPresentWhenDoingRoundTrip) {
+  const char *Args[] = {"-cc1", "-round-trip-args"};
+
+  ASSERT_TRUE(CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags));
+
+  ASSERT_THAT(std::string(Invocation.getCodeGenOpts().CmdArgs.begin(),
+  Invocation.getCodeGenOpts().CmdArgs.end()),
+  StartsWith("-cc1"));
+}
+
+TEST_F(CommandLineTest, CC1FlagPresentWhenNotDoingRoundTrip) {
+  const char *Args[] = {"-cc1", "-no-round-trip-args"};
+
+  ASSERT_TRUE(CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags));
+
+  ASSERT_THAT(std::string(Invocation.getCodeGenOpts().CmdArgs.begin(),
+  Invocation.getCodeGenOpts().CmdArgs.end()),
+  StartsWith("-cc1"));
+}
+
 TEST_F(CommandLineTest, BoolOptionDefaultTrueSingleFlagUnknownPresent) {
   const char *Args[] = {"-ftemp-file"};
 
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -4544,7 +4544,10 @@
 return CreateFromArgsImpl(Invocation, CommandLineArgs, Diags, Argv0);
   },
   [](CompilerInvocation &Invocation, SmallVectorImpl &Args,
- StringAllocator SA) { Invocation.generateCC1CommandLine(Args, SA); },
+ StringAllocator SA) {
+Args.push_back("-cc1");
+Invocation.generateCC1CommandLine(Args, SA);
+  },
   Invocation, DummyInvocation, CommandLineArgs, Diags, Argv0);
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130620: Fix lack of cc1 flag in llvmcmd sections when assertions are enabled

[Aiden Grossman via Phabricator via cfe-commits]

aidengrossman updated this revision to Diff 448152.
aidengrossman marked an inline comment as done.
aidengrossman added a comment.

Added two unit tests to ensure functionality is correct in both
the assertion case and the default case. Should be sufficient
to ensure correct functionality without utilizing regression tests.
However, if regression tests are also desired, let me know and I can
add one in.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130620

Files:
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/unittests/Frontend/CompilerInvocationTest.cpp


Index: clang/unittests/Frontend/CompilerInvocationTest.cpp
===
--- clang/unittests/Frontend/CompilerInvocationTest.cpp
+++ clang/unittests/Frontend/CompilerInvocationTest.cpp
@@ -145,6 +145,22 @@
   ASSERT_THAT(GeneratedArgs, Contains(StrEq("-fno-temp-file")));
 }
 
+TEST_F(CommandLineTest, CC1FlagPresentWhenDoingRoundTrip) {
+  const char *Args[] = {"-round-trip-args"};
+
+  ASSERT_TRUE(CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags));
+
+  ASSERT_THAT(GeneratedArgs, Contains(StrEq("-cc1")));
+}
+
+TEST_F(CommandLineTest, CC1FlagPresentWhenNotDoingRoundTrip) {
+  const char *Args[] = {"-no-round-trip-args", "-cc1"};
+
+  ASSERT_TRUE(CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags));
+
+  ASSERT_THAT(GeneratedArgs, Contains(StrEq("-cc1")));
+}
+
 TEST_F(CommandLineTest, BoolOptionDefaultTrueSingleFlagUnknownPresent) {
   const char *Args[] = {"-ftemp-file"};
 
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -4544,7 +4544,10 @@
 return CreateFromArgsImpl(Invocation, CommandLineArgs, Diags, Argv0);
   },
   [](CompilerInvocation &Invocation, SmallVectorImpl &Args,
- StringAllocator SA) { Invocation.generateCC1CommandLine(Args, SA); },
+ StringAllocator SA) {
+Args.push_back("-cc1");
+Invocation.generateCC1CommandLine(Args, SA);
+  },
   Invocation, DummyInvocation, CommandLineArgs, Diags, Argv0);
 }
 


Index: clang/unittests/Frontend/CompilerInvocationTest.cpp
===
--- clang/unittests/Frontend/CompilerInvocationTest.cpp
+++ clang/unittests/Frontend/CompilerInvocationTest.cpp
@@ -145,6 +145,22 @@
   ASSERT_THAT(GeneratedArgs, Contains(StrEq("-fno-temp-file")));
 }
 
+TEST_F(CommandLineTest, CC1FlagPresentWhenDoingRoundTrip) {
+  const char *Args[] = {"-round-trip-args"};
+
+  ASSERT_TRUE(CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags));
+
+  ASSERT_THAT(GeneratedArgs, Contains(StrEq("-cc1")));
+}
+
+TEST_F(CommandLineTest, CC1FlagPresentWhenNotDoingRoundTrip) {
+  const char *Args[] = {"-no-round-trip-args", "-cc1"};
+
+  ASSERT_TRUE(CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags));
+
+  ASSERT_THAT(GeneratedArgs, Contains(StrEq("-cc1")));
+}
+
 TEST_F(CommandLineTest, BoolOptionDefaultTrueSingleFlagUnknownPresent) {
   const char *Args[] = {"-ftemp-file"};
 
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -4544,7 +4544,10 @@
 return CreateFromArgsImpl(Invocation, CommandLineArgs, Diags, Argv0);
   },
   [](CompilerInvocation &Invocation, SmallVectorImpl &Args,
- StringAllocator SA) { Invocation.generateCC1CommandLine(Args, SA); },
+ StringAllocator SA) {
+Args.push_back("-cc1");
+Invocation.generateCC1CommandLine(Args, SA);
+  },
   Invocation, DummyInvocation, CommandLineArgs, Diags, Argv0);
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130620: Fix lack of cc1 flag in llvmcmd sections when assertions are enabled

[Aiden Grossman via Phabricator via cfe-commits]

aidengrossman marked an inline comment as done.
aidengrossman added inline comments.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:4549
+Invocation.generateCC1CommandLine(Args, SA);
+Args.insert(Args.begin(), "-cc1");
+  },

jansvoboda11 wrote:
> This will shift all generated arguments in the vector. Could you do something 
> like this instead?
> 
> ```
> Args.push_back("-cc1");
> Invocation.generateCC1CommandLine(Args, SA);
> ```
> 
> I think `generateCC1CommandLine()` //appends// to `Args`, so this should be 
> safe to do.
Fixed. Thanks for the suggestion. Just tested it and everything looks good. For 
some reason when I was working on it this morning I didn't think it would be 
that easy.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130620

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

[PATCH] D130620: Fix lack of cc1 flag in llvmcmd sections when assertions are enabled

[Aiden Grossman via Phabricator via cfe-commits]

aidengrossman updated this revision to Diff 448132.
aidengrossman added a comment.

Moved insertion of cc1 flag to prevent shifting entire array


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130620

Files:
  clang/lib/Frontend/CompilerInvocation.cpp


Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -4544,7 +4544,10 @@
 return CreateFromArgsImpl(Invocation, CommandLineArgs, Diags, Argv0);
   },
   [](CompilerInvocation &Invocation, SmallVectorImpl &Args,
- StringAllocator SA) { Invocation.generateCC1CommandLine(Args, SA); },
+ StringAllocator SA) {
+Args.push_back("-cc1");
+Invocation.generateCC1CommandLine(Args, SA);
+  },
   Invocation, DummyInvocation, CommandLineArgs, Diags, Argv0);
 }
 


Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -4544,7 +4544,10 @@
 return CreateFromArgsImpl(Invocation, CommandLineArgs, Diags, Argv0);
   },
   [](CompilerInvocation &Invocation, SmallVectorImpl &Args,
- StringAllocator SA) { Invocation.generateCC1CommandLine(Args, SA); },
+ StringAllocator SA) {
+Args.push_back("-cc1");
+Invocation.generateCC1CommandLine(Args, SA);
+  },
   Invocation, DummyInvocation, CommandLineArgs, Diags, Argv0);
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130620: Fix lack of cc1 flag in llvmcmd sections when assertions are enabled

[Aiden Grossman via Phabricator via cfe-commits]

aidengrossman created this revision.
Herald added a project: All.
aidengrossman requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Currently when assertions are enabled, the cc1 flag is not
inserted into the llvmcmd section of object files with embedded
bitcode. This deviates from the normal behavior where this is
the first flag that is inserted. This error stems from incorrect
use of the function generateCC1CommandLine() which requires
manually adding in the -cc1 flag which is currently not done.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D130620

Files:
  clang/lib/Frontend/CompilerInvocation.cpp


Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -4544,7 +4544,10 @@
 return CreateFromArgsImpl(Invocation, CommandLineArgs, Diags, Argv0);
   },
   [](CompilerInvocation &Invocation, SmallVectorImpl &Args,
- StringAllocator SA) { Invocation.generateCC1CommandLine(Args, SA); },
+ StringAllocator SA) {
+Invocation.generateCC1CommandLine(Args, SA);
+Args.insert(Args.begin(), "-cc1");
+  },
   Invocation, DummyInvocation, CommandLineArgs, Diags, Argv0);
 }
 


Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -4544,7 +4544,10 @@
 return CreateFromArgsImpl(Invocation, CommandLineArgs, Diags, Argv0);
   },
   [](CompilerInvocation &Invocation, SmallVectorImpl &Args,
- StringAllocator SA) { Invocation.generateCC1CommandLine(Args, SA); },
+ StringAllocator SA) {
+Invocation.generateCC1CommandLine(Args, SA);
+Args.insert(Args.begin(), "-cc1");
+  },
   Invocation, DummyInvocation, CommandLineArgs, Diags, Argv0);
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits