[PATCH] ARM: Implement __attribute__((interrupt(...)))
t.p.northover at gmail.com
Tue Sep 24 05:20:28 PDT 2013
Thanks for your review, I'm working on an updated Clang patch. Could I
ask a couple of questions?
>> +def err_attribute_argument_invalid : Error<
>> + "'%0' attribute parameter '%1' is invalid">;
> This is not needed (more below).
> There is code auto-generated for you that does this, named something like
> ConvertStrToInterruptType; you should use that, and get rid of the custom
> logic for error handling.
As far as I can see, the auto-generated logic is very self-contained
and only takes a StringRef and converts it to an enum. Am I missing a
separate interface that'll do the whole Sema for me? Otherwise it
seems most of the error-handling is necessary.
>> + Fn->addFnAttr("interrupt", Kind);
>> + if (cast<ARMABIInfo>(getABIInfo()).getABIKind() == ARMABIInfo::APCS)
>> + return;
> This could be hoisted higher in the function.
Any higher and the LLVM function would not get the attribute.
>> + d->addAttr(::new (S.Context)
>> + ARMInterruptAttr(Attr.getLoc(), S.Context,
>> + d->addAttr(::new (S.Context) UsedAttr(Attr.getLoc(), S.Context));
> This breaks the as-written attributes.
Do you mean the fact that we end up with two "used" attributes if it's
also specified manually? Or am I doing something wrong with the
ARMInterruptAttr too (aside from not setting the spelling index)?
More information about the cfe-commits