Skip to content

[AIEX] Hazard Recognizer drops Load Store MemoryBank conflicts#981

Merged
F-Stuckmann merged 2 commits into
aie-publicfrom
stuckmann.mem-bank-loads-only
May 12, 2026
Merged

[AIEX] Hazard Recognizer drops Load Store MemoryBank conflicts#981
F-Stuckmann merged 2 commits into
aie-publicfrom
stuckmann.mem-bank-loads-only

Conversation

@F-Stuckmann

@F-Stuckmann F-Stuckmann commented May 4, 2026

Copy link
Copy Markdown
Collaborator

I have a conflict() refactor in here. We previously had multiple different conflict locations and i unified them in this PR.

@andcarminati

Copy link
Copy Markdown
Collaborator

Could we verify if two stores may collide if they are one cycle apart? For example, we have stores accessing memory on cycle 5, 6 or even 7. If a 5-cycle store is scheduled just before a 6-cycle store, they will stall the memory interface. Could we still handle store banks separately?

@F-Stuckmann

Copy link
Copy Markdown
Collaborator Author

I think we already handle this because already use the same resources.
Nonetheless i will add a unit-test to verify

Comment thread llvm/lib/Target/AIE/AIEHazardRecognizer.cpp Outdated
Comment thread llvm/lib/Target/AIE/AIEHazardRecognizer.cpp Outdated
};
},
FUDepthLimit);
return Found;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not return anyStage(...) ?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I deleted the NFC refactor commit, it was just adding noise

Comment thread llvm/lib/Target/AIE/AIEHazardRecognizer.cpp Outdated
Comment thread llvm/lib/Target/AIE/AIEHazardRecognizer.cpp Outdated
Comment thread llvm/lib/Target/AIE/AIEPostPipeliner.cpp
@F-Stuckmann F-Stuckmann force-pushed the stuckmann.mem-bank-loads-only branch 3 times, most recently from 57b2f0d to 5d7ac78 Compare May 11, 2026 14:51

// Same cycle without slot, resources don't overlap -> no hazard.
EXPECT_FALSE(HR.hazard(14, 0, /*SlotSet=*/0b0, /*MemoryBanks=*/0,
/*ObjectsBits=*/0, /*MemoryAccessCycle=*/{6}));

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed also expect no hazard at -2.

@F-Stuckmann F-Stuckmann force-pushed the stuckmann.mem-bank-loads-only branch from 5d7ac78 to abb0223 Compare May 11, 2026 16:11
Stop tracking memory-bank usage for store instructions in the AIE
scoreboard. Two facts justify this:

1. Loads and stores use separate HW ports, so a load and a store on the
   same bank in the same cycle never conflict.
2. AIE2/AIE2P/AIE2PS each have a single store slot
Two unit tests document the post-load-only-bank-tracking behaviour for
stores, in response to a review question: now that stores no longer
populate the memory-bank scoreboard, what keeps two stores from
issuing in the same cycle?

* storeStoreSameMemoryCycleSerializesViaSlot -- two stores with the
  same MemoryAccessCycles and an overlapping st_slot collide at the
  emission cycle via the slot scoreboard, while one-cycle-apart deltas
  are clear (no spurious bank conflict).

* storeStoreDifferentMemoryCycleSerializesViaSlot -- two stores with
  *different* MemoryAccessCycles (e.g. {5} vs {6}/{7}) at the same
  emission cycle still collide via the slot scoreboard, regardless of
  memory-cycle disagreement. Also verifies that a staggered emission
  whose memory accesses would have aligned under the previous
  all-stores bank-tracking policy reports no hazard, since stores now
  carry zero bank bits.

Both tests pass MemoryBanks=0 to model the new policy and rely on the
overlapping SlotSet to drive the hazard, mirroring the
bankConflictHazard tests' use of MockStages classes 1 and 3 (no
resource overlap).
@F-Stuckmann F-Stuckmann force-pushed the stuckmann.mem-bank-loads-only branch from abb0223 to 73091fa Compare May 12, 2026 07:40
@F-Stuckmann F-Stuckmann merged commit 5fe6378 into aie-public May 12, 2026
7 checks passed
@F-Stuckmann F-Stuckmann deleted the stuckmann.mem-bank-loads-only branch May 12, 2026 09:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants