[PATCH] ARM: Implement __attribute__((interrupt(...)))

Tim Northover t.p.northover at gmail.com
Tue Sep 24 05:20:28 PDT 2013


Hi Aaron,

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,
>> Kind.getValue()));
>> +  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)?

Cheers.

Tim.



More information about the cfe-commits mailing list