[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-06-08 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers added a comment. For future travelers, we're tracking 2 related bugs: - `-no-integrated-as` providing different results: https://github.com/ClangBuiltLinux/linux/issues/500 - `IfConverter` `assert`ing on `INLINEASM_BR` `MachineInstr`s:

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-06-08 Thread Craig Topper via Phabricator via cfe-commits
craig.topper added a comment. There's still something weird in the backend, but things seem to generally work if you pass -fno-integrated-as to clang which the linux kernel does. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56571/new/ https://reviews.llvm.org/D56571

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-06-08 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment. In D56571#1523895 , @nickdesaulniers wrote: > Was this committed accidentally? > > Today in master I see: > > - r362106: Revert "clang support gnu asm goto." Erich Keane > > - r362059: Mark CodeGen/asm-goto.c as x86 specific

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-06-08 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment. There is a bug.. I took GCC’s example for asm goto and trunk emits: https://godbolt.org/z/9EcTR9 Wrong jb instruction target.. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56571/new/ https://reviews.llvm.org/D56571

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-06-04 Thread Jennifer Yu via Phabricator via cfe-commits
jyu2 closed this revision. jyu2 added a comment. jyu2 committed rGb8fee677bf8e : Re-check in clang support gun asm goto after fixing tests. (authored by jyu2). CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56571/new/

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-06-01 Thread Jennifer Yu via Phabricator via cfe-commits
jyu2 added a comment. jyu2 committed rG954ec09aed4f : clang support gnu asm goto In D56571#1523895 , @nickdesaulniers wrote: > Was this committed accidentally? > > Today in master I

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-06-01 Thread Kees Cook via Phabricator via cfe-commits
kees added a comment. Nick points out that "REQUIRES: x86-registered-target" is likely not needed. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56571/new/ https://reviews.llvm.org/D56571 ___ cfe-commits mailing list

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-06-01 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers added a comment. Was this committed accidentally? Today in master I see: - r362106: Revert "clang support gnu asm goto." Erich Keane - r362059: Mark CodeGen/asm-goto.c as x86 specific after r362045 Fangrui Song - r362045: clang support gnu asm goto. Jennifer Yu and yet

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-06-01 Thread Kees Cook via Phabricator via cfe-commits
kees added inline comments. Comment at: test/Analysis/asm-goto.cpp:1 +// RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCFG %s 2>&1 | FileCheck %s + Based on Nathan's comments in another thread, it seems like this needs to be: ``` // REQUIRES:

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-05-24 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith accepted this revision. rsmith marked an inline comment as done. rsmith added a comment. Looks good with a few largely-mechanical changes. Comment at: include/clang/Basic/DiagnosticParseKinds.td:30 "GNU-style inline assembly is disabled">; -def

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-05-24 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers accepted this revision. nickdesaulniers added a comment. Ok, from the Linux kernel's perspective, I believe we have worked out all underlying issues in LLVM wrt callbr that would prevent us from using asm goto successfully. This patch now has my blessing; thanks for all the

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-05-05 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers added a comment. Also: https://reviews.llvm.org/D61560 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56571/new/ https://reviews.llvm.org/D56571 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-04-25 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers added a comment. In D56571#1477992 , @jyu2 wrote: > Please let me know if that is clang problem. It's an orthogonal issue with Clang's integrated assembler. @compudj confirmed that setting `-no-integrated-as` solves the issue (which is

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-04-24 Thread Jennifer Yu via Phabricator via cfe-commits
jyu2 added a comment. In D56571#1476266 , @nickdesaulniers wrote: > @compudj email me the preprocessed output of basic_percpu_ops_test.c and I'll > take a look. (Should be able to find my email via `git log`). @nickdesaulniers, thanks for looking into

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-04-23 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers added a comment. @compudj email me the preprocessed output of basic_percpu_ops_test.c and I'll take a look. (Should be able to find my email via `git log`). CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56571/new/ https://reviews.llvm.org/D56571

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-04-23 Thread Mathieu Desnoyers via Phabricator via cfe-commits
compudj added a comment. Hi, First, thanks for working on this. I am the author of the "rseq" system call in the Linux kernel, and the user-space code required to interact with that system call requires asm goto. I therefore look forward to getting asm goto support in clang. I tried this

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-04-23 Thread Gabor Marton via Phabricator via cfe-commits
martong resigned from this revision. martong added a comment. The changes in `ASTImporter.cpp` looks good to me! And I have no competence about the rest of the change, thus I resign as a reviewer. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56571/new/ https://reviews.llvm.org/D56571

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-04-22 Thread Craig Topper via Phabricator via cfe-commits
craig.topper added a comment. @void, @jyknight given that looks like a LLVM issue rather than a clang issue, can we file a bugzilla to track separately from this clang frontend patch? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56571/new/ https://reviews.llvm.org/D56571

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-04-19 Thread James Y Knight via Phabricator via cfe-commits
jyknight added a comment. There shouldn't be an empty string ("") in the asm output -- that should be a reference to the "l_yes" label, not the empty string. That seems very weird... Even odder: running clang -S on the above without -fno-integrated-as emits a ".quad .Ltmp00" (note the extra

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-04-15 Thread Bill Wendling via Phabricator via cfe-commits
void added a comment. In D56571#1467358 , @craig.topper wrote: > I think things don't work right unless you disable the integrated assembler. > I'm not sure why though. Using `-no-integrated-as` does allow it to compile, but it doesn't link (with

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-04-15 Thread Craig Topper via Phabricator via cfe-commits
craig.topper added a comment. In D56571#1467333 , @void wrote: > This code: > > ; ModuleID = 'arch_static_branch.bc' > source_filename = "arch/x86/entry/vsyscall/vsyscall_64.c" > target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128" > target

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-04-15 Thread Bill Wendling via Phabricator via cfe-commits
void added a comment. ; ModuleID = 'arch_static_branch.bc' source_filename = "arch/x86/entry/vsyscall/vsyscall_64.c" target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128" target triple = "x86_64-grtev4-linux-gnu" %struct.atomic_t = type { i32 } %struct.jump_entry = type {

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-04-03 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers added a comment. Note that with: 1. https://reviews.llvm.org/D60208 ("[X86] Extend boolean arguments to inline-asm according to getBooleanType") 2. https://reviews.llvm.org/D58260 ("[INLINER] allow inlining of blockaddresses if sole uses are callbrs") 3.

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-03-06 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers added a comment. > which is making things hard for the folks testing this with the Linux kernel > and needing to apply this patch themselves. If we can get inlining of callbr working first before landing this, then all of our CI won't go immediately red (as it would from

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-03-06 Thread Eli Friedman via Phabricator via cfe-commits
efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. I'm fine merging in this state... but please wait for rsmith to respond before you merge. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56571/new/

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-03-06 Thread Craig Topper via Phabricator via cfe-commits
craig.topper added a comment. @rsmith, @efriedma, @chandlerc, is this in good enough shape that we can commit and move to incremental fixes/improvement? Jennifer has had to rebase this a couple times which is making things hard for the folks testing this with the Linux kernel and needing to

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-03-04 Thread Jennifer Yu via Phabricator via cfe-commits
jyu2 updated this revision to Diff 189205. jyu2 added a comment. Rebased CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56571/new/ https://reviews.llvm.org/D56571 Files: include/clang/AST/Stmt.h include/clang/Basic/DiagnosticParseKinds.td

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-03-04 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers added a comment. needs another rebase CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56571/new/ https://reviews.llvm.org/D56571 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-03-04 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers added a comment. Should the langref be updated (specifically the section on blockaddress): This value only has defined behavior when used as an operand to the ‘indirectbr’ instruction, or for comparisons against null. https://reviews.llvm.org/D53765 touched the langref, but I

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-02-22 Thread Jennifer Yu via Phabricator via cfe-commits
jyu2 updated this revision to Diff 188021. jyu2 added a comment. Rebase CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56571/new/ https://reviews.llvm.org/D56571 Files: include/clang/AST/Stmt.h include/clang/Basic/DiagnosticParseKinds.td include/clang/Basic/DiagnosticSemaKinds.td

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-02-15 Thread Jennifer Yu via Phabricator via cfe-commits
jyu2 updated this revision to Diff 187124. jyu2 marked 5 inline comments as done. jyu2 added a comment. Herald added a subscriber: rnkovacs. Review comments addressed. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56571/new/ https://reviews.llvm.org/D56571 Files:

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-02-15 Thread Jennifer Yu via Phabricator via cfe-commits
jyu2 added inline comments. Comment at: lib/AST/StmtProfile.cpp:324 VisitStringLiteral(S->getClobberStringLiteral(I)); + ID.AddInteger(S->getNumLabels()); } rsmith wrote: > Don't we also need to profile the labels themselves? How this can be lost? :-(

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-02-15 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added inline comments. Herald added a reviewer: martong. Comment at: lib/AST/StmtProfile.cpp:324 VisitStringLiteral(S->getClobberStringLiteral(I)); + ID.AddInteger(S->getNumLabels()); } Don't we also need to profile the labels themselves?

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-02-15 Thread Jennifer Yu via Phabricator via cfe-commits
jyu2 added inline comments. Comment at: lib/Sema/JumpDiagnostics.cpp:675 + // asm-goto. + //if (!IsAsmGoto && IndirectJumpTargets.empty()) { + if (JumpTargets.empty()) { efriedma wrote: > Commented-out code? > > We probably don't need a diagnostic for an asm

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-02-15 Thread Jennifer Yu via Phabricator via cfe-commits
jyu2 updated this revision to Diff 187036. jyu2 marked an inline comment as done. jyu2 added a comment. Review comment addressed CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56571/new/ https://reviews.llvm.org/D56571 Files: include/clang/AST/Stmt.h

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-02-14 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment. I think this is in good shape. Comment at: lib/Sema/JumpDiagnostics.cpp:675 + // asm-goto. + //if (!IsAsmGoto && IndirectJumpTargets.empty()) { + if (JumpTargets.empty()) { Commented-out code? We probably don't need a diagnostic

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-02-12 Thread Jennifer Yu via Phabricator via cfe-commits
jyu2 updated this revision to Diff 186577. jyu2 added a comment. Cleanup CFG code, remove obj-lifetime code when build CFE for asm-goto. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56571/new/ https://reviews.llvm.org/D56571 Files: include/clang/AST/Stmt.h

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-02-11 Thread Jennifer Yu via Phabricator via cfe-commits
jyu2 updated this revision to Diff 186356. jyu2 added a comment. I find some ambiguous error for asm-goto out of scope. Instead call VerifyIndirectOrAsmJumps one time, call that function twice one for indirect-goto one for asm-goto. Let me know if you see any problems. Thanks for all the

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-02-10 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers marked an inline comment as done. nickdesaulniers added inline comments. Comment at: test/CodeGen/asm.c:276 + return 1; +} nickdesaulniers wrote: > I don't understand why, but this particular test case I cannot compile > standalone without

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-02-10 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers added inline comments. Comment at: test/CodeGen/asm.c:276 + return 1; +} I don't understand why, but this particular test case I cannot compile standalone without `-emit-llvm`. ``` int t32(int cond) { asm goto("testl %0, %0; jne %l1;" ::

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-02-07 Thread Jennifer Yu via Phabricator via cfe-commits
jyu2 added inline comments. Comment at: lib/Analysis/CFG.cpp:3137 + + Block = createBlock(false); + Block->setTerminator(G); efriedma wrote: > efriedma wrote: > > Passing add_successor=false seems suspect; this could probably use more > > test coverage. > Did

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-02-07 Thread Jennifer Yu via Phabricator via cfe-commits
jyu2 updated this revision to Diff 185911. jyu2 marked an inline comment as done. jyu2 added a comment. Remove check for %lN Fix missing successor of asm goto in CFG build. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56571/new/ https://reviews.llvm.org/D56571 Files:

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-02-07 Thread Craig Topper via Phabricator via cfe-commits
craig.topper added inline comments. Comment at: lib/AST/Stmt.cpp:628 + DiagOffs = CurPtr-StrStart-1; + return diag::err_asm_invalid_operand_for_goto_labels; +} efriedma wrote: > jyu2 wrote: > > rsmith wrote: > > > jyu2 wrote: > > > >

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-02-07 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added inline comments. Comment at: lib/AST/Stmt.cpp:628 + DiagOffs = CurPtr-StrStart-1; + return diag::err_asm_invalid_operand_for_goto_labels; +} jyu2 wrote: > rsmith wrote: > > jyu2 wrote: > > > rsmith wrote: > > > > I'm

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-02-07 Thread Jennifer Yu via Phabricator via cfe-commits
jyu2 marked an inline comment as done. jyu2 added inline comments. Comment at: lib/AST/Stmt.cpp:628 + DiagOffs = CurPtr-StrStart-1; + return diag::err_asm_invalid_operand_for_goto_labels; +} rsmith wrote: > jyu2 wrote: > > rsmith wrote:

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-02-06 Thread Kees Cook via Phabricator via cfe-commits
kees added a comment. In D56571#1386973 , @kees wrote: > Not sure if this is the fault of the LLVM half or the Clang half, but I'm > seeing mis-compilations in the current patches (llvm > ca1e713fdd4fab5273b36ba6f292a844fca4cb2d with D53765 >

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-02-06 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added inline comments. Comment at: lib/AST/Stmt.cpp:628 + DiagOffs = CurPtr-StrStart-1; + return diag::err_asm_invalid_operand_for_goto_labels; +} jyu2 wrote: > rsmith wrote: > > I'm curious why we're checking this here, when all

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-02-06 Thread Kees Cook via Phabricator via cfe-commits
kees added a comment. I reduced the C code to this: volatile int mystery = 0; static int noinline demo(int klen) { int err; int len; err = mystery * klen; if (err) return err; if (len > klen) len =

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-02-06 Thread Kees Cook via Phabricator via cfe-commits
kees added a comment. Not sure if this is the fault of the LLVM half or the Clang half, but I'm seeing mis-compilations in the current patches (llvm ca1e713fdd4fab5273b36ba6f292a844fca4cb2d with D53765 .185490 and clang 01879634f01bdbfac4636ebe03b68e85b20cd664

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-02-05 Thread Jennifer Yu via Phabricator via cfe-commits
jyu2 updated this revision to Diff 185489. jyu2 added a comment. small fix. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56571/new/ https://reviews.llvm.org/D56571 Files: include/clang/AST/Stmt.h include/clang/Basic/DiagnosticASTKinds.td

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-02-05 Thread Jennifer Yu via Phabricator via cfe-commits
jyu2 added inline comments. Comment at: lib/AST/Stmt.cpp:457-458 + this->NumLabels = NumLabels; + assert((NumLabels != 0 && NumOutputs == 0) || + (NumOutputs != 0 && NumLabels == 0)); rsmith wrote: > This can be simplified a little: > ``` >

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-02-05 Thread Jennifer Yu via Phabricator via cfe-commits
jyu2 updated this revision to Diff 185477. jyu2 marked 4 inline comments as done. jyu2 added a comment. More change from review. Add scope checking for asm-goto. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56571/new/ https://reviews.llvm.org/D56571 Files: include/clang/AST/Stmt.h

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-02-05 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added inline comments. Comment at: lib/AST/Stmt.cpp:457-458 + this->NumLabels = NumLabels; + assert((NumLabels != 0 && NumOutputs == 0) || + (NumOutputs != 0 && NumLabels == 0)); This can be simplified a little: ``` assert(!(NumOutputs &&

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-02-05 Thread Jennifer Yu via Phabricator via cfe-commits
jyu2 marked 2 inline comments as done. jyu2 added inline comments. Comment at: lib/AST/Stmt.cpp:457-460 this->NumOutputs = NumOutputs; this->NumInputs = NumInputs; this->NumClobbers = NumClobbers; + this->NumLabels = NumLabels; rsmith wrote: > jyu2

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-02-05 Thread Jennifer Yu via Phabricator via cfe-commits
jyu2 updated this revision to Diff 185360. jyu2 added a comment. 1>emit better error for use of output constraints. 2> Add assert for NumLabels and NumOutputs are both larger than 0. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56571/new/ https://reviews.llvm.org/D56571 Files:

Re: [PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-02-05 Thread Eli Friedman via cfe-commits
; natechancel...@gmail.com; tstel...@redhat.com; Eli Friedman; srhi...@google.com; era...@google.com; cfe-commits@lists.llvm.org Subject: [EXT] Re: [PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang Note that kernel developers are requesting output support with ASM goto. Doesn't

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-02-05 Thread Craig Topper via Phabricator via cfe-commits
craig.topper added a subscriber: jyknight. craig.topper added inline comments. Comment at: lib/CodeGen/CGStmt.cpp:2236 +llvm::CallBrInst *Result = +Builder.CreateCallBr(FTy, IA, Fallthrough, Transfer, Args); +UpdateAsmCallInst(cast(*Result), HasSideEffect,

Re: [PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-02-05 Thread Nick Desaulniers via cfe-commits
Note that kernel developers are requesting output support with ASM goto. Doesn't need to be in V1 of ASM go-to support, but we should expect it to be implemented in the future. We should help users with diagnostics that this is not supported, and add asserts that will help us implement such a

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-02-04 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added inline comments. Comment at: lib/AST/Stmt.cpp:457-460 this->NumOutputs = NumOutputs; this->NumInputs = NumInputs; this->NumClobbers = NumClobbers; + this->NumLabels = NumLabels; jyu2 wrote: > rsmith wrote: > > Please assert that you don't

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-02-04 Thread Jennifer Yu via Phabricator via cfe-commits
jyu2 added inline comments. Comment at: lib/AST/Stmt.cpp:457-460 this->NumOutputs = NumOutputs; this->NumInputs = NumInputs; this->NumClobbers = NumClobbers; + this->NumLabels = NumLabels; rsmith wrote: > Please assert that you don't have both outputs

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-02-04 Thread Jennifer Yu via Phabricator via cfe-commits
jyu2 updated this revision to Diff 185206. jyu2 marked 8 inline comments as done. jyu2 added a comment. More review comments addressed. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56571/new/ https://reviews.llvm.org/D56571 Files: include/clang/AST/Stmt.h

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-02-04 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added inline comments. Comment at: lib/Sema/JumpDiagnostics.cpp:347 + LabelAndGotoScopes[S] = ParentScope; + Jumps.push_back(S); +} jyu2 wrote: > efriedma wrote: > > This doesn't look right; I think we need to add it to IndirectJumps > >

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-02-04 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added inline comments. Comment at: lib/AST/Stmt.cpp:457-460 this->NumOutputs = NumOutputs; this->NumInputs = NumInputs; this->NumClobbers = NumClobbers; + this->NumLabels = NumLabels; Please assert that you don't have both outputs and labels

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-02-04 Thread Jennifer Yu via Phabricator via cfe-commits
jyu2 marked 2 inline comments as done. jyu2 added inline comments. Comment at: lib/Sema/JumpDiagnostics.cpp:347 + LabelAndGotoScopes[S] = ParentScope; + Jumps.push_back(S); +} efriedma wrote: > This doesn't look right; I think we need to add it to

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-02-04 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added inline comments. Comment at: lib/Sema/JumpDiagnostics.cpp:347 + LabelAndGotoScopes[S] = ParentScope; + Jumps.push_back(S); +} This doesn't look right; I think we need to add it to IndirectJumps instead. This probably impacts a

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-02-04 Thread Jennifer Yu via Phabricator via cfe-commits
jyu2 marked 12 inline comments as done. jyu2 added inline comments. Comment at: include/clang/AST/Stmt.h:1008 + (*iterator_adaptor_base::I)->getStmtClass() <= lastExprConstant); + return *reinterpret_cast(iterator_adaptor_base::I); } rsmith

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-02-04 Thread Jennifer Yu via Phabricator via cfe-commits
jyu2 updated this revision to Diff 185094. jyu2 added a comment. 1> Add code for scope checking 2> Using CastInterator 3> CFG change to remove duplicate Block list CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56571/new/ https://reviews.llvm.org/D56571 Files:

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-02-04 Thread Jennifer Yu via Phabricator via cfe-commits
jyu2 commandeered this revision. jyu2 edited reviewers, added: craig.topper; removed: jyu2. jyu2 added inline comments. Comment at: lib/Analysis/CFG.cpp:2126 + if (cast(S)->isGCCAsmGoto()) +return VisitGCCAsmStmt(cast(S)); + return VisitStmt(S, asc);

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-02-01 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment. Forgot about protected scopes... This patch is missing code and testcases for JumpScopeChecker. (The behavior should be roughly equivalent to what we do for indirect goto.) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56571/new/

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-02-01 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added inline comments. Comment at: lib/Analysis/CFG.cpp:3137 + + Block = createBlock(false); + Block->setTerminator(G); Passing add_successor=false seems suspect; this could probably use more test coverage. Comment at:

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-02-01 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added inline comments. Comment at: include/clang/AST/Stmt.h:1008 + (*iterator_adaptor_base::I)->getStmtClass() <= lastExprConstant); + return *reinterpret_cast(iterator_adaptor_base::I); } Ugh. This cast violates strict aliasing, and

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-01-30 Thread Craig Topper via Phabricator via cfe-commits
craig.topper commandeered this revision. craig.topper edited reviewers, added: jyu2; removed: craig.topper. craig.topper added a comment. Commandeering so I can update to match an IRBuilder interface change in the llvm patch CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56571/new/

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-01-30 Thread Craig Topper via Phabricator via cfe-commits
craig.topper updated this revision to Diff 184447. craig.topper added a comment. Pass FunctionType into the IRBuilder::CreateCallBr to avoid needing to make an opaque pointer update later CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56571/new/ https://reviews.llvm.org/D56571 Files:

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-01-30 Thread Jennifer Yu via Phabricator via cfe-commits
jyu2 updated this revision to Diff 184349. jyu2 marked an inline comment as done. jyu2 added a comment. Change test for BE IR change. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56571/new/ https://reviews.llvm.org/D56571 Files: include/clang/AST/Expr.h

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-01-17 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers added a comment. Also of note: glib, a super popular library used across many Linux distro's will benefit from the implementation of asm goto (I will send them a patch to fix their version detection once we land in Clang):

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-01-15 Thread Jennifer Yu via Phabricator via cfe-commits
jyu2 updated this revision to Diff 181973. jyu2 marked an inline comment as done. jyu2 added a comment. Change error message. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56571/new/ https://reviews.llvm.org/D56571 Files: include/clang/AST/Expr.h include/clang/AST/ExprCXX.h

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-01-15 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers added a comment. With this patch, DD53765, and one out of tree kernel patch to work around a separate issue , I can confirm that I can compile, link, and boot an x86_64 Linux kernel with

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-01-15 Thread Jennifer Yu via Phabricator via cfe-commits
jyu2 marked 2 inline comments as done. jyu2 added inline comments. Comment at: lib/Sema/SemaStmtAsm.cpp:470 +if (NS->isGCCAsmGoto() && +Exprs[ConstraintIdx]->getStmtClass() == Stmt::AddrLabelExprClass) + break; efriedma wrote: > jyu2 wrote: > >

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-01-15 Thread Jennifer Yu via Phabricator via cfe-commits
jyu2 updated this revision to Diff 181862. jyu2 added a comment. I add additional test. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56571/new/ https://reviews.llvm.org/D56571 Files: include/clang/AST/Expr.h include/clang/AST/ExprCXX.h include/clang/AST/ExprObjC.h

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-01-15 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added inline comments. Comment at: include/clang/Basic/DiagnosticASTKinds.td:214 + def err_asm_invalid_operand_number_for_goto_labels : Error < +"invalid operand number which isn't point to a goto label in asm string">; } Grammar. Also, "invalid

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-01-15 Thread Chandler Carruth via Phabricator via cfe-commits
chandlerc added a subscriber: hans. chandlerc added a comment. Adding Hans so he can be aware of the progress. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56571/new/ https://reviews.llvm.org/D56571 ___ cfe-commits mailing list

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-01-14 Thread Jennifer Yu via Phabricator via cfe-commits
jyu2 marked an inline comment as done. jyu2 added inline comments. Comment at: lib/Sema/SemaStmtAsm.cpp:470 +if (NS->isGCCAsmGoto() && +Exprs[ConstraintIdx]->getStmtClass() == Stmt::AddrLabelExprClass) + break; efriedma wrote: > jyu2 wrote: > >

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-01-14 Thread Jennifer Yu via Phabricator via cfe-commits
jyu2 updated this revision to Diff 181702. jyu2 added a comment. Add code to diagnostic error for use of the "l" modifier that does not point to a label in the label list. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56571/new/ https://reviews.llvm.org/D56571 Files:

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-01-14 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added inline comments. Comment at: lib/Sema/SemaStmtAsm.cpp:470 +if (NS->isGCCAsmGoto() && +Exprs[ConstraintIdx]->getStmtClass() == Stmt::AddrLabelExprClass) + break; jyu2 wrote: > efriedma wrote: > > jyu2 wrote: > > > jyu2 wrote: > > >

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-01-14 Thread Jennifer Yu via Phabricator via cfe-commits
jyu2 marked an inline comment as done. jyu2 added inline comments. Comment at: lib/Sema/SemaStmtAsm.cpp:470 +if (NS->isGCCAsmGoto() && +Exprs[ConstraintIdx]->getStmtClass() == Stmt::AddrLabelExprClass) + break; efriedma wrote: > jyu2 wrote: > >