[PATCH] D103197: [AMDGPU] All GWS instructions need aligned VGPR on gfx90a

Matt Arsenault via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jun 1 15:09:18 PDT 2021


arsenm added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/SIInstrInfo.cpp:4351-4353
+    // Register class of these instructions is fixed in the finalize lowering,
+    // thus do not check it before we done with SSA which is guaranteed past
+    // that point.
----------------
rampitec wrote:
> rampitec wrote:
> > arsenm wrote:
> > > The SSA check doesn't make sense, the class is unrelated
> > Any idea how to skip verifier after selection with gisel and before finalize lowering? We should have lot more problems like this as finalizeLowering() calls legalizeOperands() for many more instructions, we just do not have enough tests I guess.
> Here is what happens with gisel:
> 
> 
> ```
> # *** IR Dump After InstructionSelect (instruction-select) ***:
> # Machine code for function gws_init_odd_agpr: IsSSA, TracksLiveness, Legalized, RegBankSelected, Selected
> Function Live Ins: $sgpr0_sgpr1 in %1, $sgpr2 in %6
> ...
>   DS_GWS_INIT %16:vgpr_32, 0, implicit $m0, implicit $exec :: (store 4 into custom "GWSResource")
> ...
> # *** IR Dump After Finalize ISel and expand pseudo-instructions (finalize-isel) ***:
> # Machine code for function gws_init_odd_agpr: IsSSA, TracksLiveness, Legalized, RegBankSelected, Selected
> Function Live Ins: $sgpr0_sgpr1 in %1, $sgpr2 in %6
> ...
>   BUNDLE implicit %19:vreg_64_align2, implicit $m0, implicit $exec {
>     DS_GWS_INIT %19.sub0:vreg_64_align2, 0, implicit $m0, implicit $exec, implicit %19:vreg_64_align2 :: (store 4 into custom "GWSResource")
>     S_WAITCNT 0
>   }
> ```
> 
> If verifier kiks-in before finalizeLowering the instruction is not yet fixed, just like any other instruction which is supposed to be legalized there.
Oh right, this problem. We currently have an ugly hack where we disable the verifier for selectiondag before finalize lowering, but I would like to avoid spreading that problem.

Can you adjust the class directly in selectDSGWSIntrinsic for gisel?


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

https://reviews.llvm.org/D103197



More information about the llvm-commits mailing list