From: Borja F. <bor...@gm...> - 2012-03-05 20:57:38
|
I'm currently implementing support for interrupt handling. But before going any further into implementing it in the backend (the frontend part is nearly done) I want to get things clear. Eric this one is probably for you because it's about avr-libc but anyways, here we go: There are 2 function attributes to add: 1) signal (ISR macro): normal interrupt handler. Every register used inside the handler gets saved in the stack, including special code to save SREG. So we have to mark all registers to be callee saved. 2) interrupt (ISR_NOBLOCK): same as above but adding a sei instruction at the very top of function entry. Now, the naked attribute is already implemented and i don't need to handle anything with it. It avoids any prologue/epilogue code to be inserted. BUT by default the ret/reti instruction gets emitted, and avr-libc says that it should be done by the programmer so I have to handle that, right? This doesn't matter if it's an ISR or a normal func. The ISR_ALIASOF macro works correctly, so no more work is needed there (using NOBLOCK or NAKED with ALIASOF doesn't make sense at all, right?) Now i want to talk about combining attributes: Truth table format: |signal | interrupt | naked| 1) 0 0 0 -> nothing 2) 0 0 1 -> no prologue/epilogue and no ret/reti (used in any arbitrary function)(so as i said before, don't emit ret/reti?) 3) 0 1 0 -> emit sei, and save all used regs inside the function plus SREG. 4) 0 1 1 -> naked wins, do case 1. 5) 1 0 0 -> normal ISR macro, do what i said when explaining the signal attrib. 6) 1 0 1 -> naked wins again. 7) 1 1 0 -> interrupt wins, do case 3. 8) 1 1 1 -> naked wins. So this is what I understand I have to do, please let me know if I'm wrong on anything I've mentioned. |
From: Weddington, E. <Eri...@at...> - 2012-03-05 21:39:11
|
Hi Borja, >From my view, "interrupt" and "signal" are mutually exclusive. They both define an ISR, where "interrupt" allows nested interrupts and "signal" disallows nested interrupts. I don't know if we can error out if a user defines both simultaneously. "naked" does something completely different than interrupt|signal. One can have a naked function without it being an ISR. Can one have a "naked" ISR? I suppose if one is writing the ISR in assembly... Another reason to have interrupt|signal is to put that function into the IVT. So, yes, the combination is valid, but naked should have priority, and the prologue/epilogue should not be generated for that ISR, and it is up to the user to write. Technically, the RET/RETI is usually a part of the epilogue. Naked functions do not have prologues/epilogues, hence, they do not have a RET/RETI, and the user has to provide those. Hope that answers any questions. Again, my only suggestion would be to error out on simultaneous "interrupt" and "signal" attributes. Eric Weddington > -----Original Message----- > From: Borja Ferrer [mailto:bor...@gm...] > Sent: Monday, March 05, 2012 1:58 PM > To: avr...@li... > Subject: [avr-llvm-devel] Interrupt handling > > I'm currently implementing support for interrupt handling. But before going > any further into implementing it in the backend (the frontend part is nearly > done) I want to get things clear. Eric this one is probably for you because > it's about avr-libc but anyways, here we go: > > There are 2 function attributes to add: > > 1) signal (ISR macro): normal interrupt handler. Every register used inside > the handler gets saved in the stack, including special code to save SREG. So > we have to mark all registers to be callee saved. > 2) interrupt (ISR_NOBLOCK): same as above but adding a sei instruction at the > very top of function entry. > > Now, the naked attribute is already implemented and i don't need to handle > anything with it. It avoids any prologue/epilogue code to be inserted. BUT by > default the ret/reti instruction gets emitted, and avr-libc says that it > should be done by the programmer so I have to handle that, right? > This doesn't matter if it's an ISR or a normal func. > > The ISR_ALIASOF macro works correctly, so no more work is needed there (using > NOBLOCK or NAKED with ALIASOF doesn't make sense at all, right?) > > Now i want to talk about combining attributes: > Truth table format: |signal | interrupt | naked| > > 1) 0 0 0 -> nothing > > 2) 0 0 1 -> no prologue/epilogue and no ret/reti (used in any arbitrary > function)(so as i said before, don't emit ret/reti?) > > 3) 0 1 0 -> emit sei, and save all used regs inside the function plus SREG. > > 4) 0 1 1 -> naked wins, do case 1. > > 5) 1 0 0 -> normal ISR macro, do what i said when explaining the signal > attrib. > > 6) 1 0 1 -> naked wins again. > > 7) 1 1 0 -> interrupt wins, do case 3. > > 8) 1 1 1 -> naked wins. > > So this is what I understand I have to do, please let me know if I'm wrong on > anything I've mentioned. > |
From: Borja F. <bor...@gm...> - 2012-03-06 00:43:25
|
Ok lets go by parts: I will fix the naked attribute to not emit the ret/reti instructions. I don't know in gcc, but in llvm the ret instruction is inserted very early, not during prologue/epilogue insertion, that's why it's getting emitted by default. So additional code has to be added to handle this. Yes, I've tried naked in non ISR functions aswell so I knew it was something valid to do. I wasn't sure about how it worked in combination with other attributes. About the interrupt and signal attributes, i think it should be possible to error out if they're both defined. But looking in avr-libc's code when you add the ISR_NOBLOCK macro inside the ISR macro you're both defining signal and interrupt at the same time, so it would always error out, or am I missing something? #define ISR(vector, ...) \ void vector (void) __attribute__ ((signal,__INTR_ATTRS)) __VA_ARGS__; \ void vector (void) # define ISR_NOBLOCK __attribute__((interrupt)) 2012/3/5 Weddington, Eric <Eri...@at...> > Hi Borja, > > From my view, "interrupt" and "signal" are mutually exclusive. They both > define an ISR, where "interrupt" allows nested interrupts and "signal" > disallows nested interrupts. > > I don't know if we can error out if a user defines both simultaneously. > > "naked" does something completely different than interrupt|signal. One > can have a naked function without it being an ISR. > > Can one have a "naked" ISR? I suppose if one is writing the ISR in > assembly... Another reason to have interrupt|signal is to put that > function into the IVT. So, yes, the combination is valid, but naked > should have priority, and the prologue/epilogue should not be generated > for that ISR, and it is up to the user to write. > > Technically, the RET/RETI is usually a part of the epilogue. Naked > functions do not have prologues/epilogues, hence, they do not have a > RET/RETI, and the user has to provide those. > > Hope that answers any questions. Again, my only suggestion would be to > error out on simultaneous "interrupt" and "signal" attributes. > > Eric Weddington > > > -----Original Message----- > > From: Borja Ferrer [mailto:bor...@gm...] > > Sent: Monday, March 05, 2012 1:58 PM > > To: avr...@li... > > Subject: [avr-llvm-devel] Interrupt handling > > > > I'm currently implementing support for interrupt handling. But before > going > > any further into implementing it in the backend (the frontend part is > nearly > > done) I want to get things clear. Eric this one is probably for you > because > > it's about avr-libc but anyways, here we go: > > > > There are 2 function attributes to add: > > > > 1) signal (ISR macro): normal interrupt handler. Every register used > inside > > the handler gets saved in the stack, including special code to save > SREG. So > > we have to mark all registers to be callee saved. > > 2) interrupt (ISR_NOBLOCK): same as above but adding a sei instruction > at the > > very top of function entry. > > > > Now, the naked attribute is already implemented and i don't need to > handle > > anything with it. It avoids any prologue/epilogue code to be inserted. > BUT by > > default the ret/reti instruction gets emitted, and avr-libc says that > it > > should be done by the programmer so I have to handle that, right? > > This doesn't matter if it's an ISR or a normal func. > > > > The ISR_ALIASOF macro works correctly, so no more work is needed there > (using > > NOBLOCK or NAKED with ALIASOF doesn't make sense at all, right?) > > > > Now i want to talk about combining attributes: > > Truth table format: |signal | interrupt | naked| > > > > 1) 0 0 0 -> nothing > > > > 2) 0 0 1 -> no prologue/epilogue and no ret/reti (used in any > arbitrary > > function)(so as i said before, don't emit ret/reti?) > > > > 3) 0 1 0 -> emit sei, and save all used regs inside the function plus > SREG. > > > > 4) 0 1 1 -> naked wins, do case 1. > > > > 5) 1 0 0 -> normal ISR macro, do what i said when explaining the > signal > > attrib. > > > > 6) 1 0 1 -> naked wins again. > > > > 7) 1 1 0 -> interrupt wins, do case 3. > > > > 8) 1 1 1 -> naked wins. > > > > So this is what I understand I have to do, please let me know if I'm > wrong on > > anything I've mentioned. > > > > |
From: Weddington, E. <Eri...@at...> - 2012-03-06 05:05:07
|
> -----Original Message----- > From: Borja Ferrer [mailto:bor...@gm...] > Sent: Monday, March 05, 2012 5:43 PM > To: Weddington, Eric > Cc: avr...@li... > Subject: Re: [avr-llvm-devel] Interrupt handling > > > #define ISR(vector, ...) \ > void vector (void) __attribute__ ((signal,__INTR_ATTRS)) __VA_ARGS__; \ > void vector (void) > # define ISR_NOBLOCK __attribute__((interrupt)) > > Hmm. Yeah, ok, you're correct about that. I forgot about all that stuff that was added to avr-libc. So, in this case, the "interrupt" attribute overrides "signal". I think you're good to go now. :-) Eric |
From: Borja F. <bor...@gm...> - 2012-03-06 11:22:50
|
Heh ok :) that's why I did the truth table above, because some attributes override others and we want to have the same behaviour as gcc. Now that everything is clear I'll implement this next. Btw, I've already fixed the case of not emitting ret/reti when naked is present. 2012/3/6 Weddington, Eric <Eri...@at...> > > > > -----Original Message----- > > From: Borja Ferrer [mailto:bor...@gm...] > > Sent: Monday, March 05, 2012 5:43 PM > > To: Weddington, Eric > > Cc: avr...@li... > > Subject: Re: [avr-llvm-devel] Interrupt handling > > > > > > #define ISR(vector, ...) \ > > void vector (void) __attribute__ ((signal,__INTR_ATTRS)) > __VA_ARGS__; \ > > void vector (void) > > # define ISR_NOBLOCK __attribute__((interrupt)) > > > > > > Hmm. Yeah, ok, you're correct about that. I forgot about all that stuff > that was added to avr-libc. > > So, in this case, the "interrupt" attribute overrides "signal". > > I think you're good to go now. :-) > > Eric > |
From: Borja F. <bor...@gm...> - 2012-03-06 18:56:42
|
I've added some checks to the frontend when setting the signal/interrupt attribute that are the following: 1) warn when the attrib is used in things that aren't function declarations. 2) warn when the function has arguments 3) warn when the function doesnt return a void If anybody can think of any more checks that are required let me know. Oh, and here are the warning messages printed for cases 2 and 3 respectively, again, if somebody has a better descriptive string or with better english I'll change it: 2) "'%0' attribute only applies to functions with no arguments" 3) "'%0' attribute only applies to functions with a void return type" where %0 is either interrupt or signal 2012/3/6 Borja Ferrer <bor...@gm...> > Heh ok :) that's why I did the truth table above, because some attributes > override others and we want to have the same behaviour as gcc. Now that > everything is clear I'll implement this next. > > Btw, I've already fixed the case of not emitting ret/reti when naked is > present. > > 2012/3/6 Weddington, Eric <Eri...@at...> > > >> >> > -----Original Message----- >> > From: Borja Ferrer [mailto:bor...@gm...] >> > Sent: Monday, March 05, 2012 5:43 PM >> > To: Weddington, Eric >> > Cc: avr...@li... >> > Subject: Re: [avr-llvm-devel] Interrupt handling >> > >> > >> > #define ISR(vector, ...) \ >> > void vector (void) __attribute__ ((signal,__INTR_ATTRS)) >> __VA_ARGS__; \ >> > void vector (void) >> > # define ISR_NOBLOCK __attribute__((interrupt)) >> > >> > >> >> Hmm. Yeah, ok, you're correct about that. I forgot about all that stuff >> that was added to avr-libc. >> >> So, in this case, the "interrupt" attribute overrides "signal". >> >> I think you're good to go now. :-) >> >> Eric >> > > |
From: Weddington, E. <Eri...@at...> - 2012-03-06 19:05:36
|
> -----Original Message----- > From: Borja Ferrer [mailto:bor...@gm...] > Sent: Tuesday, March 06, 2012 11:57 AM > To: Weddington, Eric > Cc: avr...@li... > Subject: Re: [avr-llvm-devel] Interrupt handling > > I've added some checks to the frontend when setting the signal/interrupt > attribute that are the following: > > 1) warn when the attrib is used in things that aren't function declarations. > 2) warn when the function has arguments > 3) warn when the function doesnt return a void > > If anybody can think of any more checks that are required let me know. > > Oh, and here are the warning messages printed for cases 2 and 3 respectively, > again, if somebody has a better descriptive string or with better english I'll > change it: > > 2) "'%0' attribute only applies to functions with no arguments" > 3) "'%0' attribute only applies to functions with a void return type" > where %0 is either interrupt or signal > Semantics question: Do you mean that it issues just a *warning*? Or does it error out on these? |
From: Borja F. <bor...@gm...> - 2012-03-06 19:22:08
|
It issues a warning, so, it doesn't apply the attribute and it doesn't abort compilation. I checked gcc and it warns aswell. However i can error out if it's better, i leave that up to you. 2012/3/6 Weddington, Eric <Eri...@at...> > > > > -----Original Message----- > > From: Borja Ferrer [mailto:bor...@gm...] > > Sent: Tuesday, March 06, 2012 11:57 AM > > To: Weddington, Eric > > Cc: avr...@li... > > Subject: Re: [avr-llvm-devel] Interrupt handling > > > > I've added some checks to the frontend when setting the > signal/interrupt > > attribute that are the following: > > > > 1) warn when the attrib is used in things that aren't function > declarations. > > 2) warn when the function has arguments > > 3) warn when the function doesnt return a void > > > > If anybody can think of any more checks that are required let me know. > > > > Oh, and here are the warning messages printed for cases 2 and 3 > respectively, > > again, if somebody has a better descriptive string or with better > english I'll > > change it: > > > > 2) "'%0' attribute only applies to functions with no arguments" > > 3) "'%0' attribute only applies to functions with a void return type" > > where %0 is either interrupt or signal > > > > Semantics question: Do you mean that it issues just a *warning*? Or does > it error out on these? > |
From: Weddington, E. <Eri...@at...> - 2012-03-06 19:32:16
|
Well, we don't have to slavishly follow GCC either. What do you think? If you were a programmer, would you want the compiler to warn you, then not apply the attribute and continue compilation? Should the compiler assume that you did not want the attribute to be applied in the case of a non-void function or one with arguments? The compiler really doesn't know what the fix should be. I would think, that in this case, it really is equivalent to a syntax error. These attributes are only allowed on a non-void, non-argument function. Otherwise, the user needs to the fix the code to expressly say what needs to be done. This is outside the realm of the C standard, since we're dealing with function attributes and ISRs. Again, what do you think? Eric Weddington > -----Original Message----- > From: Borja Ferrer [mailto:bor...@gm...] > Sent: Tuesday, March 06, 2012 12:22 PM > To: Weddington, Eric > Cc: avr...@li... > Subject: Re: [avr-llvm-devel] Interrupt handling > > It issues a warning, so, it doesn't apply the attribute and it doesn't abort > compilation. I checked gcc and it warns aswell. However i can error out if > it's better, i leave that up to you. > > > 2012/3/6 Weddington, Eric <Eri...@at...> > > > > > > -----Original Message----- > > From: Borja Ferrer [mailto:bor...@gm...] > > > Sent: Tuesday, March 06, 2012 11:57 AM > > To: Weddington, Eric > > Cc: avr...@li... > > Subject: Re: [avr-llvm-devel] Interrupt handling > > > > > I've added some checks to the frontend when setting the > signal/interrupt > > attribute that are the following: > > > > 1) warn when the attrib is used in things that aren't function > declarations. > > 2) warn when the function has arguments > > 3) warn when the function doesnt return a void > > > > If anybody can think of any more checks that are required let me know. > > > > Oh, and here are the warning messages printed for cases 2 and 3 > respectively, > > again, if somebody has a better descriptive string or with better > english I'll > > change it: > > > > 2) "'%0' attribute only applies to functions with no arguments" > > 3) "'%0' attribute only applies to functions with a void return type" > > where %0 is either interrupt or signal > > > > > Semantics question: Do you mean that it issues just a *warning*? Or does > it error out on these? > > |
From: Borja F. <bor...@gm...> - 2012-03-06 19:44:05
|
In my opinion the best thing should be to error out, it doesn't make sense to set this attributes in non void functions. Technically from a codegen perspective it doesn't matter because registers are all saved and restored unless the naked attrib is set so it wouldnt't make a difference except for some useless instructions. Doing this kind of things shows that the programmer doesn't understand what he's doing. I issued the warning to be sort of backwards compat with gcc, although in this case we would be supporting wrong code. In conclusion, error out. 2012/3/6 Weddington, Eric <Eri...@at...> > > Well, we don't have to slavishly follow GCC either. > > What do you think? > > If you were a programmer, would you want the compiler to warn you, then > not apply the attribute and continue compilation? > > Should the compiler assume that you did not want the attribute to be > applied in the case of a non-void function or one with arguments? > > The compiler really doesn't know what the fix should be. I would think, > that in this case, it really is equivalent to a syntax error. These > attributes are only allowed on a non-void, non-argument function. > Otherwise, the user needs to the fix the code to expressly say what > needs to be done. > > This is outside the realm of the C standard, since we're dealing with > function attributes and ISRs. Again, what do you think? > > Eric Weddington > > > -----Original Message----- > > From: Borja Ferrer [mailto:bor...@gm...] > > Sent: Tuesday, March 06, 2012 12:22 PM > > To: Weddington, Eric > > Cc: avr...@li... > > Subject: Re: [avr-llvm-devel] Interrupt handling > > > > It issues a warning, so, it doesn't apply the attribute and it doesn't > abort > > compilation. I checked gcc and it warns aswell. However i can error > out if > > it's better, i leave that up to you. > > > > > > 2012/3/6 Weddington, Eric <Eri...@at...> > > > > > > > > > > > -----Original Message----- > > > From: Borja Ferrer [mailto:bor...@gm...] > > > > > Sent: Tuesday, March 06, 2012 11:57 AM > > > To: Weddington, Eric > > > Cc: avr...@li... > > > Subject: Re: [avr-llvm-devel] Interrupt handling > > > > > > > > I've added some checks to the frontend when setting the > > signal/interrupt > > > attribute that are the following: > > > > > > 1) warn when the attrib is used in things that aren't function > > declarations. > > > 2) warn when the function has arguments > > > 3) warn when the function doesnt return a void > > > > > > If anybody can think of any more checks that are required let > me know. > > > > > > Oh, and here are the warning messages printed for cases 2 and > 3 > > respectively, > > > again, if somebody has a better descriptive string or with > better > > english I'll > > > change it: > > > > > > 2) "'%0' attribute only applies to functions with no > arguments" > > > 3) "'%0' attribute only applies to functions with a void > return type" > > > where %0 is either interrupt or signal > > > > > > > > > Semantics question: Do you mean that it issues just a *warning*? > Or does > > it error out on these? > > > > > > |
From: Weddington, E. <Eri...@at...> - 2012-03-06 22:14:31
|
Agreed. ;-) Eric > -----Original Message----- > From: Borja Ferrer [mailto:bor...@gm...] > Sent: Tuesday, March 06, 2012 12:44 PM > To: Weddington, Eric > Cc: avr...@li... > Subject: Re: [avr-llvm-devel] Interrupt handling > > In my opinion the best thing should be to error out, it doesn't make sense to > set this attributes in non void functions. Technically from a codegen > perspective it doesn't matter because registers are all saved and restored > unless the naked attrib is set so it wouldnt't make a difference except for > some useless instructions. Doing this kind of things shows that the programmer > doesn't understand what he's doing. > I issued the warning to be sort of backwards compat with gcc, although in this > case we would be supporting wrong code. In conclusion, error out. > > |
From: Borja F. <bor...@gm...> - 2012-03-08 11:51:58
|
Eric in case you're not following the "Current status?" thread, please check it out, I have a question for you there in the last message. 2012/3/6 Weddington, Eric <Eri...@at...> > > Agreed. ;-) > > Eric > > > -----Original Message----- > > From: Borja Ferrer [mailto:bor...@gm...] > > Sent: Tuesday, March 06, 2012 12:44 PM > > To: Weddington, Eric > > Cc: avr...@li... > > Subject: Re: [avr-llvm-devel] Interrupt handling > > > > In my opinion the best thing should be to error out, it doesn't make > sense to > > set this attributes in non void functions. Technically from a codegen > > perspective it doesn't matter because registers are all saved and > restored > > unless the naked attrib is set so it wouldnt't make a difference > except for > > some useless instructions. Doing this kind of things shows that the > programmer > > doesn't understand what he's doing. > > I issued the warning to be sort of backwards compat with gcc, although > in this > > case we would be supporting wrong code. In conclusion, error out. > > > > > |