[PATCH] D75909: [AMDGPU] Remove the gfx10 VALU register destination cache model

[Jay Foad via Phabricator via llvm-commits]

foad marked an inline comment as done.
foad added a comment.

In D75909#1916525 <https://reviews.llvm.org/D75909#1916525>, @nhaehnle wrote:

> Either way, the moral of it is that if we can schedule dependent instructions to land *exactly* after the latency of the instructions that they depend on, then that's a win -- but it's a weak one and should be deprioritized relative to almost all other concerns.


My gut feeling is that in most cases the scheduler would already like to use results ASAP, to reduce overall latency. Teaching it that the advantage of using a result ASAP instead of 1 cycle later is //more// than just saving 1 cycle, would be an incredibly subtle change.

> The bigger problem is that I don't see how the existing modeling via HWRC models what we want in the first place

Exactly. It defines a resource that is used for 1 cycle by every register write, but I don't understand how that is supposed to improve the scheduling.



================
Comment at: llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.update.dpp.ll:65-68
 ; GFX10-NEXT:    v_mov_b32_e32 v3, s1
 ; GFX10-NEXT:    v_mov_b32_e32 v2, s0
-; GFX10-NEXT:    v_mov_b32_e32 v5, s3
-; GFX10-NEXT:    v_mov_b32_e32 v4, s2
-; GFX10-NEXT:    v_add_co_u32_e64 v6, vcc_lo, v2, v0
-; GFX10-NEXT:    v_add_co_ci_u32_e32 v7, vcc_lo, v3, v1, vcc_lo
-; GFX10-NEXT:    global_load_dwordx2 v[2:3], v[6:7], off
+; GFX10-NEXT:    v_add_co_u32_e64 v0, vcc_lo, v2, v0
+; GFX10-NEXT:    v_add_co_ci_u32_e32 v1, vcc_lo, v3, v1, vcc_lo
----------------
nhaehnle wrote:
> This is a regression because the distance between dependent instructions is reduced too much. But before worrying about that, we need to worry about what looks like an unnecessary move from SGPR.
It's hard for the (pre-RA) scheduler to do a good job with copies/moves because it doesn't know what the register allocator will do with them. So I think this is probably just luck.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75909