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; ---------------- jyu2 wrote: > efriedma wrote: > > jyu2 wrote: > > > efriedma wrote: > > > > jyu2 wrote: > > > > > efriedma wrote: > > > > > > This looks suspicious; an AddrLabelExpr could be an input or > > > > > > output, e.g. `"r"(&&foo)`. > > > > > Syntax for asm goto: > > > > > Syntax: > > > > > asm [volatile] goto ( AssemblerTemplate > > > > > : > > > > > : InputOperands > > > > > : Clobbers > > > > > : GotoLabels) > > > > > > > > > > Only input is allowed. Output is not allowed > > > > > > > > > That doesn't really address my point here... ignore the "or output" > > > > part of the comment. > > > Sorry did not realize that. Thank you so much for catching that. Need > > > to add other condition "ConstraintIdx > NS->getNumInputs() - 1", change > > > to : > > > > > > if (NS->isGCCAsmGoto() && ConstraintIdx > NS->getNumInputs() - 1 && > > > Exprs[ConstraintIdx]->getStmtClass() == Stmt::AddrLabelExprClass) > > > break; > > > > > > Is this ok with you? Thanks > > That's the right idea. But I still see a few issues at that point: > > > > 1. The AddrLabelExprClass check is redundant. > > 2. "NS->getNumInputs() - 1" can overflow; probably should use > > "ConstraintIdx >= NS->getNumInputs()". > > 3. "break" exits the loop completely (so it skips validating all > > constraints written after the label). > > 4. The code needs to verify that the user correctly specified the "l" > > constraint modifier. > Sorry not done yet. For you comment 4: The code needs to verify that the user correctly specified the "l" constraint modifier. We already emit error like following? Do you mean, we need more checking here? Thanks. n.c:4:35: error: unknown symbolic operand name in inline assembly string asm goto ("frob %%r5, %1; jc %l[error]; mov (%2), %%r5" ~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~ n.c:8:15: error: use of undeclared label 'error1' : error1); Test is: int frob(int x) { int y; asm goto ("frob %%r5, %1; jc %l[error]; mov (%2), %%r5" : /* No outputs. */ : "r"(x), "r"(&y) : "memory" : error1); return y; error: return -1; } CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56571/new/ https://reviews.llvm.org/D56571 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits