[aiecc] Reject unknown command-line arguments before -- (#2989)#3066
Open
hunhoffe wants to merge 5 commits into
Open
[aiecc] Reject unknown command-line arguments before -- (#2989)#3066hunhoffe wants to merge 5 commits into
-- (#2989)#3066hunhoffe wants to merge 5 commits into
Conversation
aiecc previously declared a `cl::Sink` list that absorbed every unrecognized command-line argument and forwarded them to host compilation. This silently swallowed typos like `--aie-genrate-npu-insts` and made it impossible to tell aiecc-targeted flags apart from host-compiler passthrough. Pre-split argv on the first standalone `--`. Everything before is parsed by `cl::ParseCommandLineOptions` with no sink, so unrecognized flags become hard errors. Everything after is captured verbatim into a new `hostExtraArgs` vector and emitted in user-specified order during host compilation. Host source files appearing among the post-`--` args are also picked up by `getHostSourceFiles()`. Adds `test/aiecc/unknown_args_rejected.mlir` covering `--garbage`, the `--aie-genrate-npu-insts` typo from the issue, and the post-`--` passthrough path. Updates `cpp_host_compile.mlir`'s HOSTFULL run line to demonstrate the `--` separator form. Existing aiecc-recognized options (-I, -L, -l, -o, --sysroot, etc.) keep working without `--` since they bind to their own cl::opt/cl::list declarations. Co-Authored-By: Claude Opus 4 (1M context) <noreply@anthropic.com>
Contributor
Coverage ReportCreated: 2026-05-13 18:40Click here for information about interpreting this report.
Generated by llvm-cov -- llvm version 18.1.3 |
These three Makefiles passed --aie-generate-npu alongside the real --aie-generate-npu-insts flag; the former was silently ignored before this PR's strict argument check and is now a hard error. Co-Authored-By: Claude Opus 4 (1M context) <noreply@anthropic.com>
hunhoffe
requested review from
abisca,
andrej,
denolf,
fifield,
jackl-xilinx,
jgmelber,
pvasireddy-amd and
stephenneuendorffer
as code owners
jgmelber
approved these changes
May 14, 2026
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.
Summary
Closes #2989.
aieccpreviously declared acl::Sinklist that quietly absorbed every unrecognized command-line argument and forwarded them to host compilation. That meant typos like--aie-genrate-npu-insts(missinge) silently turned into clang flags, and the user got no signal that their option was unknown.This change drops the sink and adds an explicit
--separator:--must be a recognized aiecc option or a positional input file. Unknown flags are a hard error fromcl::ParseCommandLineOptions.--is captured verbatim intohostExtraArgsand passed straight to the host compiler in user-specified order. Host source files mixed in are picked up bygetHostSourceFiles()as well.Behavior for existing recognized options
-I/foo,-L/bar,-lbaz,-o test.elf,--sysroot=...,--host-target=...are all already registered as aiecc options (cl::Prefix lists cl::opt). They keep working without--. The vast majority of in-tree tests need no changes.The only flags that genuinely have to move past
--are ones aiecc never knew about — typically-Wl,...linker flags,-Ddefines, etc.Tests
test/aiecc/unknown_args_rejected.mlircovers--garbage, the--aie-genrate-npu-inststypo from the issue, and the post---passthrough path.test/aiecc/cpp_host_compile.mlirHOSTFULL run line updated to demonstrate the--form.Required follow-up: Xilinx/mlir-air
The 26 lit files under
mlir-air/test/airhost/*/run.litinvoke aiecc through the%airhost_libs%substitution, which expands to-Wl,--whole-archive -lairhost -Wl,-R... -Wl,-rpath,... -Wl,--no-whole-archive -lpthread -lstdc++ -lsysfs -ldl -lrt -lelf. The-Wl,...flags were absorbed by the legacy sink and are not recognized aiecc options. After this lands, those tests will fail until they're updated to insert--after the input MLIR —aiecc.py %S/aie.mlir -- -I... %airhost_libs% %S/test.cpp -o %T/test.elf. A companion PR will follow.amd/iron is unaffected — its callers only pass recognized aiecc flags.
Test plan