Hi Richard,
On Jan 7, 2013, at 3:02 PM, Richard Smith <rich...@metafoo.co.uk> wrote:
> Hi Chad,
>
> On Mon, Jan 7, 2013 at 12:52 PM, Chad Rosier <mcros...@apple.com> 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?
The documentation I've been looking at can be located here:
http://faculty.petra.ac.id/indi/files/
Then click the "C++ and Assembly Language Reference.pdf" link.
Unfortunately, I don't think there's a great deal of documentation readily
available. For the most part I was planning on implementing what llvm-gcc
supported. I've also contacted Microsoft to see if they have any
documentation; I didn't see any online.
I'll get back to you once I have more information. Thanks again for the
feedback, Richard.
Chad
> 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.
Will do.
>
> --- 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.
>
I'll remove it.
>
> --- 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
cfe-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
