[PATCH] D95136: [X86] Fix tile config register spill issue.

[Pengfei Wang via Phabricator via llvm-commits]

pengfei added inline comments.


================
Comment at: llvm/lib/Target/X86/X86PreTileConfig.cpp:283
+    // But iterating all BBs isn't too expensive here.
+    for (auto I : BBVisitedInfo) {
+      if (I.second.NeedUpdatePred)
----------------
xiangzhangllvm wrote:
> I am not sure the order of iterate  BBVisitedInfo is right.
> In map it may use "<" to sort its elements.
The order doesn't matter. Just make sure all BBs are iterated.


================
Comment at: llvm/lib/Target/X86/X86PreTileConfig.cpp:292
+        if (BBVisitedInfo[MBB].LastAMX)
+          S = HasBeforeCallAMX;
+        if (BBVisitedInfo[MBB].HasCallBeforeAMX)
----------------
xiangzhangllvm wrote:
> What does HasBeforeCallAMX really mean, why set it once BB has AMX ?
It will be updated soon in line 293. We can assume it alway has AMX before call first. Then together with no AMX case, we check if there's call before AMX. If there is, we update it to `HasAfterCallAMX`.


================
Comment at: llvm/lib/Target/X86/X86PreTileConfig.cpp:298
+          if (!BBVisitedInfo.count(*I))
+            continue;
+          if (S > BBVisitedInfo[*I].MaxSucc) {
----------------
xiangzhangllvm wrote:
> Not sure directly continue is right, if the status required by the visited predecessors.
We don't need to care about the BB which is not found when we do searching from top to bottom.
Becaused these BBs are never to be a successor of any AMX instruction.
We might meet them sometimes since we are searching from bottom to top. Ignoring them is safe.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D95136/new/

https://reviews.llvm.org/D95136