[llvm] [AMDGPU] Fix GFX1250 hazard: S_SET_VGPR_MSB dropped (PR #184904)
Stanislav Mekhanoshin via llvm-commits
llvm-commits at lists.llvm.org
Mon Mar 9 10:53:45 PDT 2026
rampitec wrote:
> > I certainly do not like it in its current form. It is spaghetti code predicting what will come next. It is a layer violation. It is a downstream merge disaster. Either move mitigation into the VGPR lowering itself (easy), or change pass ordering and prove nothing will break (hard).
>
> Rebased to Jay's PR (#184987).
>
> Removed most of code for predicting MSB. Now it is mush simpler. Just find consecutive setreg and set msb insts and insert nop.
>
> My understanding is that this patch is handling the hazard due to normal mode setting inst with size > 12 (case 2). In this case we cannot piggyback the imm since those bits are part of mode which we cannot change. So in this case, due to the first hardware bug that part of mode bits leaks into VGPR MSB and the second hardware bug that the subsequent set msb inst not working, we have to insert nop to workaround these two hardware bugs. This is not due to piggyback. Even if we do not do piggyback, the issue still happens. After these two hardware bugs fixed, we can remove this workaround.
OK, let's try this. There is one pass now in between of the hazard recognizer and VGPR lowering, but it does not create VALU.
However, hazard recognizer itself create VALU uses and defs. Out of relevant for gfx1250 that is `fixVcmpxPermlaneHazards()`. Please add a test that hazard fix targets correct VGPR with proper ordering in respect to S_SET_VGPR_MSB. I assume it does not at least with respect to `vdst` of the produced `V_MOV_B32` if `vsdt` and `src0` MSBs are different.
https://github.com/llvm/llvm-project/pull/184904
More information about the llvm-commits
mailing list