[PATCH] D129572: [X86] initial -mfunction-return=thunk-extern support

2022-07-12 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers added inline comments. Comment at: clang/test/CodeGen/attr-function-return.c:42 +// CHECK: @double_keep_thunk2() [[EXTERN]] +[[gnu::function_return("thunk-keep")]][[gnu::function_return("thunk-extern")]] +void double_keep_thunk2(void) {} I just

[PATCH] D129572: [X86] initial -mfunction-return=thunk-extern support

2022-07-12 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D129572#3646074 , @MaskRay wrote: > In D129572#3646044 , @erichkeane > wrote: > >> In D129572#3646004 , >> @nickdesaulniers wrote: >>

[PATCH] D129572: [X86] initial -mfunction-return=thunk-extern support

2022-07-12 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D129572#3646044 , @erichkeane wrote: > In D129572#3646004 , > @nickdesaulniers wrote: > >> https://godbolt.org/z/rf16T83Kj >> >> IMO, the standards bodies focusing on standardizing

[PATCH] D129572: [X86] initial -mfunction-return=thunk-extern support

2022-07-12 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment. In D129572#3646004 , @nickdesaulniers wrote: > In D129572#3645934 , @erichkeane > wrote: > >> Our typical rule is to keep the 1st one I think, and reject the 2nd. > > But then the

[PATCH] D129572: [X86] initial -mfunction-return=thunk-extern support

2022-07-12 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers added a comment. In D129572#3645934 , @erichkeane wrote: > Our typical rule is to keep the 1st one I think, and reject the 2nd. But then the _codegen_ will differ from GCC. And we _want_ clang to be a drop in replacement, so differing

[PATCH] D129572: [X86] initial -mfunction-return=thunk-extern support

2022-07-12 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment. In D129572#3645917 , @nickdesaulniers wrote: > In D129572#3645895 , @erichkeane > wrote: > >> Did a post-commit review on the CFE changes, and all look OK to me. > > Thanks for the

[PATCH] D129572: [X86] initial -mfunction-return=thunk-extern support

2022-07-12 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers added a comment. In D129572#3645895 , @erichkeane wrote: > Did a post-commit review on the CFE changes, and all look OK to me. Thanks for the review! > That FIXME is a shame, we should see if we can fix that ASAP. We should AT >

[PATCH] D129572: [X86] initial -mfunction-return=thunk-extern support

2022-07-12 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment. Did a post-commit review on the CFE changes, and all look OK to me. That FIXME is a shame, we should see if we can fix that ASAP. We should AT LEAST document in the FIXME what semantics we are looking to emulate from GCC, and what those semantics ARE.

[PATCH] D129572: [X86] initial -mfunction-return=thunk-extern support

2022-07-12 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers added a comment. In D129572#3645660 , @nickdesaulniers wrote: > - add more links to commit message as they come in Oh, right, updating the commit message doesn't update the description in phab. Oops! Repository: rG LLVM Github

[PATCH] D129572: [X86] initial -mfunction-return=thunk-extern support

2022-07-12 Thread Nick Desaulniers 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 rG2240d72f15f3: [X86] initial -mfunction-return=thunk-extern support (authored by nickdesaulniers). Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D129572: [X86] initial -mfunction-return=thunk-extern support

2022-07-12 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers updated this revision to Diff 443986. nickdesaulniers added a comment. - add more links to commit message as they come in Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129572/new/ https://reviews.llvm.org/D129572 Files:

[PATCH] D129572: [X86] initial -mfunction-return=thunk-extern support

2022-07-12 Thread Craig Topper via Phabricator via cfe-commits
craig.topper accepted this revision. craig.topper added a comment. I also reviewed off-list and the X86 parts LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129572/new/ https://reviews.llvm.org/D129572

[PATCH] D129572: [X86] initial -mfunction-return=thunk-extern support

2022-07-12 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. > Many thanks for folks that provided discrete review off list due to the > embargoed nature of this hardware vulnerability. I was one of those off-list reviewers, and the Clang

[PATCH] D129572: [X86] initial -mfunction-return=thunk-extern support

2022-07-12 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers created this revision. Herald added subscribers: jsji, jdoerfert, pengfei, hiraditya, mgorny. Herald added a reviewer: aaron.ballman. Herald added a project: All. nickdesaulniers requested review of this revision. Herald added subscribers: llvm-commits, cfe-commits, MaskRay. Herald