Conversation
mlir-aie now provides prefix_symbols_in_object() and resolves llvm-objcopy, llvm-nm and llvm-ar through aie.utils.config, so the copies here can go. _prefix_symbols() built the nm -> rename-map -> objcopy pipeline by hand and carried a separate implementation per platform: a Windows branch that shelled out to an inline python -c script, and a POSIX branch that ran nm and awk under sh. Both are replaced by one PythonCallbackCompilationCommand, which also drops the .symbol_map and .symbol_map.syms files this left beside every prefixed object. The upstream parser reads the symbol name as the last field rather than awk's positional $3. _find_tool/_find_working_tool/_tool_runs searched peano_dir, mlir_aie_dir and PATH, because upstream's resolvers looked only in the mlir-aie bin directory and would not find llvm-nm or llvm-ar, which ship with peano instead. Upstream searches both trees as of the config change, so this is now duplicated logic rather than a workaround, and the execute-it-first guard that _find_working_tool added lives there too. peano_dir is no longer threaded into the rules, and ArchiveCompilationRule needs no constructor at all. Verified by compiling a real kernel through KernelCompilationRule with prefix_symbols set and reading the object back with llvm-nm: symbols come out as op0_add_one and op0_helper_fn, from two commands instead of three. The fusion tests that exercise this in anger need XRT, which this host lacks; they fail identically with and without this change. Co-Authored-By: Claude <noreply@anthropic.com>
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
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.
Important
Blocked on a wheel containing both Xilinx/mlir-aie#3584 and Xilinx/mlir-aie#. Newest nightly is
1.4.4.dev18+gf5196d8— the commit before #3584 — andrequirements.txtpins1.4.4.dev4. That bump belongs here; CI fails until the wheel exists. Draft until then.mlir-aie now provides
prefix_symbols_in_object()and resolvesllvm-objcopy,llvm-nmandllvm-arviaaie.utils.config, so the copies here can go. −105 lines.Added
Changed
KernelCompilationRuleemits onePythonCallbackCompilationCommandcalling upstreamprefix_symbols_in_object()instead of hand-building thenm→ rename-map →objcopypipeline. Two commands instead of three, and no.symbol_map/.symbol_map.symslitter beside every prefixed object._rename_symbols()andArchiveCompilationRuletake tool paths fromaie.utils.config.objcopy_path()/ar_path().AIEContext.compilation_rulesno longer threadspeano_dirthrough.Removed
_prefix_symbols()and its two platform implementations — a Windows branch shelling out to an inlinepython -c, and a POSIX branch runningnmandawkundersh. Upstream also reads the symbol name as the last field rather thanawk's positional$3._find_tool()/_find_working_tool()/_tool_runs(). These existed because upstream looked only in the mlir-aiebin/dir and couldn't findllvm-nm/llvm-ar, which ship with peano. Upstream now searches both trees and has the execute-it-first guard, so this is duplication, not a workaround.ArchiveCompilationRule.__init__, which no longer needs either path.PR Merge Checklist
develcommit and pointing todevel.