[snippy] Reject function-number/function-layers with explicit call-graph#420
[snippy] Reject function-number/function-layers with explicit call-graph#420MaxGud10 wants to merge 1 commit into
Conversation
| # CHECK: error: Options "function-number" cannot be used together with "call-graph" | ||
|
|
||
| options: | ||
| mtriple: riscv64-unknown-elf |
There was a problem hiding this comment.
| mtriple: riscv64-unknown-elf | |
| mtriple: riscv64 |
| 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 |
There was a problem hiding this comment.
reuse sections from Inputs/ directory if possible
| static bool isFunctionLayersSpecified(); | ||
| static bool isFunctionNumberSpecified(); |
| 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(); | ||
| } |
There was a problem hiding this comment.
I understand what you want to achieve here but let's just report them separately.
With 2 separate return's
4e50804 to
21742b2
Compare
21742b2 to
2be1ac0
Compare
|
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. |
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.