AIESW-27905: special handling for hint_bitmap 0 preemption point#292
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds special handling in the AIE4 ASM preprocessor for PREEMPT points whose hint_bitmap is all-zero. Since such hintmaps require zero scratchpad, the parser now injects minimal dummy save (NOP) / restore (LOAD_LAST_PDI) jobs instead of expanding the full BD / stream-switch save/restore templates, providing valid label targets for the PREEMPT opcode without unnecessary work. Two new ctrlcode tests (1-column and 3-column) exercise the mixed zero/non-zero hintmap scenario, and the existing fused_hw_package_subgraph_0 golden md5 is updated to reflect the new behavior. The header generator is also tightened to emit a const global map.
Changes:
- Inject minimal dummy save/restore jobs when
grp.size == 0(zero hint_bitmap) ininject_hintmap_save_restore. - Add 1col_preempt_0hint and 3col_preempt_0hint test fixtures (asm/config/CMake/gold.md5) and register them in the parent CMakeLists.
- Mark the generated
aie4_save_restore_mapand accessor return type asconstingenerate_aie4_header.py; update the existing fused_hw_package_subgraph_0 gold.md5.
Reviewed changes
Copilot reviewed 17 out of 18 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/cpp/preprocessor/asm/asm_parser.cpp | Adds zero-hintmap fast path emitting dummy save/restore jobs with proper labels. |
| specification/aie4/preemption_asm/generate_aie4_header.py | Makes the generated save/restore map and accessor return type const. |
| test/aie4-ctrlcode/CMakeLists.txt | Registers the two new preempt_0hint test directories. |
| test/aie4-ctrlcode/1col_preempt_0hint/{CMakeLists.txt,config.json,test.asm,pdi.asm,gold.md5} | New 1-column test exercising mixed zero/non-zero hintmaps. |
| test/aie4-ctrlcode/3col_preempt_0hint/{CMakeLists.txt,config.json,test.asm,uc0.asm,uc2.asm,uc4.asm,pdi.asm,gold.md5} | New 3-column test exercising mixed zero/non-zero hintmaps across groups. |
| test/aie4-ctrlcode/compile_time/fused_hw_package_subgraph_0/gold.md5 | Updated golden hash reflecting new zero-hintmap codegen behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -9,7 +9,7 @@ | |||
| #include <cstdint> | |||
|
|
|||
| // C++17 inline variable - single instance across all translation units | |||
| inline std::map<uint32_t, std::pair<std::vector<std::string>, std::vector<std::string>>> aie4_save_restore_shimbd_map = { | |||
| inline const std::map<uint32_t, std::pair<std::vector<std::string>, std::vector<std::string>>> aie4_save_restore_shimbd_map = { | |||
There was a problem hiding this comment.
warning: expected unqualified-id [clang-diagnostic-error]
inline const std::map<uint32_t, std::pair<std::vector<std::string>, std::vector<std::string>>> aie4_save_restore_shimbd_map = {
^| @@ -9,7 +9,7 @@ | |||
| #include <cstdint> | |||
|
|
|||
| // C++17 inline variable - single instance across all translation units | |||
| inline std::map<uint32_t, std::pair<std::vector<std::string>, std::vector<std::string>>> aie4_save_restore_shimbd_map = { | |||
| inline const std::map<uint32_t, std::pair<std::vector<std::string>, std::vector<std::string>>> aie4_save_restore_shimbd_map = { | |||
There was a problem hiding this comment.
warning: no member named 'string' in namespace 'std' [clang-diagnostic-error]
inline const std::map<uint32_t, std::pair<std::vector<std::string>, std::vector<std::string>>> aie4_save_restore_shimbd_map = {
^| @@ -21,13 +21,13 @@ inline std::map<uint32_t, std::pair<std::vector<std::string>, std::vector<std::s | |||
| {14, {{"DMAWRITE_data_125", "DMAWRITE_data_126"}, {"DMAWRITE_data_0", "DMAWRITE_data_1", "DMAWRITE_data_2", "DMAWRITE_data_3"}}} | |||
| }; | |||
|
|
|||
| inline std::map<uint32_t, std::pair<std::vector<std::string>, std::vector<std::string>>>& | |||
| inline const std::map<uint32_t, std::pair<std::vector<std::string>, std::vector<std::string>>>& | |||
There was a problem hiding this comment.
warning: expected unqualified-id [clang-diagnostic-error]
inline const std::map<uint32_t, std::pair<std::vector<std::string>, std::vector<std::string>>>&
^| @@ -21,13 +21,13 @@ inline std::map<uint32_t, std::pair<std::vector<std::string>, std::vector<std::s | |||
| {14, {{"DMAWRITE_data_125", "DMAWRITE_data_126"}, {"DMAWRITE_data_0", "DMAWRITE_data_1", "DMAWRITE_data_2", "DMAWRITE_data_3"}}} | |||
| }; | |||
|
|
|||
| inline std::map<uint32_t, std::pair<std::vector<std::string>, std::vector<std::string>>>& | |||
| inline const std::map<uint32_t, std::pair<std::vector<std::string>, std::vector<std::string>>>& | |||
There was a problem hiding this comment.
warning: no member named 'string' in namespace 'std' [clang-diagnostic-error]
inline const std::map<uint32_t, std::pair<std::vector<std::string>, std::vector<std::string>>>&
^| get_aie4_save_restore_shimbd() | ||
| { | ||
| return aie4_save_restore_shimbd_map; | ||
| } | ||
|
|
||
| inline std::map<uint32_t, std::pair<std::vector<std::string>, std::vector<std::string>>> aie4_save_restore_membd_map = { | ||
| inline const std::map<uint32_t, std::pair<std::vector<std::string>, std::vector<std::string>>> aie4_save_restore_membd_map = { |
There was a problem hiding this comment.
warning: expected unqualified-id [clang-diagnostic-error]
inline const std::map<uint32_t, std::pair<std::vector<std::string>, std::vector<std::string>>> aie4_save_restore_membd_map = {
^| get_aie4_save_restore_shimbd() | ||
| { | ||
| return aie4_save_restore_shimbd_map; | ||
| } | ||
|
|
||
| inline std::map<uint32_t, std::pair<std::vector<std::string>, std::vector<std::string>>> aie4_save_restore_membd_map = { | ||
| inline const std::map<uint32_t, std::pair<std::vector<std::string>, std::vector<std::string>>> aie4_save_restore_membd_map = { |
There was a problem hiding this comment.
warning: no member named 'string' in namespace 'std' [clang-diagnostic-error]
inline const std::map<uint32_t, std::pair<std::vector<std::string>, std::vector<std::string>>> aie4_save_restore_membd_map = {
^| @@ -39,13 +39,13 @@ inline std::map<uint32_t, std::pair<std::vector<std::string>, std::vector<std::s | |||
| {14, {{"DMAWRITE_data_127", "DMAWRITE_data_128"}, {"DMAWRITE_data_4", "DMAWRITE_data_5", "DMAWRITE_data_6", "DMAWRITE_data_7"}}} | |||
| }; | |||
|
|
|||
| inline std::map<uint32_t, std::pair<std::vector<std::string>, std::vector<std::string>>>& | |||
| inline const std::map<uint32_t, std::pair<std::vector<std::string>, std::vector<std::string>>>& | |||
There was a problem hiding this comment.
warning: expected unqualified-id [clang-diagnostic-error]
inline const std::map<uint32_t, std::pair<std::vector<std::string>, std::vector<std::string>>>&
^| @@ -39,13 +39,13 @@ inline std::map<uint32_t, std::pair<std::vector<std::string>, std::vector<std::s | |||
| {14, {{"DMAWRITE_data_127", "DMAWRITE_data_128"}, {"DMAWRITE_data_4", "DMAWRITE_data_5", "DMAWRITE_data_6", "DMAWRITE_data_7"}}} | |||
| }; | |||
|
|
|||
| inline std::map<uint32_t, std::pair<std::vector<std::string>, std::vector<std::string>>>& | |||
| inline const std::map<uint32_t, std::pair<std::vector<std::string>, std::vector<std::string>>>& | |||
There was a problem hiding this comment.
warning: no member named 'string' in namespace 'std' [clang-diagnostic-error]
inline const std::map<uint32_t, std::pair<std::vector<std::string>, std::vector<std::string>>>&
^| get_aie4_save_restore_membd() | ||
| { | ||
| return aie4_save_restore_membd_map; | ||
| } | ||
|
|
||
| inline std::map<uint32_t, std::pair<std::vector<uint8_t>, std::vector<uint8_t>>> aie4_save_restore_map = { | ||
| inline const std::map<uint32_t, std::pair<std::vector<uint8_t>, std::vector<uint8_t>>> aie4_save_restore_map = { |
There was a problem hiding this comment.
warning: initialization of 'aie4_save_restore_map' with static storage duration may throw an exception that cannot be caught [cert-err58-cpp]
inline const std::map<uint32_t, std::pair<std::vector<uint8_t>, std::vector<uint8_t>>> aie4_save_restore_map = {
^Additional context
/usr/include/c++/13/bits/stl_map.h:239: possibly throwing constructor declared here
map(initializer_list<value_type> __l,
^Signed-off-by: Himanshu Choudhary <Himanshu.Choudhary@amd.com>
Signed-off-by: Himanshu Choudhary <Himanshu.Choudhary@amd.com>
c17fdd0 to
86d01f6
Compare
|
|
||
| // C++17 inline variable - single instance across all translation units | ||
| inline std::map<uint32_t, std::pair<std::vector<std::string>, std::vector<std::string>>> aie4_save_restore_shimbd_map = { | ||
| inline const std::map<uint32_t, std::pair<std::vector<std::string>, std::vector<std::string>>> aie4_save_restore_shimbd_map = { |
There was a problem hiding this comment.
warning: initialization of 'aie4_save_restore_shimbd_map' with static storage duration may throw an exception that cannot be caught [cert-err58-cpp]
inline const std::map<uint32_t, std::pair<std::vector<std::string>, std::vector<std::string>>> aie4_save_restore_shimbd_map = {
^Additional context
/usr/include/c++/13/bits/stl_map.h:239: possibly throwing constructor declared here
map(initializer_list<value_type> __l,
^| get_aie4_save_restore_shimbd() | ||
| { | ||
| return aie4_save_restore_shimbd_map; | ||
| } | ||
|
|
||
| inline std::map<uint32_t, std::pair<std::vector<std::string>, std::vector<std::string>>> aie4_save_restore_membd_map = { | ||
| inline const std::map<uint32_t, std::pair<std::vector<std::string>, std::vector<std::string>>> aie4_save_restore_membd_map = { |
There was a problem hiding this comment.
warning: initialization of 'aie4_save_restore_membd_map' with static storage duration may throw an exception that cannot be caught [cert-err58-cpp]
inline const std::map<uint32_t, std::pair<std::vector<std::string>, std::vector<std::string>>> aie4_save_restore_membd_map = {
^Additional context
/usr/include/c++/13/bits/stl_map.h:239: possibly throwing constructor declared here
map(initializer_list<value_type> __l,
^
Problem solved by the commit
AIE4 preemption: emit dummy save/restore jobs when hint_bitmap is zero
Problem
When a PREEMPT opcode references a hintmap with the scratchpad size zero — implementation still injected the full save/restore job template for the zero-size case, which included:
This was wasteful and incorrect: even though the BDs carried zero length, the surrounding stream-switch and DMA infrastructure was still emitted.
Dummy save and restore controlcode added:
save:
restore:
Tests
Two new test cases are added under test/aie4-ctrlcode/:
Bug / issue (if any) fixed, which PR introduced the bug, how it was discovered
How problem was solved, alternative solutions (if any) and why they were rejected
Risks (if any) associated the changes in the commit
What has been tested and how, request additional testing if necessary
Documentation impact (if any)