Skip to content

[snippy] Dump initial registers without model#421

Open
MaxGud10 wants to merge 2 commits into
syntacore:mainfrom
MaxGud10:feat/dump-initial-registers-without-model
Open

[snippy] Dump initial registers without model#421
MaxGud10 wants to merge 2 commits into
syntacore:mainfrom
MaxGud10:feat/dump-initial-registers-without-model

Conversation

@MaxGud10

@MaxGud10 MaxGud10 commented Apr 2, 2026

Copy link
Copy Markdown
Contributor

Fixes #12

Previously, initial register state was dumped only through the simulator path.

As a result, --dump-initial-registers-yaml did not work when model-plugin was
set to None, even though the initial register state was already available.

This change dumps the initial register state directly in the no-model path.

Added a regression test based on the reproducer from the issue.

@MaxGud10 MaxGud10 changed the title Feat/dump initial registers without model [snippy] Dump initial registers without model Apr 2, 2026
@MaxGud10 MaxGud10 force-pushed the feat/dump-initial-registers-without-model branch from 276889c to 7054dff Compare April 2, 2026 23:42

@arromanoff arromanoff 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.

In general it now looks like snippy will dump initial registers with and without model via different mechanisms (in simulator and this new dump... function). Can we unify them instead?

@@ -0,0 +1,20 @@
# RUN: rm -f %t.initial-regs.yaml
# RUN: llvm-snippy %s -mtriple=riscv64-unknown-elf -num-instrs=10 \

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.

Suggested change
# RUN: llvm-snippy %s -mtriple=riscv64-unknown-elf -num-instrs=10 \
# RUN: llvm-snippy %s -mtriple=riscv64 -num-instrs=10 \

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.

Also add -model-plugin None

@@ -0,0 +1,20 @@
# RUN: rm -f %t.initial-regs.yaml

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.

Please, don't

Comment on lines +7 to +17
sections:
- name: text
VMA: 0x1000
LMA: 0x1000
SIZE: 0x1000
ACCESS: rx
- name: data
VMA: 0x3000
LMA: 0x3000
SIZE: 0x1000
ACCESS: rw

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.

Can we reuse sections from Inputs/ directory?

@MaxGud10 MaxGud10 force-pushed the feat/dump-initial-registers-without-model branch from 7054dff to e9ab843 Compare June 20, 2026 15:46
@MaxGud10

Copy link
Copy Markdown
Contributor Author

Updated the PR on top of the current main and addressed the review comments.

Initial register dumping is now handled through SnippyProgramContext and reused from both model and no-model paths. The test was also updated to use Inputs/sections.yaml, remove the manual cleanup step, and explicitly pass --model-plugin=None.

const TargetSubtargetInfo &STI,
const RegPoolWrapper &RP) {
assert(!TargetContext && "Double context insertion");
InitialStateSubtarget = &STI;

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 don't like this new member and it's usage
We dump register state after we generated code so there're no problems with getting subtarget from LLVMState at this point. Just call getLLVMState().getSubtargetImpl

Comment on lines +507 to +509
ProgContext.dumpInitialRegisterState(
PassCfg.RegistersConfig.InitialStateOutputYaml);

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.

There on the call site you can get the subtarget

auto *EntryFunc  = MainModule.getFunction(ProgramCfg.EntryPointName);
assert(EntryFunc);
auto &ST = State.getSubtargetImpl(*EntryFunc);
// Now you can call
dumpInitialRegisterState(PassCfg.RegistersConfig.InitialStateOutputYaml, ST);

const IRegisterState &
getInitialRegisterState(const TargetSubtargetInfo &ST) const;

void dumpInitialRegisterState(StringRef YamlPath) const;

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.

Hence this overload is redundant


void dumpInitialRegisterState(StringRef YamlPath) const;
void dumpInitialRegisterState(StringRef YamlPath,
const TargetSubtargetInfo &ST) const;

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'd also separate concerns here. Let's separate fetching this state and dumping it. So in FlowGenerator you can do:

auto &RegState = ProgramCtx.getInitialRegisterState(ST);
RegState.saveAsYAMLFile(PassCfg.RegistersConfig.InitialStateOutputYaml);

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

--dump-initial-registers-yaml shall work without model plugin

2 participants