[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:305
+        if (BBVisitedInfo[MBB].IsCallBeforeAMX)
+          S = std::min(S, HasAfterCallAMX);
+
----------------
LuoYuanke wrote:
> State HasAfterCallAMX seems useless. I didn't find we check this state anywhere.
Yes, it was used for inserting tilerelease. Now we just need 2 status, I simplified code base it.


================
Comment at: llvm/lib/Target/X86/X86PreTileConfig.cpp:313
+            // If BB has AMX, its predecessors don't need to be updated again.
+            if (!BBVisitedInfo[*I].HasAMX)
+              BBVisitedInfo[*I].NeedUpdatePred = true;
----------------
LuoYuanke wrote:
> Why if BBVisitedInfo[*I].HasAMX is true, we don't update it? I think when BBVisitedInfo[*I].MaxSucc changes, we need update its predecessor.
It is just an optimization.
If `*I` has AMX, the status it contributes to its predecessors won't be changed with its `MaxSucc`, e.g.
When we have a BB which has AMX and its `MaxSucc` is S0, it updates its predecessors with S2 in the first time. So next time, when its `MaxSucc` is changed to S1 by its successor, it still updates its predecessors with S2. This is not necessary, so it is skipped.
Anyway, since we are using 2 status now, this code is also been simplified.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95136