riscv / riscv-fast-interrupt

Proposal for a RISC-V Core-Local Interrupt Controller (CLIC)
https://jira.riscv.org/browse/RVG-63
Creative Commons Attribution 4.0 International
239 stars 49 forks source link

More interrupt attributes needed #23

Closed Kevin-Andes closed 5 years ago

Kevin-Andes commented 5 years ago

This is a reminder that we need to create 2 more interrupt attributes:

  1. Interrupt ID: this attribute specifies the interrupt ID/number for each interrupt. With this information, the compiler can automatically generate a table of function pointers for all ISRs which will become the contents of the mtvt table.

  2. An attribute to specify whether each interrupt will use vector mode (inline-ABI) or common-code mode (C-ABI). This is used for selective vectoring.

jim-wilson commented 5 years ago

1) Constructing the interrupt table in the compiler sounds hard. Because of separate compilation, the compiler can't expect to see all of the functions. This actually has to be done in the linker. But the linker isn't designed to construct RISC-V vector tables. This might require a new linker feature which would be complicated. An alternative might be to add a library that has table of functions and somehow overwrite table entries, but then we have compiler plus linker plus library patch which is just as complicated. I would want to see a credible plan for how to implement this in both gcc and llvm before adding it to the specification.

2) The C-ABI model uses regular C functions. There is no interrupt attribute necessary there. Interrupt attributes are only for the inline-ABI case.

brucehoult commented 5 years ago
  1. I think the programmer should do this. You can create a table of function pointers anywhere in memory (sufficiently aligned) and make a single call/asm instruction to write a pointer to the table into mtvt

  2. I believe this is done by setting nvbits to 1 and then setting or clearing the LSB of clicintcfg[i] for each interrupt. If the bit is set for interrupt i then it is hardware vectored to an interrupt attribute function via the table pointed to by mtvt. If the bit for interrupt i is cleared then the (assembly language, possibly compiler runtime library) function pointer stored in mtvec is called and it eventually after saving registers etc calls the C ABI function pointer stored in the table pointed to by mtvt.

Thus, as I understand it, you can mix standard C functions and interrupt C functions in the same table of function pointers, and choose the calling mechanism for each one using the LSB of clicintcfg[i] for each interrupt i.

It is the programmer's responsibility to correctly set cliccfg and clicintcfg to make this work. This can of course be hidden in a library, with for example each interrupt set up with a call such as setInterrupt(int i, void (*handler)(), enum {CABI, InterruptABI} mode) and then the appropriate value for clicintcfg calculated and installed by another function.

kito-cheng commented 5 years ago

Hi Jim:

Constructing the interrupt table in the compiler sounds hard. Because of separate compilation, the compiler can't expect to see all of the functions. This actually has to be done in the linker. But the linker isn't designed to construct RISC-V vector tables. This might require a new linker feature which would be complicated. An alternative might be to add a library that has table of functions and somehow overwrite table entries, but then we have compiler plus linker plus library patch which is just as complicated. I would want to see a credible plan for how to implement this in both gcc and llvm before adding it to the specification.

Here is the approach in our NDS32 toolchain, we've construct the default one in weak symbol, and built that in an archive within GCC[1].

Compiler will generate strong symbol if interrupt N is given, then put pointer of interrupter handler into .nds32_vector.N section to make sure the right order during link time. That's relay on the behavior of the linker will merge section with same prefix, and then the default one will be override by linker due to strong/weak symbol merge rule.

So in our experience, it can be done in compiler plus a library without linker modification.

[1] https://github.com/riscv/riscv-gcc/tree/riscv-gcc-8.2.0/libgcc/config/nds32/isr-library

Kevin-Andes commented 5 years ago

Hi Bruce,

  1. I think the programmer should do this. You can create a table of function pointers anywhere in memory (sufficiently aligned) and make a single call/asm instruction to write a pointer to the table into mtvt

If there are only 10 interrupts, I definitely agree with you. However, if there are 500 interrupts, manually creating this function pointer table becomes a daunting task! Any slightest typo or misplacement in ordering will create hideous bugs. And, believe me, it is no fun to debug interrupt handlers. Therefore, this automation with compiler/linker will be very handy and save users tons of trouble/problems. As an aside, we have already implemented this attribute in GCC (for previous Andes architecture) and our customers like it a lot.

  1. I believe this is done by setting nvbits to 1 and then setting or clearing the LSB of clicintcfg[i] for each interrupt. If the bit is set for interrupt i then it is hardware vectored to an interrupt attribute function via the table pointed to by mtvt. If the bit for interrupt i is cleared then the (assembly language, possibly compiler runtime library) function pointer stored in mtvec is called and it eventually after saving registers etc calls the C ABI function pointer stored in the table pointed to by mtvt.

Thus, as I understand it, you can mix standard C functions and interrupt C functions in the same table of function pointers, and choose the calling mechanism for each one using the LSB of clicintcfg[i] for each interrupt i.

The reason I suggest to use an attribute is because the compiler needs to compile them differently. For in-line ABI, the compiler must only use callee-saved registers. On the other hand, for C-ABI, the compiler can treat the function normally.

It is the programmer's responsibility to correctly set cliccfg and clicintcfg to make this work. This can of course be hidden in a library, with for example each interrupt set up with a call such as setInterrupt(int i, void (*handler)(), enum {CABI, InterruptABI} mode) and then the appropriate value for clicintcfg calculated and installed by another function.

Alternatively, we can create similar “intrinsic functions” to register interrupt. The main advantage is that we no longer need (or rely on) the library anymore. Actually, we have also implemented these functions in GCC for our previous CPU products.

CONFIDENTIALITY NOTICE:

This e-mail (and its attachments) may contain confidential and legally privileged information or information protected from disclosure. If you are not the intended recipient, you are hereby notified that any disclosure, copying, distribution, or use of the information contained herein is strictly prohibited. In this case, please immediately notify the sender by return e-mail, delete the message (and any accompanying documents) and destroy all printed hard copies. Thank you for your cooperation.

Copyright ANDES TECHNOLOGY CORPORATION - All Rights Reserved.

Kevin-Andes commented 5 years ago

Sorry, I sent the previous reply by e-mail but the formatting got all dropped and messed up. Hope you can still find my replies and comments (under each section).

Kevin-Andes commented 5 years ago

Hi Bruce,

  1. I think the programmer should do this. You can create a table of function pointers anywhere in >memory (sufficiently aligned) and make a single call/asm instruction to write a pointer to the table >into mtvt

If there are only 10 interrupts, I definitely agree with you. However, if there are 500 interrupts, manually creating this function pointer table becomes a daunting task! Any slightest typo or misplacement in ordering will create hideous bugs. And, believe me, it is no fun to debug interrupt handlers. Therefore, this automation with compiler/linker will be very handy and save users tons of trouble/problems. As an aside, we have already implemented this attribute in GCC (for previous Andes architecture) and our customers like it a lot.

  1. I believe this is done by setting nvbits to 1 and then setting or clearing the LSB of clicintcfg[i] for >each interrupt. If the bit is set for interrupt i then it is hardware vectored to an interrupt attribute >function via the table pointed to by mtvt. If the bit for interrupt i is cleared then the (assembly >language, possibly compiler runtime library) function pointer stored in mtvec is called and it >eventually after saving registers etc calls the C ABI function pointer stored in the table pointed to by >mtvt.

The reason I suggest to use an attribute is because the compiler needs to compile them differently. For in-line ABI, the compiler must only use callee-saved registers. On the other hand, for C-ABI, the compiler can treat the function normally.

It is the programmer's responsibility to correctly set cliccfg and clicintcfg to make this work. This can >of course be hidden in a library, with for example each interrupt set up with a call such as >setInterrupt(int i, void (*handler)(), enum {CABI, InterruptABI} mode) and then the appropriate value >for clicintcfg calculated and installed by another function.

Alternatively, we can create similar “intrinsic functions” to register interrupt. The main advantage is that we no longer need (or rely on) the library anymore. Actually, we have also implemented these functions in GCC for our previous CPU products.

ilg-ul commented 5 years ago

I concur with Jim & Bruce, the array of pointers to interrupt handlers is an application issue and should not be a compiler/linker concern; the compiler should be neutral to device specifics.

In the Cortex-M world, the silicone vendors provide a file with this flash stored array (part of CMSIS), pointing to fixed name functions, defaulting to a no-op handler.

The application programmer does not have to manually create this table, he only has to include this file and redefine the handlers relevant for his application; very convenient.

If you prefer to maintain a custom compiler which includes such tables for all your devices, feel free to continue to do so, but I think it is not realistic to expect all silicone vendors to do the same, or to contribute patches to the compiler for all their distinct devices.

Kevin-Andes commented 5 years ago

If most people think it is fine to manually create the vector table (a table of function pointers up to 1024 items), then we don't need these new attributes. We can always revisit and create them in the future if people find them necessary.