Skip to content

ume: Add interpolation to rv64ume and custom template tags - #834

Open
arcane-quill wants to merge 28 commits into
masterfrom
wip/ume-viam-def
Open

ume: Add interpolation to rv64ume and custom template tags#834
arcane-quill wants to merge 28 commits into
masterfrom
wip/ume-viam-def

Conversation

@arcane-quill

@arcane-quill arcane-quill commented Mar 22, 2026

Copy link
Copy Markdown
Contributor

PR for issue #757 (and #761, #759)
interpolated hardcoded rv64ume files from linux-user
created UmeHardcodedRiscvDefinitionPass and UmeTemplateRenderingPass to add custom template tags (e.g. registers, exceptions..) with hardcoded values that later get filled in by the vadl file
added VIAM definition UserModeEmulation

@flofriday

flofriday commented Mar 22, 2026

Copy link
Copy Markdown
Contributor

I think the branch of this PR isn't cleanly based on a fresh master, because it contains some of my commits that are already on master.

So let's rebase on master with (on your branch):

git fetch
git rebase origin/master

There might be some conflicts but if you execute these commands in IntelliJ you'll get some help resolving the conflicts in a nice UI. 😉

@arcane-quill

Copy link
Copy Markdown
Contributor Author

I think the branch of this PR isn't cleanly based on a fresh master, because it contains some of my commits that are already on master.

So let's rebase on master with (on your branch):

git fetch
git rebase origin/master

There might be some conflicts but if you execute these commands in IntelliJ you'll get some help resolving the conflicts in a nice UI. 😉

I tried to rebase twice now but it doesnt seem to be fixed @flofriday

@flofriday

Copy link
Copy Markdown
Contributor

Yeah sometimes rebasing still contains some weired artifacts, but I just tried it on my machine and it (mostly) worked. I'll send you a DM (on matrix), let's figure it out there.

@arcane-quill
arcane-quill force-pushed the wip/ume-viam-def branch 2 times, most recently from ce9eaf4 to 0d64625 Compare March 22, 2026 19:08
@arcane-quill arcane-quill changed the title Wip/ume viam def ume: Add interpolation to rv64ume and custom template tags Mar 23, 2026
@arcane-quill
arcane-quill force-pushed the wip/ume-viam-def branch 2 times, most recently from 4d6afb8 to f54e597 Compare April 4, 2026 02:18
@arcane-quill
arcane-quill force-pushed the wip/ume-viam-def branch 2 times, most recently from 4d6afb8 to f54e597 Compare April 12, 2026 01:50
Comment thread vadl/main/vadl/ast/AstVisitor.java

@Jozott00 Jozott00 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately the current state is too RISC-V specific and needs an overhaul. Please take a closer look at other VIAM definitions and how they are handled in the code emission.

Comment thread vadl/main/resources/templates/iss/linux-user/gen-arch/cpu_loop.c Outdated
Comment thread vadl/main/resources/templates/iss/linux-user/gen-arch/cpu_loop.c Outdated
Comment thread vadl/main/resources/templates/iss/linux-user/gen-arch/cpu_loop.c Outdated
Comment thread vadl/main/resources/templates/iss/linux-user/gen-arch/cpu_loop.c Outdated
Comment thread vadl/main/resources/templates/iss/linux-user/gen-arch/signal.c Outdated
Comment thread vadl/main/resources/templates/iss/linux-user/gen-arch/signal.c Outdated
Comment thread vadl/main/vadl/iss/passes/UmeTemplateRenderingPass.java Outdated
Comment thread vadl/main/vadl/pass/PassOrders.java Outdated
Comment thread vadl/main/vadl/viam/UserModeEmulation.java Outdated
Comment thread vadl/main/vadl/viam/UserModeEmulation.java Outdated
@arcane-quill

Copy link
Copy Markdown
Contributor Author

Unfortunately the current state is too RISC-V specific and needs an overhaul. Please take a closer look at other VIAM definitions and how they are handled in the code emission.

Thanks for the extensive review, I'm working on it now

@arcane-quill

Copy link
Copy Markdown
Contributor Author

hi @Jozott00 I resolved the stuff that was missing, hopefully we can get this merged now

and do u know why passorders keeps telling me there is a merge conflict despite none of it showing up locally, even after rebasing?

@arcane-quill
arcane-quill requested a review from Jozott00 May 9, 2026 10:55
@flofriday

flofriday commented May 9, 2026

Copy link
Copy Markdown
Contributor

and do u know why passorders keeps telling me there is a merge conflict despite none of it showing up locally, even after rebasing?

Mostly guesses here but I assume you are not syncing your local branches with the remote (GitHub).

For example

git rebase master

Isn't good enough to rebase with the newest changes from GitHub.
This is mostly because master on your machine isn't updated and is stuck on an old commit.

The long way is to do:

git checkout master
git pull # (download from the remote, and also update the current master branch to the newest version from the remote)
git checkout <your-branch>
git rebase master

But you can even shorten this a bit with

git fetch # Downloads the branches from the remote but does not update the local branches only the branches prefixed with origin will be updated, so master is still stuck on an old commit but origin/master is synced with the remote
git rebase origin/master

@Jozott00 Jozott00 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there are still some important things to change. Mostly, what and how the VIAM definition stores specification details.

I also think, that we should reduce the scope for the PR a bit, as signals are just a bit too much to start with. Therefore, let's completely remove all logic that is related to signals.

The UME generation requires quite a bit of information from the ABI definition and could share some constructs (like alias registers and special register references). Therefore, I will do a little ABI refactoring, so you can use RegisterRef instead of defining you own RegisterLocation as mentioned in a comment below.

EDIT: Also note, that this PR won't fully close #761 as there is still RISC-V specific stuff in the template files.

Comment thread vadl/main/vadl/viam/UserModeEmulation.java Outdated
Comment thread vadl/main/vadl/viam/UserModeEmulation.java Outdated
Comment thread vadl/main/vadl/viam/UserModeEmulation.java Outdated
Comment thread vadl/main/vadl/viam/UserModeEmulation.java Outdated
Comment thread vadl/main/vadl/viam/UserModeEmulation.java Outdated
Comment thread vadl/main/resources/templates/iss/linux-user/gen-arch/cpu_loop.c Outdated
Comment thread vadl/main/resources/templates/iss/linux-user/gen-arch/cpu_loop.c Outdated
env->pc = regs->sepc;
env->x[RV64UME_REG_SP] = regs->sp;
env->[(${pc_reg.name_lower})] = regs->[(${config.initialPc})];
env->[(${config.mainRegisterFile})][ [(${config.spReg})] ] = regs->[(${config.initalSp})];

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

major: As we just hardcode the target_pt_regs anyway, just use regs->pc and regs->sp instead.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Jozott00 can I add pc to the struct? else it will always give me this error " ‘struct target_pt_regs’ has no member named ‘pc’; did you mean ‘sepc’?" for regs->pc

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor: This whole table is currently hardcoded. However, I think we should do the dynamic interpolation in a separate PR.

Comment thread vadl/main/vadl/pass/PassOrders.java Outdated
Jozott00 added a commit that referenced this pull request May 11, 2026
…` interface (#948)

This PR extracts most of the `Abi.RegisterRef` to
`vadl.viam.RegisterRef`, so the datastructure can be reused in other
definitions like the UME (#834).

Additionally, it removes the `GeneratesRegisterFileName` which does not
fit into the VIAM hierarchy and is already covered by the
`RegisterResource` class.

@Jozott00 Jozott00 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your branch is currently conflicting with origin/master. This is because we moved the ISS PassOrder construction to IssPassOrder.

Additionally there are still some issues from the last review that were not addressed yet.

Also please test the generated QEMU, I think at the PR's current state, the generated QEMU wouldn't work.

Comment thread sys/risc-v/rv64ume.vadl
Comment on lines +8 to +36
alias register a0 = X(10)
alias register a1 = X(11)
alias register a2 = X(12)
alias register a3 = X(13)
alias register a4 = X(14)
alias register a5 = X(15)
alias register a6 = X(16)
alias register a7 = X(17)
alias register sp = X(2)
alias register ra = X(1)
alias register gp = X(3)
alias register fp = X(8)

return address = ra
stack pointer = sp
return value = a0
frame pointer = fp
global pointer = gp
function argument = a{0..7}
caller saved = [ a{0..7} ]
callee saved = [ ra, fp,
X(9), X(18), X(19), X(20), X(21),
X(22), X(23), X(24), X(25), X(26), X(27) ]

special absolute address load instruction = LA
special local address load instruction = LLA

special return instruction = RET
special call instruction = CALL

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fine for now, later this should be minimized to a minimal set of definitions that is required for the UME (#968).

private final InstructionSetArchitecture isa;
private final Abi abi;

private final List<RegisterUtils.Register> args;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

major: You are still using the LCB template RegisterUtils instead of the RegisterRef as added in #948.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea I'm having issues here because my local master branch won't properly update (to pull in the RegisterRef), so I think I'd need to rebase, but I'm behind quite a few commits and I'm worried I'll mess up the history

@Jozott00 Jozott00 Jun 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should be able to merge origin/master into your feature branch.
I will squash this PR during merge, so the branch-local commit history doesn't really matter.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I'll do a merge of master into this branch instead of a rebase then

Comment thread vadl/main/vadl/iss/passes/UmeTemplateRenderingPass.java Outdated
Comment thread vadl/main/vadl/iss/passes/UmeTemplateRenderingPass.java Outdated
Comment thread vadl/main/vadl/iss/passes/UmeTemplateRenderingPass.java Outdated
Comment thread vadl/main/vadl/viam/UserModeEmulation.java Outdated
Comment thread vadl/main/vadl/iss/passes/UmeHardcodedRiscvDefinitionPass.java Outdated
Comment thread vadl/main/vadl/iss/passes/UmeHardcodedRiscvDefinitionPass.java Outdated
Comment thread vadl/main/vadl/iss/passes/UmeHardcodedRiscvDefinitionPass.java
@github-actions github-actions Bot added the hardware Related to the MiA and hardware generation label Jun 3, 2026
@Jozott00
Jozott00 force-pushed the wip/ume-viam-def branch from 5c4264c to 1379c83 Compare June 3, 2026 17:20
@arcane-quill

Copy link
Copy Markdown
Contributor Author

I'm retrieving the args from the ABI now, is it fine like this?

@Jozott00

Jozott00 commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

I'm retrieving the args from the ABI now, is it fine like this?

You mean the syscall argument registers? Those must come from the UME definition and can't be taken from the ABI, as in general, the ABI argument registers and the syscall argument registers are not the same.

@arcane-quill

Copy link
Copy Markdown
Contributor Author

I'm retrieving the args from the ABI now, is it fine like this?

You mean the syscall argument registers? Those must come from the UME definition and can't be taken from the ABI, as in general, the ABI argument registers and the syscall argument registers are not the same.

right but since I only have the ABI right now, I was thinking it might be a bit cleaner than using the hardcoded version, but I can also revert it back to the way it was to avoid confusion

@Jozott00

Jozott00 commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

I think for now you should create the register references the hardcoded way. However, don't create the X register tensor itself.

@arcane-quill

Copy link
Copy Markdown
Contributor Author

@Jozott00 I'm trying to get the cpuloop to work using the synthetic syscall exception. Is what I implemented in my latest commit the right approach?

@Jozott00

Copy link
Copy Markdown
Contributor

Yes, looks good to me 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

frontend This is frontend related hardware Related to the MiA and hardware generation iss This is ISS related lcb This is LCB related

Projects

None yet

3 participants