[PATCH] D69876: Allow output constraints on "asm goto"

void updated this revision to Diff 244841. void added a comment. Herald added a subscriber: martong. Typo Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69876/new/ https://reviews.llvm.org/D69876 Files: clang/docs/LanguageExtensions.rst

[PATCH] D69876: Allow output constraints on "asm goto"

void updated this revision to Diff 240720. void added a comment. Merge "__has_extension" patch into this patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69876/new/ https://reviews.llvm.org/D69876 Files: clang/docs/LanguageExtensions.rst

[PATCH] D69876: Allow output constraints on "asm goto"

void added a comment. In D69876#1843199 , @MaskRay wrote: > Does this depend on D69868 ? Yes. I added it as a parent. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D69876: Allow output constraints on "asm goto"

void updated this revision to Diff 240692. void added a comment. Fix extension example, found by eagle-eye @maskray! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69876/new/ https://reviews.llvm.org/D69876 Files:

[PATCH] D69876: Allow output constraints on "asm goto"

MaskRay added a comment. Does this depend on D69868 ? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69876/new/ https://reviews.llvm.org/D69876 ___ cfe-commits mailing

[PATCH] D69876: Allow output constraints on "asm goto"

void updated this revision to Diff 237223. void added a comment. Move plus constraints to after the label constraints. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69876/new/ https://reviews.llvm.org/D69876 Files:

[PATCH] D69876: Allow output constraints on "asm goto"

void marked an inline comment as done. void added a comment. In D69876#1812724 , @jyknight wrote: > Reopening, since this didn't actually land yet. BTW, this review still has > the wrong contents in the latest uploaded-diff (showing llvm changes, not >

[PATCH] D69876: Allow output constraints on "asm goto"

jyknight added inline comments. Comment at: clang/lib/AST/Stmt.cpp:646-648 + // Labels are placed after "InOut" operands. Adjust accordingly. + if (IsLabel) +N += getNumPlusOperands(); void wrote: > jyknight wrote: > > I'm confused about this

[PATCH] D69876: Allow output constraints on "asm goto"

jyknight reopened this revision. jyknight added a comment. This revision is now accepted and ready to land. Reopening, since this didn't actually land yet. BTW, this review still has the wrong contents in the latest uploaded-diff (showing llvm changes, not clang changes). Repository: rG

Re: [PATCH] D69876: Allow output constraints on "asm goto"

Yeah. I reverted the push. The correct code should be in Phabricator now. :-( On Tue, Jan 7, 2020 at 2:45 PM James Y Knight via Phabricator < revi...@reviews.llvm.org> wrote: > jyknight added a comment. > > I think you pushed the incorrect change? This was the clang half, but now > it's showing

[PATCH] D69876: Allow output constraints on "asm goto"

jyknight added a comment. I think you pushed the incorrect change? This was the clang half, but now it's showing the LLVM half. Can you re-upload the diff for the clang half? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69876/new/

[PATCH] D69876: Allow output constraints on "asm goto"

void updated this revision to Diff 236683. void added a comment. Push the correct changes to Phabricator. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69876/new/ https://reviews.llvm.org/D69876 Files: clang/docs/LanguageExtensions.rst

[PATCH] D69876: Allow output constraints on "asm goto"

This revision was automatically updated to reflect the committed changes. Closed by commit rG52366088a8e4: Allow output constraints on asm goto (authored by void). Changed prior to commit: https://reviews.llvm.org/D69876?vs=236683=236684#toc Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D69876: Allow output constraints on "asm goto"

void updated this revision to Diff 236482. void marked 2 inline comments as done. void added a comment. Reword the extension explanation to be more readable. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69876/new/ https://reviews.llvm.org/D69876

[PATCH] D69876: Allow output constraints on "asm goto"

void added inline comments. Comment at: clang/lib/AST/Stmt.cpp:646-648 + // Labels are placed after "InOut" operands. Adjust accordingly. + if (IsLabel) +N += getNumPlusOperands(); jyknight wrote: > I'm confused about this part. Why isn't the

[PATCH] D69876: Allow output constraints on "asm goto"

jyknight accepted this revision. jyknight added a comment. LGTM modulo some minor wording. Comment at: clang/docs/LanguageExtensions.rst:1256-1258 +Clang provides support for the `goto form of GCC's extended +assembly`_

[PATCH] D69876: Allow output constraints on "asm goto"

nickdesaulniers accepted this revision. nickdesaulniers added a comment. This revision is now accepted and ready to land. No, thanks for the work on this @void ! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69876/new/

[PATCH] D69876: Allow output constraints on "asm goto"

void marked an inline comment as done. void added inline comments. Comment at: clang/test/CodeGen/asm-goto.c:91 + return 1; +} nickdesaulniers wrote: > Thanks for adding this test. I think it doesn't test that `addr` is > *clobbered* though? The `+` modifier

[PATCH] D69876: Allow output constraints on "asm goto"

nickdesaulniers added inline comments. Comment at: clang/test/CodeGen/asm-goto.c:91 + return 1; +} Thanks for adding this test. I think it doesn't test that `addr` is *clobbered* though? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D69876: Allow output constraints on "asm goto"

void updated this revision to Diff 234852. void marked 5 inline comments as done. void added a comment. - Improve the asm goto note in the language extensions doc. - Include another testcase. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D69876: Allow output constraints on "asm goto"

void added inline comments. Comment at: clang/docs/LanguageExtensions.rst:1275 +It's important to note that outputs are valid only on the "fallthrough" branch. +For example, the value assigned to `y` is not valid in the above `err` block. + nickdesaulniers

[PATCH] D69876: Allow output constraints on "asm goto"

void added a comment. In D69876#1791989 , @nickdesaulniers wrote: > In D69876#1791663 , @void wrote: > > > I'm not getting the `Undefined temporary symbol` in your example (even with > > optimizations): > > > >

[PATCH] D69876: Allow output constraints on "asm goto"

nickdesaulniers added a comment. In D69876#1791663 , @void wrote: > I'm not getting the `Undefined temporary symbol` in your example (even with > optimizations): > > [morbo@fawn:llvm-project] clang -o - -S ../bug.c > `-S` is going the AsmPrinter

[PATCH] D69876: Allow output constraints on "asm goto"

void added a comment. I'm not getting the `Undefined temporary symbol` in your example (even with optimizations): [morbo@fawn:llvm-project] cat ../bug.c void baz(void) { int y; asm volatile goto ("mov 42, %0; ja %1": "=d"(y) ::: quux); asm volatile goto ("mov 42, %0; ja

[PATCH] D69876: Allow output constraints on "asm goto"

void added a comment. In D69876#1791475 , @nickdesaulniers wrote: > Thinking hard about this, I think there's four cases we really need to think > hard about: > > 1. asm redirects control flow BEFORE assigning to output. ie. > > ``` int foo(void) {

[PATCH] D69876: Allow output constraints on "asm goto"

nickdesaulniers requested changes to this revision. nickdesaulniers added a comment. This revision now requires changes to proceed. Thinking hard about this, I think there's four cases we really need to think hard about: 1. asm redirects control flow BEFORE assigning to output. ie. int

[PATCH] D69876: Allow output constraints on "asm goto"

void added a comment. Note to reviewers: I would *really* like to get this feature into the 10.0.0 release. That's coming up in January. Do you think you'll have your reviews finished by then? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D69876: Allow output constraints on "asm goto"

void marked an inline comment as done. void added inline comments. Comment at: clang/test/Analysis/uninit-asm-goto.cpp:10 +return -1; +} rnk wrote: > This could use a test for the case where an input is uninitialized, and where > an uninitialized value is

[PATCH] D69876: Allow output constraints on "asm goto"

rnk added a subscriber: rsmith. rnk added a comment. Changes seem fine to me, FWIW. +@rsmith Comment at: clang/test/Analysis/uninit-asm-goto.cpp:10 +return -1; +} This could use a test for the case where an input is uninitialized, and where an

[PATCH] D69876: Allow output constraints on "asm goto"

void added a comment. Any further comments? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69876/new/ https://reviews.llvm.org/D69876 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D69876: Allow output constraints on "asm goto"

void marked 2 inline comments as done. void added inline comments. Comment at: clang/lib/Analysis/UninitializedValues.cpp:831 + for (auto i = as->begin_outputs(), e = as->end_outputs(); i != e; ++i) +if (const VarDecl *VD = findVar(*i).getDecl()) + vals[VD] =

[PATCH] D69876: Allow output constraints on "asm goto"

void updated this revision to Diff 229921. void added a comment. Adjust loop. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69876/new/ https://reviews.llvm.org/D69876 Files: clang/docs/LanguageExtensions.rst clang/include/clang/AST/Stmt.h

[PATCH] D69876: Allow output constraints on "asm goto"

nickdesaulniers added inline comments. Comment at: clang/lib/Analysis/UninitializedValues.cpp:831 + for (auto i = as->begin_outputs(), e = as->end_outputs(); i != e; ++i) +if (const VarDecl *VD = findVar(*i).getDecl()) + vals[VD] = Initialized; this is

[PATCH] D69876: Allow output constraints on "asm goto"

void added a comment. Changes made. PTAL Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69876/new/ https://reviews.llvm.org/D69876 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D69876: Allow output constraints on "asm goto"

void updated this revision to Diff 229904. void marked 2 inline comments as done. void added a comment. Remove a "const_cast" and use iterators for output constraints. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69876/new/

[PATCH] D69876: Allow output constraints on "asm goto"

nickdesaulniers added inline comments. Comment at: clang/lib/Analysis/UninitializedValues.cpp:830 + + for (unsigned i = 0, e = as->getNumOutputs(); i != e; ++i) { +FindVarResult Var = findVar(as->getOutputExpr(i)); `GCCAsmStmt` inherits from `AsmStmt` which

[PATCH] D69876: Allow output constraints on "asm goto"

void added a comment. Friendly ping. :-) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69876/new/ https://reviews.llvm.org/D69876 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D69876: Allow output constraints on "asm goto"

void updated this revision to Diff 229166. void added a comment. Move adjustment before error check. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69876/new/ https://reviews.llvm.org/D69876 Files: clang/docs/LanguageExtensions.rst

[PATCH] D69876: Allow output constraints on "asm goto"

void updated this revision to Diff 229021. void added a comment. Adjust label references to account for in/out constraints. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69876/new/ https://reviews.llvm.org/D69876 Files:

[PATCH] D69876: Allow output constraints on "asm goto"

void updated this revision to Diff 228777. void added a comment. Add blurb about asm goto with output constraints to the "language extensions" documentation. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69876/new/ https://reviews.llvm.org/D69876

[PATCH] D69876: Allow output constraints on "asm goto"

void updated this revision to Diff 228775. void added a comment. the real update this time. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69876/new/ https://reviews.llvm.org/D69876 Files: clang/include/clang/AST/Stmt.h clang/lib/AST/Stmt.cpp

[PATCH] D69876: Allow output constraints on "asm goto"

void added a comment. In D69876#1741381 , @nickdesaulniers wrote: > > Nice catch! I updated the patch with a fix. PTAL. > > Please add a test for that! I did, but the "arc" program pushed the wrong change. :-( Fixed. When are we going to use GitHub's

[PATCH] D69876: Allow output constraints on "asm goto"

nickdesaulniers added a comment. In D69876#1741294 , @void wrote: > In D69876#1740286 , @jyknight wrote: > > > I think -Wuninitialized (UninitializedValues.cpp) should be taught how to > > detect the use of output

[PATCH] D69876: Allow output constraints on "asm goto"

void added a comment. In D69876#1740289 , @jyknight wrote: > Also, since this means we are no longer simply implementing according to > GCC's documentation, I think this means we'll need a brand new section in the > Clang docs for its inline-asm

[PATCH] D69876: Allow output constraints on "asm goto"

void added a comment. In D69876#1740286 , @jyknight wrote: > I think -Wuninitialized (UninitializedValues.cpp) should be taught how to > detect the use of output variables in error blocks, at least for trivial > cases. > > Actually, for some reason --

[PATCH] D69876: Allow output constraints on "asm goto"

void updated this revision to Diff 228762. void added a comment. Herald added subscribers: llvm-commits, hiraditya. Herald added a project: LLVM. Analyze "asm goto" for initialized variables. An "asm goto" statement is a terminator and thus doesn't indicate variable assignments like normal

[PATCH] D69876: Allow output constraints on "asm goto"

jyknight added a comment. Also, since this means we are no longer simply implementing according to GCC's documentation, I think this means we'll need a brand new section in the Clang docs for its inline-asm support. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D69876: Allow output constraints on "asm goto"

void marked an inline comment as done. void added a comment. This change is ready for review. PTAL. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69876/new/ https://reviews.llvm.org/D69876 ___

[PATCH] D69876: Allow output constraints on "asm goto"

jyknight added a comment. I think -Wuninitialized (UninitializedValues.cpp) should be taught how to detect the use of output variables in error blocks, at least for trivial cases. Actually, for some reason -- it looks like the warning is failing the wrong way right now, and emits an

[PATCH] D69876: Allow output constraints on "asm goto"

void updated this revision to Diff 228595. void added a comment. Adjust the ASM so that it references labels. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69876/new/ https://reviews.llvm.org/D69876 Files: clang/include/clang/AST/Stmt.h

[PATCH] D69876: Allow output constraints on "asm goto"

void updated this revision to Diff 228594. void added a comment. Emit asm goto fallthrough early so that any value extracts will show up in the fallthrough block and not directly after the callbr. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D69876: Allow output constraints on "asm goto"

void created this revision. void added reviewers: jyknight, nickdesaulniers, hfinkel. Herald added a project: clang. Herald added a subscriber: cfe-commits. THIS IS A WIP! Remove the restrictions that preventing "asm goto" from returning non-void values. The value returned by "asm goto" is only