Skip to content

update codegen and codegen test for diagonal direction#288

Merged
tancheng merged 3 commits intomainfrom
testbench
Mar 18, 2026
Merged

update codegen and codegen test for diagonal direction#288
tancheng merged 3 commits intomainfrom
testbench

Conversation

@n0thingNoob
Copy link
Copy Markdown
Collaborator

King Mesh Diagonal Codegen Update

Background

GenerateCodePass previously handled only 4-neighbor directions:

  • EAST
  • WEST
  • NORTH
  • SOUTH

When running on a king_mesh topology (8-neighbor connectivity), diagonal links could be resolved as LOCAL or mapped incorrectly in generated outputs. This caused mismatches between mapping/link intent and emitted instruction operands in:

  • tmp-generated-instructions.yaml
  • tmp-generated-instructions.asm

What Was Changed

1) Direction support in codegen

File updated:

  • dataflow/lib/NeuraDialect/Transforms/GenerateCodePass.cpp

Changes:

  • Extended Topology::getDirBetween(...) to support diagonal directions:
    • (1, 1) -> NORTHEAST
    • (-1, 1) -> NORTHWEST
    • (1, -1) -> SOUTHEAST
    • (-1, -1) -> SOUTHWEST
  • Extended Topology::invertDir(...) accordingly:
    • NORTHEAST <-> SOUTHWEST
    • NORTHWEST <-> SOUTHEAST
  • Kept LOCAL semantics unchanged for true local/no-link scenarios.

2) Dedicated king-mesh architecture spec for tests

New file:

  • dataflow/test/arch_spec/architecture_king_mesh.yaml

This file is copied from the regular test architecture and sets:

  • multi_cgra_defaults.base_topology: "king_mesh"
  • per_cgra_defaults.base_topology: "king_mesh"

3) New codegen test

New file:

  • dataflow/test/code_gen/test_code_generate_king_mesh.mlir

Test behavior:

  • Runs the full codegen pipeline using architecture_king_mesh.yaml
  • Checks full mapping IR expectation (MAPPING)
  • Checks partial YAML/ASM expectation focused on diagonal path evidence
  • Uses --implicit-check-not=LOCAL for YAML/ASM to catch accidental fallback

Verification Summary

The new king-mesh codegen test passes:

  • llvm-lit -v dataflow/test/code_gen/test_code_generate_king_mesh.mlir

Manual inspection of generated artifacts confirms diagonal operands are emitted as expected (e.g., NORTHEAST, NORTHWEST, SOUTHEAST, SOUTHWEST) instead of incorrect LOCAL.

Copilot AI review requested due to automatic review settings March 18, 2026 03:32
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates Neura’s codegen to correctly resolve diagonal link directions when targeting a king-mesh (8-neighbor) topology, and adds a dedicated lit test + architecture spec to validate diagonal operands in generated YAML/ASM outputs.

Changes:

  • Extend Topology::getDirBetween / invertDir to handle NORTHEAST/NORTHWEST/SOUTHEAST/SOUTHWEST.
  • Add a king-mesh test architecture YAML for lit-based codegen testing.
  • Add a new codegen lit test that checks diagonal direction evidence in emitted YAML/ASM.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File Description
lib/NeuraDialect/Transforms/GenerateCodePass.cpp Adds diagonal direction resolution/inversion for king-mesh link operands.
test/arch_spec/architecture_king_mesh.yaml Introduces a king-mesh topology architecture spec for tests.
test/code_gen/test_code_generate_king_mesh.mlir Adds a new lit test ensuring diagonals are emitted (and not incorrectly treated as LOCAL).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@tancheng
Copy link
Copy Markdown
Contributor

BTW, why do we need king-mesh? I thought our evaluations are only running on mesh.

@n0thingNoob
Copy link
Copy Markdown
Collaborator Author

BTW, why do we need king-mesh? I thought our evaluations are only running on mesh.

I am currently working on a case study and found that mapping is quite difficult with a mesh-only topology. @ShangkunLi mentioned that it works on a king mesh, but I noticed that the generated ASM and YAML were missing the proper directions for diagonal connections.

@tancheng
Copy link
Copy Markdown
Contributor

BTW, why do we need king-mesh? I thought our evaluations are only running on mesh.

I am currently working on a case study and found that mapping is quite difficult with a mesh-only topology. @ShangkunLi mentioned that it works on a king mesh, but I noticed that the generated ASM and YAML were missing the proper directions for diagonal connections.

For which kernel? I thought we can now map most kernels you are working on.

@ShangkunLi
Copy link
Copy Markdown
Collaborator

BTW, why do we need king-mesh? I thought our evaluations are only running on mesh.

I am currently working on a case study and found that mapping is quite difficult with a mesh-only topology. @ShangkunLi mentioned that it works on a king mesh, but I noticed that the generated ASM and YAML were missing the proper directions for diagonal connections.

For which kernel? I thought we can now map most kernels you are working on.

@n0thingNoob introduces a synthetic function composed of two gemv kernels and a relu kernel for a case study, which can only be mapped on a 4x4 king-mesh/an 8x8 mesh architecture.

Considering we also need to support codegen for king-mesh architectures, we decided to make this pr.

@tancheng
Copy link
Copy Markdown
Contributor

The 2 gemm + 1 relu forming a single large DFG (how many nodes) for mapping? Or viewed as 3 tasks, temporarily scheduled?

@ShangkunLi
Copy link
Copy Markdown
Collaborator

The 2 gemm + 1 relu forming a single large DFG (how many nodes) for mapping? Or viewed as 3 tasks, temporarily scheduled?

A single large DFG for mapping, though with bad performance (i.e., compiled II = 28).

@tancheng tancheng merged commit 922b1bb into main Mar 18, 2026
1 check passed
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.

4 participants