Fix two missing parents and check the DAG against feature sets - #162
Open
haampie wants to merge 3 commits into
Open
Fix two missing parents and check the DAG against feature sets#162haampie wants to merge 3 commits into
haampie wants to merge 3 commits into
Conversation
AmpereOneA has every feature of AmpereOne plus sm3/sm4, but both listed the same parents (neoverse_n1, armv8.6a), so archspec concluded an ampere1 binary cannot run on an ampere1a machine. List ampere1 as a parent, like other same-vendor successors (zen2 from zen, m4 from m3). Signed-off-by: Harmen Stoppels <harmenstoppels@gmail.com>
Cannon Lake supports AVX-512 F/CD/VL/BW/DQ, so its feature list already covers all of x86_64_v4, but its only parent was skylake and archspec concluded an x86_64_v4 binary cannot run on a cannonlake machine. List x86_64_v4 as a parent, like skylake_avx512 does. Signed-off-by: Harmen Stoppels <harmenstoppels@gmail.com>
Feature inclusion defines a partial order of its own, and the explicit from DAG must agree with it: a descendant must have all features of its ancestors, and within a family a strict feature superset must descend from the subset when the two share a vendor or the subset is a generic level. Cross-vendor lineage is intentionally not required, since portability between vendors is expressed through the generic levels. This check would have caught the ampere1a and cannonlake omissions fixed in the previous commits. Signed-off-by: Harmen Stoppels <harmenstoppels@gmail.com>
alalazo
force-pushed
the
fix/feature-dag-consistency
branch
from
August 19, 2026 12:00
26f125d to
377bf74
Compare
alalazo
reviewed
Aug 19, 2026
Member
There was a problem hiding this comment.
Can we split the new check from the fixes? I think the fixes are correct, modulo removing a now redundant edge.
The script probably needs some more discussion:
- It "correctly" flagged
neoverse_n2 -> neoverse_v3(in the sense that code tuned for N2 can actually run on V3) - It didn't flag
neoverse_n2 <-> neoverse_v2
Iirc we decided to leave the nx and the vx branches separate from each other on purpose. Modeling things correctly would remove acyclicity of the uarch graph, which I think deserves its own PR to discuss.
| @@ -0,0 +1,96 @@ | |||
| #!/usr/bin/env python3 | |||
| # Copyright 2019-2020 Lawrence Livermore National Security, LLC and other | |||
| "from": [ | ||
| "neoverse_n1", | ||
| "ampere1", | ||
| "armv8.6a" |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is Claude fixing two issues I've found. Let me know if you're interested in the test.
Each microarchitecture names a feature set, so feature inclusion defines a partial order of its own. Comparing it against the explicit
fromDAG (withfeature_aliasesapplied) shows the two agree everywhere except for deliberate design choices — cross-vendor portability goes through the generic levels only — and two omissions:ampere1a: has every feature ofampere1plussm3/sm4, but both chips listed the same parents (neoverse_n1,armv8.6a). So archspec concluded anampere1binary cannot run on an AmpereOneA machine. Nowampere1adescends fromampere1, like other same-vendor successors (zen2fromzen,m4fromm3).cannonlake: its feature list fully coversx86_64_v4(AVX-512 F/CD/VL/BW/DQ), but its only parent wasskylake, sox86_64-v4binaries were considered incompatible with Cannon Lake machines. Now it also descends fromx86_64_v4, likeskylake_avx512does.The last commit adds
tests/check_dag_consistency.pyto the validation workflow. It checks both directions: a descendant must have all features of its ancestors (up tofeature_aliases), and within a family a strict feature superset must descend from the subset when the two share a vendor or the subset is a generic level. It would have caught both omissions above, and passes after them.The full archspec test suite (638 tests) passes against the updated JSON.
Found while comparing the feature-derived partial order with the DAG for spack/spack#52856.