fix: aiecc --aiesim crashes due to missing define and wrong IO backend#2956
fix: aiecc --aiesim crashes due to missing define and wrong IO backend#2956FIM43-Redeye wants to merge 3 commits intoXilinx:mainfrom
Conversation
The static library xaienginecdo_static (linked into aiecc) is built with __AIECDO__ and __AIEDEBUG__ but not __AIESIM__. This causes xaie_sim.c to compile with error stubs instead of real ess_* implementations, so `aiecc --aiesim` crashes at XAie_SimIO_Init() with "Driver is not compiled with simulation backend". The shared library (libxaienginecdo.so) correctly includes __AIESIM__. Also adds weak ess_* stubs (ess_Write32, ess_Read32, ess_WriteCmd, ess_NpiWrite32, ess_NpiRead32) for static linking. These symbols are provided at runtime by the aiesimulator SystemC process via dlopen; the weak stubs satisfy the linker and abort if called outside the simulator. Fixes Xilinx#2955.
FIM43-Redeye
requested review from
jackl-xilinx and
stephenneuendorffer
as code owners
There was a problem hiding this comment.
Pull request overview
This PR fixes aiecc --aiesim crashes during CDO generation by ensuring the statically-linked xaienginecdo_static is built with simulation support and can resolve ess_* symbols at link time (while still allowing the simulator’s strong symbols to override at runtime).
Changes:
- Add
__AIESIM__toxaienginecdo_staticcompile definitions so the simulation backend is compiled in. - Add
runtime_lib/ess_stubs.ccontaining weakess_*stub definitions to satisfy static link requirements.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| runtime_lib/ess_stubs.c | Adds weak ess_* symbol stubs for static linking in aiesim flows. |
| runtime_lib/CMakeLists.txt | Links the stub source into xaienginecdo_static and adds the __AIESIM__ compile definition. |
💡 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.
runtime_lib/CMakeLists.txt
Outdated
|
|
||
| target_link_libraries(xaienginecdo_static PRIVATE cdo_driver_mlir_aie) | ||
| target_compile_definitions(xaienginecdo_static PRIVATE -D__AIECDO__ -D__AIEDEBUG__) | ||
| target_compile_definitions(xaienginecdo_static PRIVATE -D__AIECDO__ -D__AIEDEBUG__ -D__AIESIM__) |
There was a problem hiding this comment.
target_compile_definitions() should be given bare preprocessor symbols (e.g. __AIECDO__) rather than -D.... Passing -D__AIECDO__ causes CMake to emit an extra -D prefix (typically -D-D__AIECDO__), which can break compilation and is inconsistent with other CMake files in this repo (e.g. runtime_lib/xaiengine/lib/CMakeLists.txt).
| target_compile_definitions(xaienginecdo_static PRIVATE -D__AIECDO__ -D__AIEDEBUG__ -D__AIESIM__) | |
| target_compile_definitions(xaienginecdo_static PRIVATE __AIECDO__ __AIEDEBUG__ __AIESIM__) |
| __attribute__((weak)) uint ess_Read32(uint64_t Addr) { | ||
| (void)Addr; | ||
| fprintf(stderr, "FATAL: ess_Read32 called outside aiesimulator\n"); | ||
| abort(); | ||
| } |
There was a problem hiding this comment.
ess_Read32 is declared to return uint but has no return statement after abort(). This will fail builds that use -Werror=return-type (enabled in the top-level CMake). Mark the function as noreturn (and/or add a return 0;/__builtin_unreachable() after abort()) to satisfy the compiler.
| __attribute__((weak)) uint ess_NpiRead32(uint64_t Addr) { | ||
| (void)Addr; | ||
| fprintf(stderr, "FATAL: ess_NpiRead32 called outside aiesimulator\n"); | ||
| abort(); |
There was a problem hiding this comment.
ess_NpiRead32 is a non-void function but does not return a value after calling abort(). With -Werror=return-type this becomes a hard compile error; make the function noreturn and/or add an explicit dummy return/__builtin_unreachable() after abort().
| abort(); | |
| abort(); | |
| __builtin_unreachable(); |
When --aiesim is passed, aiecc was forwarding aieSim=true to AIETranslateToCDODirect, which selected XAIE_IO_BACKEND_SIM. The SIM backend calls ess_* functions that are only available inside a running aiesimulator process, so CDO generation crashed. CDO file generation is a compile-time step that writes binary files to disk -- it should always use XAIE_IO_BACKEND_CDO regardless of whether aiesim artifacts are also being generated.
FIM43-Redeye
changed the title
- Drop -D prefix from target_compile_definitions (CMake adds it) - Add __builtin_unreachable() after abort() in non-void ess stubs to satisfy -Werror=return-type
Summary
-D__AIESIM__toxaienginecdo_staticcompile definitions so the SIM IO backend compiles iness_*stubs for static linking (symbols provided at runtime by aiesimulator)XAIE_IO_BACKEND_CDO, notXAIE_IO_BACKEND_SIMProblem
aiecc --aiesimcrashes at CDO generation with two sequential bugs:Bug 1:
xaienginecdo_staticis built with__AIECDO__and__AIEDEBUG__but not__AIESIM__, soxaie_sim.ccompiles with error stubs instead of real SIM IO functions:Bug 2 (exposed after fixing bug 1):
generateCdoArtifacts()passesaieSim=truetoAIETranslateToCDODirect(), which selectsXAIE_IO_BACKEND_SIM. The SIM backend callsess_*functions that are only available inside a running aiesimulator process. CDO file generation is a compile-time step that writes binary files to disk -- it should always useXAIE_IO_BACKEND_CDO.Fix
-D__AIESIM__to the static lib's compile definitions inruntime_lib/CMakeLists.txtess_stubs.cwith weak definitions ofess_Write32,ess_Read32,ess_WriteCmd,ess_NpiWrite32,ess_NpiRead32-- these are forward-declared inxaie_sim.cand provided at runtime by the aiesimulator SystemC process via dlopenaieSimparameter ingenerateCdoArtifacts()fromaiesimtofalseTesting
Verified locally end-to-end:
aiecc --aiesim --xchesscc --xbridgecompiles and generates sim artifacts (no crash)aiesimulator --pkg-dir=<generated>/simruns the simulation to completion (PASS)--no-aiesim) is unaffectedNote
generateAiesim()inaiecc_aiesim.cppusesfindProgramByName("clang++")to buildps.so. In environments without a full Vitis install,aietools/bin/clang++(a wrapper expecting Vitis-bundled Peano) may shadow the system compiler. This is a separate issue.Fixes #2955.