Alexei Colin via llvm-dev
2018-May-25 22:59 UTC
[llvm-dev] MSP430: interrupt vector number out of bounds error in v6/trunk (with patch)
When building with Clang for the MSP430 architecture against headers distributed with TI MSP GCC, interrupt service routine interrupt(vector_number) attribute is rejected: __attribute__ ((interrupt(USCI_A0_VECTOR))) void ISR(void) { } error: 'interrupt' attribute parameter 48 is out of bounds This is due to the check in tools/clang/lib/Sema/SemaDeclAttr.cpp:5104 unsigned Num = NumParams.getLimitedValue(255); if ((Num & 1) || Num > 30) { S.Diag(AL.getLoc(), diag::err_attribute_argument_out_of_bounds) << AL.getName() << (int)NumParams.getSExtValue() << NumParamsExpr->getSourceRange(); return; } Also, the __isr_ symbol is emitted with the vector number divided by 2. // Step 3: Emit ISR vector alias. unsigned Num = attr->getNumber() / 2; llvm::GlobalAlias::create(llvm::Function::ExternalLinkage, "__isr_" + Twine(Num), F); Eliminating the check causes a duplicate symbol error among __isr_ symbols, because odd and even vector numbers generate the same symbol. Neither the check for even and under 30, nor the division by 2 is necessary when compiling and assembling with the TI MSP GCC, whose msp430*.h headers define vector number macros, like. #define EUSCI_A0_VECTOR (48) /* 0xFFF0 */ My guess is that the current LLVM code from above has been left over from very old and deprecated community project MSPGCC, which is very different from TI MSP GCC and is no longer supported. To make interrupts work, I had to apply attached patch and then in the app code, define the symbol manually (name of the symbol does not matter) and allocate it to the section __interrupt_vector_ for that vector number that is defined by the linker script shipped by TI MSP GCC: #define VNUM(x) x // to drop parenthesis around *_VECTOR values from TI #define ISR_NAME(vect) CONCAT(ISR_, vect) #define ISR(vect) \ void ISR_NAME(VNUM vect)(void); \ __attribute__((section("__interrupt_vector_" STR(VNUM vect)), aligned(2))) \ void(*CONCAT(__vector_, VNUM vect))(void) = ISR_NAME(VNUM vect); \ __attribute__ ((interrupt(vect))) void ISR_NAME(VNUM vect)(void) ISR(USCI_A0_VECTOR) { } Perhaps a better fix would be to emit the alias and allocate it in the correct section (by name, as I did above manually), but I couldn't find a way to do this in LLVM. With the current patch, the vector number is unused, so might need to be removed entirely. Note that with TI MSP GCC, it is not necessary to define the symbol and allocate it manually; the following just works: #define ISR(vect) \ __attribute__ ((interrupt(vect))) void ISR_NAME(VNUM vect)(void) ISR(USCI_A0_VECTOR) { } Same info here: https://stackoverflow.com/questions/33266132/how-to-define-an-interrupt-service-routine-for-msp430-with-llvm-clanggcc/33266133 -------------- next part -------------->From 241036d9abc5b754d5ef8afbb6cbf28c6b40768f Mon Sep 17 00:00:00 2001From: Alexei Colin <ac at alexeicolin.com> Date: Fri, 25 May 2018 18:16:56 -0400 Subject: [PATCH] MSP430: accept all interrupt numbers and don't emit alias This works around compile error that interrupt vector number in the interrupt() attribute is out of bounds, when building with TI MSP GCC. Despite both GCC and Clang using the same linker script, for Clang, we need to manually create a symbol and allocate it in the per-vector section (named with a name that matches the names that the linker script expects). Note that Clang creates __isr_* symbols automatically, but these are only interpreted by the deprecated MSPGCC compiler/linker, not by TI GCC for MSP. See more on this below. Clang creates __isr_* symbols for MSPGCC (see lib/CodeGen/TargetInfo.cpp MSP430TargetCodeGenInfo::SetTargetAttributes), these are not relevant for TI GCC, but they are not entirely harmless. They cause a problem because Clang generates __isr_N where N is divided by 2 (because MSPGCC wants it that way, I guess). So, if we pass a vector number, the odd and even vector numbers will generate the same symbol and therefore linking will fail with a multiply-defined symbol error. As a workaround we pass the doubled value. There is one more problem: Clang rejects vector numbers that are odd or above 30. So, this problem with the above problem form a catch-22 situation: if we pass N/2 then the atttribute is accepted, but we get multiply-defined values; if we pass N, it's rejected; and if we pass N*2, it's rejected. So, the workaround is to not emit the __isr symbol at all, and to accept the full range of interrupt vector numbers. --- lib/CodeGen/TargetInfo.cpp | 5 ----- lib/Sema/SemaDeclAttr.cpp | 2 +- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/lib/CodeGen/TargetInfo.cpp b/lib/CodeGen/TargetInfo.cpp index 4e25c72cfb..a42248edcf 100644 --- a/lib/CodeGen/TargetInfo.cpp +++ b/lib/CodeGen/TargetInfo.cpp @@ -6655,11 +6655,6 @@ void MSP430TargetCodeGenInfo::setTargetAttributes( // Step 2: Add attributes goodness. F->addFnAttr(llvm::Attribute::NoInline); - - // Step 3: Emit ISR vector alias. - unsigned Num = attr->getNumber() / 2; - llvm::GlobalAlias::create(llvm::Function::ExternalLinkage, - "__isr_" + Twine(Num), F); } } } diff --git a/lib/Sema/SemaDeclAttr.cpp b/lib/Sema/SemaDeclAttr.cpp index e4532a7e67..094087c886 100644 --- a/lib/Sema/SemaDeclAttr.cpp +++ b/lib/Sema/SemaDeclAttr.cpp @@ -5101,7 +5101,7 @@ static void handleMSP430InterruptAttr(Sema &S, Decl *D, } unsigned Num = NumParams.getLimitedValue(255); - if ((Num & 1) || Num > 30) { + if (Num > 55) { S.Diag(AL.getLoc(), diag::err_attribute_argument_out_of_bounds) << AL.getName() << (int)NumParams.getSExtValue() << NumParamsExpr->getSourceRange(); -- 2.17.0
Vadzim Dambrouski via llvm-dev
2018-May-26 15:12 UTC
[llvm-dev] MSP430: interrupt vector number out of bounds error in v6/trunk (with patch)
For some time I've been using this defines that allowed me compile the code using both clang and gcc: #ifdef __clang__ #define ISR_BEGIN(id) __attribute__((interrupt(0))) #else #define ISR_BEGIN(id) __attribute__((interrupt(id), used)) #endif #ifdef __clang__ #define ISR_END(name) \ __attribute__((section("__interrupt_vector_" #name), aligned(2))) void ( \ *__vector_##name)(void) = name; #else #define ISR_END(name) #endif And I use it like this ISR_BEGIN(USCI_A0_VECTOR) void usci_a0(void) { } ISR_END(usci_a0) ISR_BEGIN(USCI_B0_VECTOR) void usci_b0(void) { } ISR_END(usci_b0) But of course fixing it in the compiler would be great.