[PATCH] D157458: [X86][AMX] Fix virtual register traversing in case of GlobalIsel

Thu Aug 10 17:39:19 PDT 2023

e-kud added inline comments.

================
Comment at: llvm/test/CodeGen/X86/AMX/amx-fastpreconfig-gisel.mir:2
+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py UTC_ARGS: --version 2
+# RUN: llc -mtriple=x86_64-- -run-pass=fastpretileconfig -o - %s | FileCheck %s
+
----------------
e-kud wrote:
> qcolombet wrote:
> > 1. Unless you pass `-global-isel=1` GlobalISel doesn't run I believe for X86.
> > 2. X86FastPreTileConfig runs only at O0. My guess is you're seeing an issue with FastISel.
> > 
> > Could you do a print-after-all with your failure (starting back at LLVM IR level) and see where the regclass is not properly set?
> > It doesn't look like a GlobalISel issue to me.
> > Put differently, you shouldn't see any null regclass after ISel (GISel or SDISel) period.
> We don't need to pass `-global-isel=1` because this `MIR` is already obtained from `GlobalISel` and `llc` just runs the problematic pass.
> 
> On [the previous iteration](https://reviews.llvm.org/D157458?id=548410) I put a test from `.ll` but I think it is not convenient to keep it actual. After specific changes in `GlobalISel` we need to update it and in the worst case we have to find another test when a register doesn't have an assigned class. MIR test solves this problem.
> 
> Here is print-after-all output
> ```
> *** IR Dump After Module Verifier (verify) ***
> define i64 @f(i64 %0, i64 %1) {
> entry:
>   %2 = lshr i64 %0, %1
>   %3 = add i64 %2, 123456789
>   ret i64 %3
> }
> # *** IR Dump After IRTranslator (irtranslator) ***:
> # Machine code for function f: IsSSA, TracksLiveness
> Function Live Ins: $rdi, $rsi
> 
> bb.1.entry:
>   liveins: $rdi, $rsi
>   %0:_(s64) = COPY $rdi
>   %1:_(s64) = COPY $rsi
>   %3:_(s64) = G_CONSTANT i64 123456789
>   %2:_(s64) = G_LSHR %0:_, %1:_(s64)
>   %4:_(s64) = G_ADD %2:_, %3:_
>   $rax = COPY %4:_(s64)
>   RET 0, implicit $rax
> 
> # End machine code for function f.
> 
> # *** IR Dump After Legalizer (legalizer) ***:
> # Machine code for function f: IsSSA, TracksLiveness, Legalized
> Function Live Ins: $rdi, $rsi
> 
> bb.1.entry:
>   liveins: $rdi, $rsi
>   %0:_(s64) = COPY $rdi
>   %1:_(s64) = COPY $rsi
>   %3:_(s64) = G_CONSTANT i64 123456789
>   %5:_(s8) = G_TRUNC %1:_(s64)
>   %2:_(s64) = G_LSHR %0:_, %5:_(s8)
>   %4:_(s64) = G_ADD %2:_, %3:_
>   $rax = COPY %4:_(s64)
>   RET 0, implicit $rax
> 
> # End machine code for function f.
> 
> # *** IR Dump After RegBankSelect (regbankselect) ***:
> # Machine code for function f: IsSSA, TracksLiveness, Legalized, RegBankSelected
> Function Live Ins: $rdi, $rsi
> 
> bb.1.entry:
>   liveins: $rdi, $rsi
>   %0:gpr(s64) = COPY $rdi
>   %1:gpr(s64) = COPY $rsi
>   %3:gpr(s64) = G_CONSTANT i64 123456789
>   %5:gpr(s8) = G_TRUNC %1:gpr(s64)
>   %2:gpr(s64) = G_LSHR %0:gpr, %5:gpr(s8)
>   %4:gpr(s64) = G_ADD %2:gpr, %3:gpr
>   $rax = COPY %4:gpr(s64)
>   RET 0, implicit $rax
> 
> # End machine code for function f.
> 
> # *** IR Dump After InstructionSelect (instruction-select) ***:
> # Machine code for function f: IsSSA, TracksLiveness, Legalized, RegBankSelected, Selected
> Function Live Ins: $rdi, $rsi
> 
> bb.1.entry:
>   liveins: $rdi, $rsi
>   %0:gr64 = COPY $rdi
>   %1:gr64_with_sub_8bit = COPY $rsi
>   %5:gr8 = COPY %1.sub_8bit:gr64_with_sub_8bit
>   $cl = COPY %5:gr8
>   %2:gr64 = SHR64rCL %0:gr64(tied-def 0), implicit-def $eflags, implicit $cl
>   %4:gr64 = ADD64ri32 %2:gr64(tied-def 0), 123456789, implicit-def $eflags
>   $rax = COPY %4:gr64
>   RET 0, implicit $rax
> 
> # End machine code for function f.
> 
> # *** IR Dump After Finalize ISel and expand pseudo-instructions (finalize-isel) ***:
> # Machine code for function f: IsSSA, TracksLiveness, Legalized, RegBankSelected, Selected
> Function Live Ins: $rdi, $rsi
> 
> bb.1.entry:
>   liveins: $rdi, $rsi
>   %0:gr64 = COPY $rdi
>   %1:gr64_with_sub_8bit = COPY $rsi
>   %5:gr8 = COPY %1.sub_8bit:gr64_with_sub_8bit
>   $cl = COPY %5:gr8
>   %2:gr64 = SHR64rCL %0:gr64(tied-def 0), implicit-def $eflags, implicit $cl
>   %4:gr64 = ADD64ri32 %2:gr64(tied-def 0), 123456789, implicit-def $eflags
>   $rax = COPY %4:gr64
>   RET 0, implicit $rax
> ```
> 
> A virtual register `%3` is absent after instruction-select and doesn't have an assigned register class, only a register bank.
> A comment obtained using debug-only:
> ```
> Selecting:
>   %3:gpr(s64) = G_CONSTANT i64 123456789
> Is dead; erasing.
> ```
> 
> The test is obtained using `-stop-after`, so you can see that `%3` really has only a register bank not a class. The same I see on `AArch64`. Either both targets do something wrong or the problem is in general GISel algorithm. But we do have null regclass after GISel.
Yes, it is pretty simple:
Elimination of instructions during instruction-select happens before selection by checking `isTriviallyDead` predicate. Classes are assigned during selection.

https://github.com/llvm/llvm-project/commit/931904d7772b18210d7166ed657176ae91ad11d8

> In some tests, the G_CONSTANT vregs never get constrained to a class:
> the only use of the vreg was folded into another instruction, so the
> G_CONSTANT, now dead, never gets selected.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157458