[PATCH] D48412: [RISCV] Add support for interrupt attribute

[Ana Pazos via Phabricator via cfe-commits]

apazos added inline comments.


================
Comment at: test/Sema/riscv-interrupt-attr.c:23
+                                                      // expected-note {{repeated RISC-V 'interrupt' attribute is here}}
+__attribute__((interrupt("user"))) void foo8() {}
+__attribute__((interrupt("supervisor"))) void foo9() {}
----------------
aaron.ballman wrote:
> apazos wrote:
> > aaron.ballman wrote:
> > > apazos wrote:
> > > > aaron.ballman wrote:
> > > > > aaron.ballman wrote:
> > > > > > Do you intend for functions without a prototype to be accepted? foo8() can be passed an arbitrary number of arguments, which is a bit different than what I thought you wanted the semantic check to be.
> > > > > This question remains outstanding.
> > > > The checks are validating both function definitions and function prototypes like these:
> > > > _attribute__((interrupt)) void foo1() {} 
> > > > __attribute__((interrupt)) void foo(void);
> > > > Not sure what the confusion is.
> > > Ah, now I see where the confusion is.
> > > 
> > > In C, an empty parameter list declares a function without a prototype; functions without prototypes can accept any number of arguments. To declare a function that accepts no arguments, you must have a prototype for the function and the parameter list is void. In C++, all functions are prototyped and an empty parameter list is equivalent to a parameter list of void. The word "prototype" doesn't mean "forward declaration". e.g.,
> > > ```
> > > // C code
> > > void foo1(); // Declaration; no prototype; accepts any number of arguments.
> > > void foo2() {} // Definition; no prototype; accepts any number of arguments.
> > > void foo3(void); // Declaration; prototype; accepts no arguments.
> > > void foo4(void) {} // Definition; prototype; accepts no arguments.
> > > 
> > > foo2(1, 2, 3); // ok
> > > foo4(1, 2, 3); // error
> > > ```
> > > Because a function without a prototype can accept any number of arguments, I think you want to diagnose such a function signature.
> > Thanks for clarifying. 
> > 
> > I checked GCC behavior and it is less strict. For the example below, it silently accepts the interrupt attribute.
> > 
> > extern int foo2();
> > __attribute__((interrupt)) void foo();
> > void foo() {
> >   foo2();
> > }
> > 
> > while in LLVM we would be rejecting with the message: 
> > RISC-V 'interrupt' attribute only applies to functions that have no parameters. 
> > 
> > I find the reuse of the message confusing. 
> > 
> > If we want stricter rule then we probably also need a specific message for the  missing prototype.
> > 
> > I checked GCC behavior and it is less strict. For the example below, it silently accepts the interrupt attribute.
> 
> Does it drop the attribute?
> 
> > If we want stricter rule then we probably also need a specific message for the missing prototype.
> 
> If GCC silently drops the attribute in this case then we definitely want a more strict rule. We already have a good diagnostic for this: `warn_attribute_wrong_decl_type` with the expected type diagnostic index being `ExpectedFunctionWithProtoType`.
It does not drop, it compiles without warnings and it produces the code that is expected when interrupt attribute is set.


https://reviews.llvm.org/D48412