[AIE] Improve Zero-overhead loop 112-bytes padding #440

abnikant · 2025-04-18T14:56:16Z

Avoid inserting extra nop bundles if there are enough bundles between ZOLSetup instructions and LoopEnd to create 112-bytes padding. It takes into account the bundles in the loop body.

Added heuristic to check for available size between ZOLSetup last bundle and (LoopEnd - 1) - for the cases where number of loop bundles are >= LoopSetupDistance.
In plan: More heuristics to be added 1) Avoid elongating if there are 112-bytes available between writing of (ls, le, and lc) and (loop end - 1), for the case where number of loop bundles < LoopSetupDistance. The EgdeLatency needs to be updated. 2) Elongate the bundles after deducting the available size.

QoR report :

martien-de-jong · 2025-04-22T07:13:20Z

llvm/lib/Target/AIE/AIEBaseInstrInfo.h

@@ -435,6 +439,10 @@ struct AIEBaseInstrInfo : public TargetInstrInfo {
  // setting LE Reg
  bool isZOLSetupBundle(MachineBasicBlock::iterator MII) const;
  bool isLastZOLSetupBundleInMBB(MachineBasicBlock::iterator MII) const;
+  virtual const AIE::Bundle<MachineInstr>


Check: we return by value, so I guess we have the implementation of AIE::Bundle available here. Can we drop the forward declaration?

llvm/lib/Target/AIE/AIEMachineAlignment.cpp

llvm/lib/Target/AIE/AIEBaseInstrInfo.cpp

abnikant · 2025-04-22T08:40:52Z

Core_Insn_Count.csv
Core_PMSize.csv
Core_StackSize.csv
QoR data w.r.t. to original run with Peano.

martien-de-jong · 2025-04-22T08:43:32Z

llvm/lib/Target/AIE/AIEBaseInstrInfo.cpp

+      if (IsZOLBody && ZOLBodyRegionsCount > 0) {
+        AlgnCandidates.emplace_back(MI);
+        ZOLBodyRegionsCount--;
+      }


Oh, you're actually padding the bundles in the loop. I had thought you would just subtract the size of the body in units of full bundles from LoopSetupDistance, putting all padding in the preheader. That would make the algorithm logic much simpler. Am I missing something?

yes, I am padding the loop bundles this way we can avoid inserting extra nop bundles in most of the cases. Consider the case where preheader size after writing to the ZOL setup instruction is 0, we will end of inserting some nop bundles if the loop size is not sufficient.

Fair point. But I'm still worried about the nops we insert in the gemm example, where the body was large enough.

llvm/test/CodeGen/AIE/aie2/elongate/zol_112bytes_elongate2.mir

llvm/test/CodeGen/AIE/aie2/schedule/postpipeliner/gemm-1.mir

andcarminati · 2025-04-22T09:09:03Z

llvm/test/CodeGen/AIE/aie2/schedule/postpipeliner/add-store.mir

@@ -12,50 +12,6 @@

 --- |
  define dso_local void @addStore(ptr addrspace(5) noalias nocapture writeonly %d, i32 noundef %n) local_unnamed_addr #0 {
-  ; CHECK-LABEL: addStore:


What happened here?

nit update_llc_test..

llvm/lib/Target/AIE/AIEBaseInstrInfo.cpp

martien-de-jong · 2025-04-22T13:46:20Z

llvm/lib/Target/AIE/AIEBaseInstrInfo.cpp

+        AlgnCandidates.emplace_back(MI);
+        ZOLBodyRegionsCount--;
+      }
+


Still, I think LoopSetupDistance and ZOLBodyRegionsCount are the same thing. We either have a preheader or a body of a ZOL, not both, and we have a number of bundles that should be expanded to 16 bytes. The body one computes that amount of bundles before hand, the preheader starts it at the last loop setup instruction.

llvm/lib/Target/AIE/AIEMachineAlignment.cpp

llvm/lib/Target/AIE/AIEBaseSubtarget.cpp

niwinanto · 2025-04-23T12:38:08Z

llvm/lib/Target/AIE/AIEBaseInstrInfo.cpp

+  if (Last == MBB.end())
+    return 0;
+
+  for (auto MI = MBB.begin(), End = MBB.end(); MI != End; ++MI) {


Nit: for (const auto &MI: MBB)

niwinanto · 2025-04-23T12:48:57Z

llvm/lib/Target/AIE/AIEBaseInstrInfo.cpp

+    return 0;
+
+  for (auto MI = MBB.begin(), End = MBB.end(); MI != End; ++MI) {
+    if (MI->isBundle() && isZOLSetupBundle(MI) &&


What is the difference between isZOLSetupBundle and isLastZOLSetupBundleInMBB?

There can be more ZOLSetUpBundles in a block, but only one is the last.

I guess we can streamline the code by running the loop in reverse and accumulating the region size for each bundle that we see. Then we can return the region size as soon as we hit a ZOLSetUpBundle. llvm::reverse(MBB) gives you the reverse range.

yes, updated the code to use loop in reverse, please check.

niwinanto · 2025-04-23T12:59:33Z

llvm/lib/Target/AIE/AIEBaseInstrInfo.cpp

  bool IsCall = false;
  auto ZOLSupport = getZOLSupport();
+  assert(ZOLSupport);


Is getAlignmentBoundaries used just with ZOL?

Good catch! Perhaps move the assert with the if (IsZOLBody) below.

It is true that this is not used only in case of ZOL, but why this should assert even if it has no ZOL? it is a query and just returns pre-populated struct object value.

niwinanto · 2025-04-23T13:05:23Z

llvm/lib/Target/AIE/AIEBaseInstrInfo.cpp

+    // must be a 112-byte gap (in PM address space) between writing to the ls,
+    // le, and lc registers and the LoopEnd instruction.
+    ZOLBundlesCount = getZOLBundlesCount(MBB) - 1;
+    if (ZOLBundlesCount < LoopSetupDist)


LoopSetupDist and LoopSetupDistance are confusing. Could you please rename one?

updated the name.

andcarminati · 2025-04-23T14:03:19Z

llvm/lib/Target/AIE/AIEBaseInstrInfo.cpp

+
+unsigned AIEBaseInstrInfo::getPostZOLRegionSizeFromZOLBody(
+    MachineBasicBlock &LoopMBB) const {
+  for (auto *Pred : LoopMBB.predecessors()) {


Should we use AIELoopUtils::getLoopPredecessor here?

I thought this is used only when the predecessor is a loop or isSingleMBBLoop. This function is called from ZOL Body MBB and we want to get the pre-header MBB.

andcarminati · 2025-04-23T14:16:17Z

llvm/lib/Target/AIE/AIEBaseInstrInfo.cpp

+  return 0;
+}
+
+unsigned AIEBaseInstrInfo::getPostZOLRegionSizeFromZOLBody(


I feel that this Post is Pre, or even PreHeader.

This is computing the number of bytes after the last ZOL setup instruction in the preheader, I guess name captures that, what is the name you suggest ?

maybe getPostZOLSetupRegionSize + this comment.

done, also moved into a lamda function.

llvm/lib/Target/AIE/AIEBaseInstrInfo.cpp

andcarminati

LGTM. Great work @abnikant !

abnikant requested review from abhinay-anubola, andcarminati, F-Stuckmann, gbossu, katerynamuts, khallouh, konstantinschwarz, martien-de-jong, niwinanto, SagarMaheshwari99 and stephenneuendorffer as code owners April 18, 2025 14:56

abnikant force-pushed the aie2p.zol.distance branch 2 times, most recently from 4e6d300 to 8d72538 Compare April 21, 2025 15:54

martien-de-jong reviewed Apr 22, 2025

View reviewed changes

llvm/lib/Target/AIE/AIEMachineAlignment.cpp Outdated Show resolved Hide resolved

martien-de-jong reviewed Apr 22, 2025

View reviewed changes

llvm/lib/Target/AIE/AIEBaseInstrInfo.cpp Outdated Show resolved Hide resolved

andcarminati reviewed Apr 22, 2025

View reviewed changes

llvm/lib/Target/AIE/AIEBaseInstrInfo.cpp Outdated Show resolved Hide resolved

andcarminati reviewed Apr 22, 2025

View reviewed changes

llvm/lib/Target/AIE/AIEBaseInstrInfo.cpp Outdated Show resolved Hide resolved

martien-de-jong reviewed Apr 22, 2025

View reviewed changes

llvm/lib/Target/AIE/AIEBaseInstrInfo.cpp Show resolved Hide resolved

andcarminati reviewed Apr 22, 2025

View reviewed changes

llvm/lib/Target/AIE/AIEBaseInstrInfo.cpp Outdated Show resolved Hide resolved

martien-de-jong reviewed Apr 22, 2025

View reviewed changes

llvm/test/CodeGen/AIE/aie2/elongate/zol_112bytes_elongate2.mir Show resolved Hide resolved

martien-de-jong reviewed Apr 22, 2025

View reviewed changes

llvm/test/CodeGen/AIE/aie2/schedule/postpipeliner/gemm-1.mir Outdated Show resolved Hide resolved

andcarminati reviewed Apr 22, 2025

View reviewed changes

martien-de-jong reviewed Apr 22, 2025

View reviewed changes

llvm/lib/Target/AIE/AIEBaseInstrInfo.cpp Outdated Show resolved Hide resolved

martien-de-jong reviewed Apr 22, 2025

View reviewed changes

niwinanto reviewed Apr 22, 2025

View reviewed changes

llvm/lib/Target/AIE/AIEMachineAlignment.cpp Outdated Show resolved Hide resolved

niwinanto reviewed Apr 22, 2025

View reviewed changes

llvm/lib/Target/AIE/AIEBaseSubtarget.cpp Show resolved Hide resolved

abnikant force-pushed the aie2p.zol.distance branch from a0361fa to 3731b75 Compare April 23, 2025 09:22

abnikant force-pushed the aie2p.zol.distance branch 3 times, most recently from 65c621d to f58c740 Compare April 23, 2025 12:11

niwinanto reviewed Apr 23, 2025

View reviewed changes

andcarminati reviewed Apr 23, 2025

View reviewed changes

abnikant force-pushed the aie2p.zol.distance branch from f58c740 to 9106adc Compare April 23, 2025 14:28

[AIE] Refactor AIEMachineAlignment

79ccf13

abnikant force-pushed the aie2p.zol.distance branch from 9106adc to 618aeae Compare April 23, 2025 14:30

andcarminati reviewed Apr 23, 2025

View reviewed changes

llvm/lib/Target/AIE/AIEBaseInstrInfo.cpp Show resolved Hide resolved

[AIE] Improve Zero-overhead loop 112-bytes padding

87a5279

abnikant force-pushed the aie2p.zol.distance branch from 618aeae to 87a5279 Compare April 23, 2025 15:49

martien-de-jong approved these changes Apr 23, 2025

View reviewed changes

andcarminati approved these changes Apr 23, 2025

View reviewed changes

abnikant merged commit b1ac573 into aie-public Apr 23, 2025
6 checks passed

[AIE] Improve Zero-overhead loop 112-bytes padding #440

[AIE] Improve Zero-overhead loop 112-bytes padding #440

Uh oh!

Conversation

abnikant commented Apr 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

abnikant commented Apr 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

andcarminati left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

abnikant commented Apr 18, 2025 •

edited

Loading

abnikant commented Apr 22, 2025 •

edited

Loading