Skip to content

[clang] Clang ignores parameter for C23 gnu::interrupt attribute on RISC-V64 #140701

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
MofX opened this issue May 20, 2025 · 7 comments · Fixed by #140828
Closed

[clang] Clang ignores parameter for C23 gnu::interrupt attribute on RISC-V64 #140701

MofX opened this issue May 20, 2025 · 7 comments · Fixed by #140828
Labels
c23 clang:codegen IR generation bugs: mangling, exceptions, etc. confirmed Verified by a second party diverges-from:gcc Does the clang frontend diverge from gcc on this issue good first issue https://github.com/llvm/llvm-project/contribute

Comments

@MofX
Copy link

MofX commented May 20, 2025

When trying to create a supervisor trap handler on RISC-V64 using C23 attribute syntax, the parameter "supervisor" seems to be ignored and a machine trap handler is generated.

I.e. the following snipped generates an mret instead of an sret for the c23 variant, while the correct one is generated for the pre_c23 variant:

__attribute__((interrupt("supervisor"))) void pre_c23(){}

[[gnu::interrupt("supervisor")]] void c23(){}

gcc generates the correct code in both variants, see also in compiler explorer: https://godbolt.org/z/zeEjM8f5E
@llvmbot llvmbot added the clang Clang issues not falling into any other category label May 20, 2025
@EugeneZelenko EugeneZelenko added c23 clang:codegen IR generation bugs: mangling, exceptions, etc. diverges-from:gcc Does the clang frontend diverge from gcc on this issue and removed clang Clang issues not falling into any other category labels May 20, 2025
@llvmbot
Copy link
Member

llvmbot commented May 20, 2025

@llvm/issue-subscribers-clang-codegen

Author: Jörg Vehlow (MofX)

When trying to create a supervisor trap handler on RISC-V64 using C23 attribute syntax, the parameter "supervisor" seems to be ignored and a machine trap handler is generated.

I.e. the following snipped generates an mret instead of an sret for the c23 variant, while the correct one is generated for the pre_c23 variant:

__attribute__((interrupt("supervisor"))) void pre_c23(){}

[[gnu::interrupt("supervisor")]] void c23(){}

gcc generates the correct code in both variants, see also in compiler explorer: https://godbolt.org/z/zeEjM8f5E

@AaronBallman AaronBallman added the confirmed Verified by a second party label May 20, 2025
@AaronBallman
Copy link
Collaborator

Well this is neat: https://godbolt.org/z/fdo58xPE9

TranslationUnitDecl
|-FunctionDecl <line:1:1, col:57> col:47 pre_c23 'void (void)'
| |-CompoundStmt <col:56, col:57>
| `-RISCVInterruptAttr <col:16, col:38> supervisor
|-FunctionDecl <line:3:34, col:45> col:39 c23 'void (void)'
| |-CompoundStmt <col:44, col:45>
| `-RISCVInterruptAttr <col:3, col:8> machine

So it seems the [[]] spelling is deciding to use machine as the argument.

@AaronBallman
Copy link
Collaborator

We don't recognize the attribute (https://godbolt.org/z/nKj3qPePs) because the tablegen code produces a string switch with duplicate entries:

llvm-project/clang/utils/TableGen/ClangAttrEmitter.cpp

Line 3667 in 8708c42

static void GenerateHasAttrSpellingStringSwitch( 

} else if (ScopeName == "gnu") {
  return llvm::StringSwitch<int>(Name)
    .Case("interrupt", true && (T.getArch() == llvm::Triple::arm || T.getArch() == llvm::Triple::thumb || T.getArch() == llvm::Triple::armeb || T.getArch() == llvm::Triple::thumbeb) ? 1 : 0)
    .Case("interrupt", true && (T.getArch() == llvm::Triple::avr) ? 1 : 0)
    .Case("signal", true && (T.getArch() == llvm::Triple::avr) ? 1 : 0)
    .Case("alias", 1)
    .Case("aligned", 1)
    .Case("alloc_align", 1)
    .Case("alloc_size", 1)
    .Case("always_inline", 1)
    .Case("interrupt", true && (T.getArch() == llvm::Triple::x86 || T.getArch() == llvm::Triple::x86_64) ? 1 : 0)
....

Note that we have interrupt in there several times (there are more farther down, too). I think we need the table generator to emit a single Case for duplicate spellings and have the value returned be the union of all the predicates.

@AaronBallman AaronBallman added the good first issue https://github.com/llvm/llvm-project/contribute label May 20, 2025
@llvmbot
Copy link
Member

llvmbot commented May 20, 2025

Hi!

This issue may be a good introductory issue for people new to working on LLVM. If you would like to work on this issue, your first steps are:

  1. Check that no other contributor has already been assigned to this issue. If you believe that no one is actually working on it despite an assignment, ping the person. After one week without a response, the assignee may be changed.
  2. In the comments of this issue, request for it to be assigned to you, or just create a pull request after following the steps below. Mention this issue in the description of the pull request.
  3. Fix the issue locally.
  4. Run the test suite locally. Remember that the subdirectories under test/ create fine-grained testing targets, so you can e.g. use make check-clang-ast to only run Clang's AST tests.
  5. Create a Git commit.
  6. Run git clang-format HEAD~1 to format your changes.
  7. Open a pull request to the upstream repository on GitHub. Detailed instructions can be found in GitHub's documentation. Mention this issue in the description of the pull request.

If you have any further questions about this issue, don't hesitate to ask via a comment in the thread below.

@llvmbot
Copy link
Member

llvmbot commented May 20, 2025

@llvm/issue-subscribers-good-first-issue

Author: Jörg Vehlow (MofX)

When trying to create a supervisor trap handler on RISC-V64 using C23 attribute syntax, the parameter "supervisor" seems to be ignored and a machine trap handler is generated.

I.e. the following snipped generates an mret instead of an sret for the c23 variant, while the correct one is generated for the pre_c23 variant:

__attribute__((interrupt("supervisor"))) void pre_c23(){}

[[gnu::interrupt("supervisor")]] void c23(){}

gcc generates the correct code in both variants, see also in compiler explorer: https://godbolt.org/z/zeEjM8f5E

@Mr-Anyone
Copy link
Contributor

I'd like to work on this.

@shafik
Copy link
Collaborator

shafik commented May 20, 2025

@Mr-Anyone please, open a PR and then we can assign the issue to you.

Mr-Anyone added a commit to Mr-Anyone/llvm-project that referenced this issue May 21, 2025
@Mr-Anyone
[clang][TableGen] Fix Duplicate Entries in TableGen
640bc8b 
Fixed TableGen duplicate issues that causes the wrong interrupt
attribute from being selected.

resolves llvm#140701
@Mr-Anyone Mr-Anyone mentioned this issue May 21, 2025
Merged
Mr-Anyone added a commit to Mr-Anyone/llvm-project that referenced this issue May 21, 2025
@Mr-Anyone
[clang][TableGen] Fix Duplicate Entries in TableGen
f9b4429 
Fixed TableGen duplicate issues that causes the wrong interrupt
attribute from being selected.

resolves llvm#140701
Mr-Anyone added a commit to Mr-Anyone/llvm-project that referenced this issue May 21, 2025
@Mr-Anyone
[clang][TableGen] Fix Duplicate Entries in TableGen
393ccc1 
Fixed TableGen duplicate issues that causes the wrong interrupt
attribute from being selected.

resolves llvm#140701
AaronBallman pushed a commit that referenced this issue May 21, 2025
@Mr-Anyone
[clang][TableGen] Fix Duplicate Entries in TableGen (#140828)
385752c 
Fixed TableGen duplicate issues that causes the wrong interrupt
attribute from being selected.

resolves #140701
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
c23 clang:codegen IR generation bugs: mangling, exceptions, etc. confirmed Verified by a second party diverges-from:gcc Does the clang frontend diverge from gcc on this issue good first issue https://github.com/llvm/llvm-project/contribute
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[clang][TableGen] Fix Duplicate Entries in TableGen Mr-Anyone/llvm-project
6 participants
@MofX @AaronBallman @shafik @EugeneZelenko @Mr-Anyone @llvmbot