[cfe-commits] [Patch] CodeWarrior/MS-style inline assembly asm function

[Chad Rosier]

Richard,
Upon further consideration, I've decided to no purse this feature after all.  Implementing this as a naked function with an asm block in the function body should suffice is most cases.

__declspec(naked) int foo(int x) {
  __asm {
    // do some assembly magic with x
  }
}

Regardless, thanks for the review/feedback, Richard.

 Chad

On Jan 7, 2013, at 4:32 PM, Chad Rosier <mcrosier at apple.com> wrote:

> Hi Richard,
> 
> On Jan 7, 2013, at 3:02 PM, Richard Smith <richard at metafoo.co.uk> wrote:
> 
>> Hi Chad,
>> 
>> On Mon, Jan 7, 2013 at 12:52 PM, Chad Rosier <mcrosier at 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 at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits