remove constraint for having hole in hintmap, it will transfer super set of hintmap by HimanshuChoudhary-Xilinx · Pull Request #350 · Xilinx/aiebu

HimanshuChoudhary-Xilinx · 2026-08-14T08:20:16Z

Problem solved by the commit

remove constraint for having hole in hintmap, it will transfer super set of hintmap

Previously, aiebu enforced strict contiguity of set bits in the hintmap bitmap — any gap between the first and last set bit (hole) caused an invalid_asm error.

Change:
Remove the hard error for non-contiguous hintmap bits. Instead, the scratchpad region is computed as the full span from first_set_bit to last_set_bit (inclusive), regardless of holes in between. Holes within the bitmap are absorbed into the scratchpad region. The DMA transfer will cover the superset of the hintmap, which is safe — it transfers slightly more than strictly necessary.
If there is a overlap due to hole, aiebu redistribute the 64kb chunks. preempt with no hintmap remain unchanged but others get redistributed.

A log_warn is emitted when a hole is detected so it remains visible.

Note: if there is a overlap due to aiebu it does distribution where as due to compiler it will error out.

Test:
Test 1 — aie4_3col_preempt_hintmap_hole_redistribute_asm + md5sum
Test 2 — aie4_3col_merged_control_asm + md5sum

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)

Copilot

Pull request overview

This PR updates the hintmap parsing logic in the ASM preprocessor so hintmaps with non-contiguous set bits (“holes”) no longer hard-fail, and instead compute a scratchpad region that spans from the first to the last set bit (transferring a superset that includes any holes).

Changes:

Removed the “contiguous bits only” constraint (previously threw invalid_asm) and replaced it with a warning log.
Changed scratchpad size calculation from set_bits * CHUNK_SIZE to span(first..last) * CHUNK_SIZE.
Expanded logging to report both span and set_bits for visibility into sparse/non-contiguous hintmaps.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

github-actions

clang-tidy made some suggestions

sonals · 2026-08-14T15:00:58Z

@HimanshuChoudhary-Xilinx , can you address Copilot and clangtidy feedback?

sonals · 2026-08-14T16:10:33Z

Also, please update the PREEMPT definition in isa-spec.yaml to include the hintmap operand.

larry9523

As we discussed, please resolve the overlap between uC after we create the new map. Though, ff there is a overlap from the original hintmap, that is an error.

…set of hintmap Signed-off-by: Himanshu Choudhary <Himanshu.Choudhary@amd.com>

Signed-off-by: Himanshu Choudhary <Himanshu.Choudhary@amd.com>

github-actions · 2026-08-19T10:37:04Z

clang-tidy review says "All clean, LGTM! 👍"

HimanshuChoudhary-Xilinx · 2026-08-19T10:47:24Z

Also, please update the PREEMPT definition in isa-spec.yaml to include the hintmap operand.

done

HimanshuChoudhary-Xilinx · 2026-08-19T10:47:32Z

@HimanshuChoudhary-Xilinx , can you address Copilot and clangtidy feedback?

done

HimanshuChoudhary-Xilinx · 2026-08-19T10:47:41Z

As we discussed, please resolve the overlap between uC after we create the new map. Though, ff there is a overlap from the original hintmap, that is an error.

done

github-actions · 2026-08-19T10:55:10Z

clang-tidy review says "All clean, LGTM! 👍"

Signed-off-by: Himanshu Choudhary <Himanshu.Choudhary@amd.com>

github-actions · 2026-08-19T11:27:03Z

clang-tidy review says "All clean, LGTM! 👍"

sonals · 2026-08-19T17:39:54Z

+needs to save/restore. Bit N (counting from the LSB of word 0) corresponds to the chunk at absolute scratchpad offset
+`N * 64KB`. The assembler uses this bitmap to automatically select the correct save/restore code variant and to
+enforce that no two controllers save the same 64KB chunk at the same preemption point (see note 4). When
+`hint_bitmap` is omitted the controller saves/restores the full default scratchpad.


Don't we also support 0 as hintmap to indicate no save restore is necessary?

larry9523

I am not quite following the algorithm. But I tried some random cases and it seems there are chunks not being transferred by any UC after the mandatory cut. Can you please confirm?

col0 PREEMPT 0, @save, @restore, @hintmap_0
hintmap_0:
.long 0x0000000f # bits 0- 31 set: 0-3
.long 0x00000000 # bits 32- 63 set: -
.long 0x00000000 # bits 64- 95 set: -
.long 0x0f000000 # bits 96-127 set: 120-123
.long 0x00000000 # bits 128-159 set: -
meaning: set chunks 0-3,120-123 (8 bits)
col2 PREEMPT 0, @save, @restore
(no hintmap argument -> save entire 3MB, chunks 48-95)
col4 PREEMPT 0, @save, @restore, @hintmap_4
hintmap_4:
.long 0x00000000 # bits 0- 31 set: -
.long 0x00000000 # bits 32- 63 set: -
.long 0x00000000 # bits 64- 95 set: -
.long 0x00000000 # bits 96-127 set: -
.long 0x00000000 # bits 128-159 set: -
meaning: all bits 0 -> save nothing

From the aiebu log, it seems the result after merge/cut is
col0 chunks 0-3 (4 chunks, 256KB)
col2 chunks 48-95 (48 chunks, 3072KB) [none: entire 3MB]
col4 nothing to transfer

NOT TRANSFERRED BY ANY UC: 120-123 (4 chunks)

sonals · 2026-08-19T20:30:18Z

+void
+asm_parser::
+validate_resolve_hintmap_overlap()
+{


Shouldn't this be a std::bitset<512> operation? Initialize three std::bitset<512> one for each column and then perform AND operation between two of them: C0 & C1, C1 & C2, C0 & C2

yes ... implemented in same way

sonals

See my comments about bit operations.

Signed-off-by: Himanshu Choudhary <Himanshu.Choudhary@amd.com>

github-actions

clang-tidy made some suggestions

github-actions · 2026-08-20T11:40:54Z

+//   scratchpad; bits 144-511 are reserved and will never be set.
+// ---------------------------------------------------------------------------
+static std::bitset<512>
+chunks_to_bitset(const std::vector<uint64_t>& chunks)


warning: 512 is a magic number; consider replacing it with a named constant [cppcoreguidelines-avoid-magic-numbers]

static std::bitset<512> ^

github-actions · 2026-08-20T11:40:55Z

+chunks_to_bitset(const std::vector<uint64_t>& chunks)
+{
+  std::bitset<512> bs;
+  for (const auto ch : chunks) {


warning: 512 is a magic number; consider replacing it with a named constant [cppcoreguidelines-avoid-magic-numbers]

^

github-actions · 2026-08-20T11:40:55Z

+  std::bitset<512> bs;
+  for (const auto ch : chunks) {
+    if (ch < 512)
+      bs.set(static_cast<std::size_t>(ch));


warning: 512 is a magic number; consider replacing it with a named constant [cppcoreguidelines-avoid-magic-numbers]

ks) { ^

github-actions · 2026-08-20T11:40:55Z

+
+    // Build a bitset marking every chunk inside the fixed region.
+    std::bitset<512> fixed_bs;
+    for (auto ch = first_chunk; ch < last_chunk && ch < 512; ++ch)


warning: 512 is a magic number; consider replacing it with a named constant [cppcoreguidelines-avoid-magic-numbers]

std::bitset<512> fixed_bs; ^

github-actions · 2026-08-20T11:40:55Z

+    // Build a bitset marking every chunk inside the fixed region.
+    std::bitset<512> fixed_bs;
+    for (auto ch = first_chunk; ch < last_chunk && ch < 512; ++ch)
+      fixed_bs.set(static_cast<std::size_t>(ch));


warning: 512 is a magic number; consider replacing it with a named constant [cppcoreguidelines-avoid-magic-numbers]

st_chunk && ch < 512; ++ch) ^

Signed-off-by: Himanshu Choudhary <Himanshu.Choudhary@amd.com>

HimanshuChoudhary-Xilinx · 2026-08-20T12:20:58Z

I am not quite following the algorithm. But I tried some random cases and it seems there are chunks not being transferred by any UC after the mandatory cut. Can you please confirm?

col0 PREEMPT 0, @save, @restore, @hintmap_0 hintmap_0: .long 0x0000000f # bits 0- 31 set: 0-3 .long 0x00000000 # bits 32- 63 set: - .long 0x00000000 # bits 64- 95 set: - .long 0x0f000000 # bits 96-127 set: 120-123 .long 0x00000000 # bits 128-159 set: - meaning: set chunks 0-3,120-123 (8 bits) col2 PREEMPT 0, @save, @restore (no hintmap argument -> save entire 3MB, chunks 48-95) col4 PREEMPT 0, @save, @restore, @hintmap_4 hintmap_4: .long 0x00000000 # bits 0- 31 set: - .long 0x00000000 # bits 32- 63 set: - .long 0x00000000 # bits 64- 95 set: - .long 0x00000000 # bits 96-127 set: - .long 0x00000000 # bits 128-159 set: - meaning: all bits 0 -> save nothing

From the aiebu log, it seems the result after merge/cut is col0 chunks 0-3 (4 chunks, 256KB) col2 chunks 48-95 (48 chunks, 3072KB) [none: entire 3MB] col4 nothing to transfer

NOT TRANSFERRED BY ANY UC: 120-123 (4 chunks)

Hi @larry9523
it was i issue where when one of the hintmap os zero but on restructuring .. it also get some bits
fixed the issue

Also i have added your test case and a script which will take elf as input and print all save/restore BD, so we get how much data is moved by each BD

larry9523 · 2026-08-20T17:53:15Z

+               << std::dec << " (set=" << seg.set_count
+               << " span=" << (seg.last - seg.first + 1) << " holes=" << holes << ")\n";
+
+    m_hintmap_region_override[{info.col, info.hintmap_key}] = {new_base, new_size};


The key of this map is (col, hintmap_key). If two preemption points share the same hintmap, will the second preemption point make its own decision based on the modified hintmap instead of the original one?

larry9523

Tried a two preemption points case. Something is still not quite right. Can you please confirm?

========================================================================
HINTMAP FED TO AIEBU

preemption point 0
col0 PREEMPT + @hintmap_0
.long 0x0000000f # bits 0- 31 set: 0-3
.long 0x00000000 # bits 32- 63 set: -
.long 0x00000000 # bits 64- 95 set: -
.long 0x0f000000 # bits 96-127 set: 120-123
.long 0x00000000 # bits 128-159 set: -
bits: 0-3,120-123
col2 PREEMPT 3-arg none -> chunks 48-95
col4 PREEMPT + @hintmap_4_0
.long 0x00000000 # bits 0- 31 set: -
.long 0x00000000 # bits 32- 63 set: -
.long 0x00000000 # bits 64- 95 set: -
.long 0x00000000 # bits 96-127 set: -
.long 0x00000000 # bits 128-159 set: -
bits: 0
preemption point 1
col0 PREEMPT + @hintmap_0
.long 0x0000000f # bits 0- 31 set: 0-3
.long 0x00000000 # bits 32- 63 set: -
.long 0x00000000 # bits 64- 95 set: -
.long 0x0f000000 # bits 96-127 set: 120-123
.long 0x00000000 # bits 128-159 set: -
bits: 0-3,120-123
col2 PREEMPT + @hintmap_2_1
.long 0x00000000 # bits 0- 31 set: -
.long 0x00000000 # bits 32- 63 set: -I
.long 0x00000000 # bits 64- 95 set: -
.long 0x00000000 # bits 96-127 set: -
.long 0x00000000 # bits 128-159 set: -
bits: 0
col4 PREEMPT + @hintmap_4_1
.long 0x00000000 # bits 0- 31 set: -
.long 0x00000000 # bits 32- 63 set: -
.long 0x00000000 # bits 64- 95 set: -
.long 0x00000000 # bits 96-127 set: -
.long 0x00000004 # bits 128-159 set: 130
bits: 130

It seems the distribution after the cut is

preemption point 0:
col0 chunks 0-3 4 chunk(s)
col2 chunks 48-95 48 chunk(s)
col4 chunks 120-123 4 chunk(s)
preemption point 1:
col0 chunks 0-3 4 chunk(s)
col2 chunks (empty) 0 chunk(s)
col4 chunks 130 1 chunk(s)

chunks 120-123 are requested here but no column's region covers them

larry9523

Another potential issue is we don't seem to have hintmap boundary check, e.g. bit > 144. Are we going to program shim DMA to access address out of 9MB? This might cause IOMMU error or data corruption?

preemption point 0
col0 PREEMPT + @hintmap_0
.long 0x00000000 # bits 0- 31 set: -
.long 0x00000000 # bits 32- 63 set: -
.long 0x00000000 # bits 64- 95 set: -
.long 0x00000000 # bits 96-127 set: -
.long 0x08000000 # bits 128-159 set: 155 <- set past chunk 143, outside the 9MB
bits: 155
out of range: 155
col2 PREEMPT + @hintmap_2
.long 0x00000000 # bits 0- 31 set: -
.long 0x00000000 # bits 32- 63 set: -
.long 0x00000000 # bits 64- 95 set: -
.long 0x00000000 # bits 96-127 set: -
.long 0x00000000 # bits 128-159 set: -
bits: 0
col4 PREEMPT + @hintmap_4
.long 0x00000000 # bits 0- 31 set: -
.long 0x00000000 # bits 32- 63 set: -
.long 0x00000000 # bits 64- 95 set: -
.long 0x00000000 # bits 96-127 set: -
.long 0x00000000 # bits 128-159 set: -
bits: 0

sonals · 2026-08-21T17:22:25Z

+  // 160-chunk span (worst case for a 10MB memtile) gives 160*64KB = 10MB,
+  // well within uint64_t range.  Reject obviously-corrupt bitmaps where span
+  // or first_bit exceed 2^32 (no real hintmap would be that wide).
+  if (first_bit != NO_BIT && (span > UINT32_MAX || first_bit > UINT32_MAX)) {


the hintmap is 512 bits wide and the first_bit should be between 0 and 511 (both inclusive). The above test does not make sense to me.

sonals · 2026-08-21T17:29:48Z

-void
-asm_parser::
-verify_hintmap_no_overlap()
+static std::vector<uint64_t>


This should be std::bitset<512>. Why are we using std::vector<uint64_t>?

sonals · 2026-08-21T17:32:00Z

+      w &= w - 1;  // clear lowest set bit
+    }
+  }
+  return chunks;  // already in ascending order since we scan low-to-high


This code should be simpler, get each of the 5 .long and add it to the bitset<512>

sonals · 2026-08-21T21:52:00Z

+        while (members.size() < segments_needed && zero_used < zero_hintmap_controllers.size())
+          members.push_back(zero_hintmap_controllers[zero_used++]);
      }
+      redistribute_component(idx, infos, members, chunks_cache, fixed_rngs);


This is too complicated. Get the final union std::bitset<512> and extract std::bitset<48> bits for each controller. Then perform the bit setting operation for the holes in each std::bitset<48>.

In future we can explore redistribution across columns.

Copilot AI lite review requested due to automatic review settings August 14, 2026 08:20

Copilot started reviewing on behalf of HimanshuChoudhary-Xilinx August 14, 2026 08:21 View session

Copilot AI reviewed Aug 14, 2026

View reviewed changes

Comment thread src/cpp/preprocessor/asm/asm_parser.cpp

Comment thread src/cpp/preprocessor/asm/asm_parser.cpp

Comment thread src/cpp/preprocessor/asm/asm_parser.cpp Outdated

github-actions Bot reviewed Aug 14, 2026

View reviewed changes

Comment thread src/cpp/preprocessor/asm/asm_parser.cpp Outdated

Comment thread src/cpp/preprocessor/asm/asm_parser.cpp Outdated

Comment thread src/cpp/preprocessor/asm/asm_parser.cpp Outdated

HimanshuChoudhary-Xilinx requested review from larry9523, sonals and xuhz August 14, 2026 14:40

larry9523 requested changes Aug 14, 2026

View reviewed changes

HimanshuChoudhary-Xilinx added 2 commits August 19, 2026 16:04

remove constraint for having hole in hintmap, it will transfer super …

d92a55f

…set of hintmap Signed-off-by: Himanshu Choudhary <Himanshu.Choudhary@amd.com>

resolve overlap due to holes + spec update + add test

86162c1

Signed-off-by: Himanshu Choudhary <Himanshu.Choudhary@amd.com>

HimanshuChoudhary-Xilinx force-pushed the hintmap_overlap branch from b307a32 to 86162c1 Compare August 19, 2026 10:50

info log

83ce35a

Signed-off-by: Himanshu Choudhary <Himanshu.Choudhary@amd.com>