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

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

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

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

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

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

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

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 >

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

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/

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

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.

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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/

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

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

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

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] D150646: [clang][X86] Add __cpuidex function to cpuid.h

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

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

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

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

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

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

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++ >

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

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/

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

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

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

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

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

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

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

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

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

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

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

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,

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

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/

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

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

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

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

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

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/

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

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

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

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

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

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

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

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

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

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/

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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/

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

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

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

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

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

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"); + },

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

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:

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

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