Skip to content
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.

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

jalr IDL and description is unclear and potentially invalid #392

Open
ThePuzzlemaker opened this issue Jan 7, 2025 · 2 comments · May be fixed by #402
Open

jalr IDL and description is unclear and potentially invalid #392

ThePuzzlemaker opened this issue Jan 7, 2025 · 2 comments · May be fixed by #402
Labels
data error An error in the database data

Comments

@ThePuzzlemaker
Copy link

The ISA manual and Sail code specify that jalr clears the LSB of the calculated target address before jumping. The IDL code and description in this database do not specify this (not even in the jump function, to my knowledge), potentially making these entries invalid.

Current value and location

jump(X[rs1] + imm)

https://github.com/riscv-software-src/riscv-unified-db/blob/main/arch/inst/I/jalr.yaml#L30

Expected value
If I understand IDL correctly, it should be something vaguely like this:

jump({(X[rs1] + imm)[XLEN - 1:1], 1'b0})

The description should also be updated in my opinion to better clarify this behaviour.

@ThePuzzlemaker ThePuzzlemaker added the data error An error in the database data label Jan 7, 2025
@ThinkOpenly
Copy link
Collaborator

ThinkOpenly commented Jan 7, 2025

Good catch!

You are correct about the jump function. It does not clear any bits. It will, however, raise an exception of the target address has low-order bits set (https://github.com/riscv-software-src/riscv-unified-db/blob/4d8b28a63a34be7262c0537962c0f3e8ef88bf43/arch/isa/globals.isa#L703C1-L710C1):

    # raise a misaligned exception if address is not aligned to IALIGN
    if ((ialign() == 16) && ((target_addr & 0x1) != 0)) {
      # the target PC is not halfword-aligned
      raise(ExceptionCode::InstructionAddressMisaligned, mode(), target_addr);
    } else if ((target_addr & 0x3) != 0) {
      # the target PC is not word-aligned
      raise(ExceptionCode::InstructionAddressMisaligned, mode(), target_addr);
    }

Another approach with IDL (to my understanding of IDL):

jump((X[rs1] + imm) & ~1)

@dhower-qc
Copy link
Collaborator

Yep, good catch. Either fix (Wren's or Paul's) should fix it. @ThePuzzlemaker, would you be able to make a PR for the fix?

@ThePuzzlemaker ThePuzzlemaker linked a pull request Jan 9, 2025 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
data error An error in the database data
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants