jyu2 marked 3 inline comments as done. jyu2 added inline comments.
================ Comment at: lib/Analysis/CFG.cpp:1474 + appendScopeBegin(JT.block, VD, G); + addSuccessor(B, JT.block); + } ---------------- efriedma wrote: > Please don't copy-paste code. :-( changed ================ Comment at: lib/Sema/SemaStmtAsm.cpp:470 + if (NS->isGCCAsmGoto() && + Exprs[ConstraintIdx]->getStmtClass() == Stmt::AddrLabelExprClass) + break; ---------------- 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. 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
