[PATCH] D28451: [AVR] Add support for the 'interrupt' and 'naked' attributes

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: include/clang/Basic/Attr.td:488
+
+def AVRSignal : InheritableAttr, TargetSpecificAttr<TargetAVR> {
+  let Spellings = [GNU<"signal">];
----------------
dylanmckay wrote:
> aaron.ballman wrote:
> > Does this attribute appertain to any specific subjects, or can you apply it to any declaration?
> It can be applied to function definitions only.
Then it should have a Subjects line like `AVRInterrupt` does.


================
Comment at: lib/CodeGen/TargetInfo.cpp:6898
+                           CodeGen::CodeGenModule &CGM) const override {
+    const FunctionDecl *FD = dyn_cast_or_null<FunctionDecl>(D);
+    if (!FD) return;
----------------
You can use `const auto *` here.


================
Comment at: lib/Sema/SemaDeclAttr.cpp:5129
 
+static void handleAVRInterruptAttr(Sema &S, Decl *D,
+                                   const AttributeList &Attr) {
----------------
You can remove this function.


================
Comment at: lib/Sema/SemaDeclAttr.cpp:5136
+
+static void handleAVRSignalAttr(Sema &S, Decl *D,
+                                const AttributeList &Attr) {
----------------
...and this one.


================
Comment at: lib/Sema/SemaDeclAttr.cpp:5158
+  case llvm::Triple::avr:
+    handleAVRInterruptAttr(S, D, Attr);
+    break;
----------------
Just call `handleSimpleAttribute()` instead.


================
Comment at: lib/Sema/SemaDeclAttr.cpp:5721
+  case AttributeList::AT_AVRSignal:
+    handleAVRSignalAttr(S, D, Attr);
+    break;
----------------
...same here.


================
Comment at: test/CodeGen/avr/attributes/naked.c:4
+// CHECK: define void @foo() #0
+__attribute__((naked)) void foo(void) { }
+
----------------
dylanmckay wrote:
> aaron.ballman wrote:
> > This test seems unrelated as you didn't modify anything about the naked attribute?
> I didn't modify the naked attribute, but I did want to include a test for AVR.
This should probably be a separate commit then (and doesn't really need code review, since it's just adding a test and is otherwise NFC); just make sure the commit message explains why the test is beneficial to add.


https://reviews.llvm.org/D28451