[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in LLVM
Jennifer Yu via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Fri Jan 11 18:07:43 PST 2019
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
More information about the cfe-commits
mailing list