Hi Chad,
On Mon, Jan 7, 2013 at 12:52 PM, Chad Rosier <[email protected]> wrote:
> All,
> The attached patch add support for CodeWarrior style asm functions.
>
> static asm void t1() {
> mov eax, 0
> }
>
> This is a WIP. The current implementation only works when there is an
> additional function specifier (e.g., static, const) before the asm keyword.
> The parser needs to be enhanced to differentiate between module-level
> assembly and asm functions.
>
> module asm "inline asm here"
>
> vs
>
> __asm void t2() {
> mov eax, 0
> }
Can you provide a link to some documentation for this feature? I have
lots of questions about how it is supposed to work... For instance,
can you declare variables in such a function? Can you apply the 'asm'
keyword to a declaration that is not the definition? (And if so, is
the definition also implicitly 'asm', even if it omits the keyword?)
Can an 'asm' function have a function-try-block? If it's a
constructor, can it have member initializers? Can it be defaulted or
deleted?
There's lots of this sort of thing in your patch:
- bool isStatic,
- StorageClass SCAsWritten,
- bool isInline,
- bool isConstexpr,
+ bool isStatic, StorageClass SCAsWritten,
+ bool isAsm, bool isInline, bool isConstexpr,
Please convert the is* flags on FunctionDecl and friends into an enum
first, as a separate commit. Sorry, this really isn't your problem,
but this is the proverbial straw that broke the camel's back... This
would also remove most of the noise from the patch.
--- include/clang/AST/Decl.h (revision 171536)
+++ include/clang/AST/Decl.h (working copy)
@@ -1470,6 +1470,7 @@
// NOTE: VC++ treats enums as signed, avoid using the StorageClass enum
unsigned SClass : 2;
unsigned SClassAsWritten : 2;
+ bool IsAsmSpecified : 1;
You'll need to add serialization code for this flag.
+ /// Set whether the "asm" keyword was specified for this function.
+ void setAsmSpecified(bool I) { IsAsmSpecified = I; }
This function is unused, and seems unnecessary.
--- lib/CodeGen/CodeGenFunction.cpp (revision 171536)
+++ lib/CodeGen/CodeGenFunction.cpp (working copy)
@@ -347,9 +347,21 @@
CurFnInfo = &FnInfo;
assert(CurFn->isDeclaration() && "Function already has body?");
+ // Mark the function as naked and noinline if this is an asm function.
+ bool IsAsmFunc = false;
+ if (const FunctionDecl *FD = dyn_cast_or_null<FunctionDecl>(D))
+ for (FunctionDecl::redecl_iterator RI = FD->redecls_begin(),
+ RE = FD->redecls_end(); RI != RE; ++RI)
+ if (RI->isAsmSpecified()) {
+ IsAsmFunc = true;
+ Fn->addFnAttr(llvm::Attribute::Naked);
+ Fn->addFnAttr(llvm::Attribute::NoInline);
+ break;
+ }
Should this really be looking for any declaration which is 'asm', or
just looking for the definition? Why does 'asm' imply 'noinline'?
--- lib/Parse/ParseDecl.cpp (revision 171536)
+++ lib/Parse/ParseDecl.cpp (working copy)
@@ -2538,6 +2540,11 @@
break;
}
+ // Microsoft asm support.
+ case tok::kw_asm:
+ isInvalid = DS.setFunctionSpecAsm(Loc);
+ break;
Emit a diagnostic here if the extension is not enabled.
--- lib/Parse/ParseStmt.cpp (revision 171536)
+++ lib/Parse/ParseStmt.cpp (working copy)
- StmtResult FnBody(ParseCompoundStatementBody());
+ StmtResult FnBody(ParseCompoundStatementBody(/*isStmtExpr*/false,
+ ParsingAsmFunction));
Instead of wiring a flag through ParseCompoundStatementBody and
friends, please implement separate parsing code here. Reusing
ParseCompoundStatementBody does not seem worthwhile, since the only
shared syntax is apparently the braces.
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
