Skip to content

[snippy] Reject function-number/function-layers with explicit call-graph#420

Open
MaxGud10 wants to merge 1 commit into
syntacore:mainfrom
MaxGud10:fix/405-call-graph-function-number
Open

[snippy] Reject function-number/function-layers with explicit call-graph#420
MaxGud10 wants to merge 1 commit into
syntacore:mainfrom
MaxGud10:fix/405-call-graph-function-number

Conversation

@MaxGud10

@MaxGud10 MaxGud10 commented Apr 2, 2026

Copy link
Copy Markdown
Contributor

Fixes #405

Previously, when an explicit call-graph was provided, function-number was
silently ignored instead of being diagnosed as an invalid configuration.

This change rejects configurations that specify an explicit call-graph together
with function-number or function-layers.

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

Updated existing tests that used now-invalid config combinations.

# CHECK: error: Options "function-number" cannot be used together with "call-graph"

options:
mtriple: riscv64-unknown-elf

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
mtriple: riscv64-unknown-elf
mtriple: riscv64

Comment on lines +12 to +27
sections:
- name: text
VMA: 0x210000
SIZE: 0x100000
LMA: 0x210000
ACCESS: rx
- name: data
VMA: 0x100000
SIZE: 0x50000
LMA: 0x100000
ACCESS: rw
- name: rodata
VMA: 0x310000
SIZE: 0x50000
LMA: 0x360000
ACCESS: r

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.

reuse sections from Inputs/ directory if possible

Comment on lines +37 to +38
static bool isFunctionLayersSpecified();
static bool isFunctionNumberSpecified();

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.

Why static?

Comment on lines +1067 to +1084
if (std::holds_alternative<FunctionDescs>(Info.PassCfg.CGLayout)) {
std::string ConflictingOpts;

if (CallGraphLayout::isFunctionNumberSpecified())
ConflictingOpts = "function-number";

if (CallGraphLayout::isFunctionLayersSpecified()) {
if (!ConflictingOpts.empty())
ConflictingOpts += ", ";
ConflictingOpts += "function-layers";
}

if (!ConflictingOpts.empty())
return formatv("Options \"{0}\" cannot be used together with "
"\"call-graph\"",
ConflictingOpts)
.str();
}

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 understand what you want to achieve here but let's just report them separately.
With 2 separate return's

@MaxGud10 MaxGud10 force-pushed the fix/405-call-graph-function-number branch from 4e50804 to 21742b2 Compare June 30, 2026 17:18
@MaxGud10 MaxGud10 force-pushed the fix/405-call-graph-function-number branch from 21742b2 to 2be1ac0 Compare June 30, 2026 17:52
@MaxGud10

Copy link
Copy Markdown
Contributor Author

Updated the PR on top of the current main and took into account the review comments.

I changed the validation logic so that the function number and functional levels are displayed separately when they are used together with an explicit call graph. I also removed the parameter validation from the static methods of CallGraphLayout.

CI also detected one existing test that was affected by this change. I updated calls-randomize-ra-for-func.yaml to be standalone, instead of reusing calls-randomize-ra.yaml, because this general configuration defines the function number/function levels, while the test itself provides an explicit call graph.

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.

Snippy ignores the function-number option when call-graph is specified

2 participants