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

[LuoYuanke via Phabricator via llvm-commits]

LuoYuanke added inline comments.


================
Comment at: llvm/lib/CodeGen/LiveIntervals.cpp:982
+
+  bool Found = false;
+  while (true) {
----------------
pengfei wrote:
> Should `Found` always be `true` in our case?
No. For example slots is { 16, 32, 64 }  and LiveI is <17, 30>. 17 is LiveI->start, then SlotI is 32, but 32 is not in the range of <17, 30>, so Found will be false.


================
Comment at: llvm/test/CodeGen/X86/AMX/amx-across-func.ll:109
   %4 = tail call x86_amx @llvm.x86.tileloadd64.internal(i16 8, i16 %1, i8* getelementptr inbounds ([3072 x i8], [3072 x i8]* @buf, i64 0, i64 1024), i64 32)
-  tail call void (...) @foo()
+  call void @foo()
   %5 = tail call x86_amx @llvm.x86.tileloadd64.internal(i16 %0, i16 %1, i8* getelementptr inbounds ([3072 x i8], [3072 x i8]* @buf, i64 0, i64 2048), i64 32)
----------------
pengfei wrote:
> How about if it is a tail call?
1. Tail call should be the last instruction of a function, so I think no tile register should live through tail call.
2. Tail call will be transformed to Jmp and won't return back, so even we insert ldtilecfg, it won't be executed.


================
Comment at: llvm/test/CodeGen/X86/AMX/amx-across-func.ll:113
   tail call void @llvm.x86.tilestored64.internal(i16 %0, i16 %1, i8* getelementptr inbounds ([3072 x i8], [3072 x i8]* @buf, i64 0, i64 2048), i64 32, x86_amx %6)
   ret void
 }
----------------
pengfei wrote:
> It's better to add another foo after all users of tmm registers to make sure no `ldtilecfg` been generated.
Good suggestion. I'll add anther case for it.


================
Comment at: llvm/test/CodeGen/X86/AMX/amx-spill-merge.ll:110
 ; CHECK-NEXT:    retq
   %c = tail call x86_amx @llvm.x86.tileloadd64.internal(i16 %0, i16 %1, i8* getelementptr inbounds ([3072 x i8], [3072 x i8]* @buf, i64 0, i64 2048), i64 32)
   br i1 undef, label %if.true, label %if.false
----------------
pengfei wrote:
> It's better add a call before any user of tmm instructions to see if ldtilecfg can be sunk
Yes, this looks a good case to me, I'll add it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94155