Scope kernel objects to build_dir/<arch>, fixing aie2/aie2p reuse - #166
Open
atassis wants to merge 1 commit into
Open
Scope kernel objects to build_dir/<arch>, fixing aie2/aie2p reuse#166atassis wants to merge 1 commit into
atassis wants to merge 1 commit into
Conversation
Kernel objects/archives are named without an arch component (e.g. "mul.o"),
but KernelCompilationRule.compile() passes a different --target and
runtime-lib -I per arch, and aie_kernels/generic/ sources compile to
different machine code per arch from the same input file. mtime-only
is_available_in_filesystem() can't see any of that, so a shared build_dir
lets one arch's build silently reuse the other's object; this is what
produced 75 bogus ElementwiseMul failures when an npu1 run's mul.o was
picked up by the following npu2 run.
move_artifacts() now places KernelObjectArtifact/KernelArchiveArtifact
under build_dir/<get_kernel_dir()>; every other artifact keeps its current
path. Also fixes dequant's expand_aie2_{tile_size}.o, which hardcoded the
aie2 literal while compiling for the current target.
iron/tests/compilation/kernel_object_arch_isolation.py fails on both
counts before this change.
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.
Problem
KernelCompilationRule.compile()(iron/common/compilation/base.py) passes adifferent
--targetandaie_runtime_libinclude path per arch, but theoutput filename it writes to is
f"{kernel_name}.o"with no arch component,and
build_diris one directory for every target(
iron/common/context.py:29). For the 9aie_kernels/generic/kernels thesame source compiles to different machine code per arch (measured:
generic/mul.ccgives ELF flags0x2/1568 B under--target=aie2and0x3/1488 B underaie2p, different sha256).CompilationArtifact.is_available_in_filesystemonly compares mtimes and never records which arch an object was built for, so
a shared path lets one arch's build silently hand its object to the other.
This produced 75 bogus
ElementwiseMulfailures when an npu1 (aie2) run'smul.owas reused by the following npu2 (aie2p) run in the samebuild/;rms_norm's weighted path linksgeneric/mul.ccthe same way. 11 morekernels (
gelu,rms_norm,softmax, ...) exist as different sources underboth
aie_kernels/aie2/andaie_kernels/aie2p/and collide the same way forthe ordinary reason that a freshly-built object is newer than either source.
Separately,
dequantnames its objectf"expand_aie2_{tile_size}.o"(
design.py,op.py) while compiling for the current target: on an aie2pdevice it names an aie2p object
expand_aie2_....Fix
CompilationArtifactGraph.move_artifactsnow placesKernelObjectArtifact/KernelArchiveArtifactunderbuild_dir/<get_kernel_dir()>/; every otherartifact (MLIR, xclbin, insts.bin) keeps its current flat path, since those
already carry the device in their name via
AIEOperatorBase.name. I pickedthis over qualifying every object name with the arch: the object name is
independently spelled twice per operator (the
Kernel(...)link_with stringin
design.pyand theKernelObjectArtifactfilename inop.py, with noshared source of truth), across ~12 operators, so qualifying names touches
~24 call sites; the build_dir change is two lines and fixes every current and
future kernel object at once.
compile()'s singlebuild_dir.mkdir()movesto a per-artifact mkdir after
move_artifacts, since kernel objects can nowresolve one level deeper.
Also fixed
dequant'sexpand_aie2_literal to useget_kernel_dir(),independent of which shape this lands as.
Test / Evidence
iron/tests/compilation/kernel_object_arch_isolation.pybuildsElementwiseMul's real artifact graph forNPU1()andNPU2()(no Peanoinvocation, no device) and asserts the two kernel object paths never collide,
and that a real leftover aie2 object is not reported available to the
following aie2p build. Both assertions fail against
aab8083(verified byreverting
compilation/base.pyalone and rerunning) and pass after thischange.
Also exercised
axpy,softmax(which additionally bundleslut_based_ops.ointo a
KernelArchiveArtifacton aie2),rms_normin both its plain andweighted (
generic/mul.cc-linking) forms,mem_copy, anddequantacrossboth devices: every kernel object/archive lands under the expected
aie2/aie2psubdirectory with no collisions, anddequant's object name nolonger says
aie2on an aie2p build.Not run:
pytest. This repo's rootconftest.py(
pytest_collection_modifyitems) unconditionally callsaie_utils.DefaultNPURuntime.device()at collection time for every test inthe suite, which probes the attached NPU over XRT regardless of which
test is selected -- out of scope for a device-free change. Confirmed by calling
the two
test_*functions directly instead.