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 wrote:
> > rsmith wrote:
> > > Please assert that you don't have both outputs and labels here. (If you
> > > did, you would assign them the same slots within `Exprs`.)
> > >
> > > You also seem to be missing `Sema` enforcement of the rule that you
> > > cannot have both outputs and labels. (If you want to actually support
> > > that as an intentional extension to the GCC functionality, you should
> > > change the implementation of `GCCAsmStmt` to use different slots for
> > > outputs and labels, add some tests, and add a `-Wgcc-compat` warning for
> > > that case. Seems better to just add an error for it for now.)
> > This is enforcement during the parer.
> >
> > for example:
> > a.c:12:23: error: expected ':'
> > asm goto ("decl %0;" :"a": "m"(cond) : "memory" );
> >
> > Do you think this is enough for now?
> > Thanks.
> > Jennifer
> Thanks, I see it now. Please still add the assert here.
>
> I'd also like a custom diagnostic for that parse error; it'd seem easy and
> useful to add an "asm goto cannot have output constraints" error.
Okay added.
================
Comment at: lib/CodeGen/CGStmt.cpp:2236
+ llvm::CallBrInst *Result =
+ Builder.CreateCallBr(FTy, IA, Fallthrough, Transfer, Args);
+ UpdateAsmCallInst(cast<llvm::CallBase>(*Result), HasSideEffect, ReadOnly,
----------------
craig.topper wrote:
> I don't think we need to pass FTy here now. I made that change before
> @jyknight added FunctionCallee that should be able to find the type from the
> InlineAsm. The CallBr IRBuilder code should have a FunctionCallee Create
> function as of yesterday.
Removed.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D56571/new/
https://reviews.llvm.org/D56571
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
