Skip to content

fix(rocr): scope the VOP3PX trap PC fixup to gfx1250 revision 0 #13797

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
shwetagkhatri wants to merge 2 commits into
base: develop
Choose a base branch
from
+84 −11
Open

fix(rocr): scope the VOP3PX trap PC fixup to gfx1250 revision 0 #13797

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
52c988f
rocr: scope the VOP3PX trap PC fixup to gfx1250 revision 0
shwetagkhatri Oct 19, 2026
110e104
rocr: refine revision-specific trap handler selection
shwetagkhatri Oct 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 28 additions & 3 deletions projects/rocr-runtime/runtime/hsa-runtime/core/runtime/amd_gpu_agent.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,13 @@ void GpuAgent::AssembleShader(const char* func_name, AssembleTarget assemble_tar
ASICShader compute_11;
ASICShader compute_12;
ASICShader compute_1250;
Comment thread
shwetagkhatri marked this conversation as resolved.
// Only populated where a gfx1250 revision 0 part needs a different blob.
// Left zeroed elsewhere, in which case compute_1250 is used for every
// revision. Note that compute_1250 is the blob WITHOUT the revision 0
// instruction fixup, so any entry that supplies a gfx1250 trap handler
// must fill this slot as well; leaving it zeroed hands a revision 0 part
// a handler that is missing the fixup it requires.
ASICShader compute_1250_rev0;
};

std::map<std::string, CompiledShader> compiled_shaders = {
Expand Down Expand Up @@ -377,7 +384,8 @@ void GpuAgent::AssembleShader(const char* func_name, AssembleTarget assemble_tar
{kCodeTrapHandlerV2_10, sizeof(kCodeTrapHandlerV2_10), 2, 4}, // gfx10
{kCodeTrapHandlerV2_11, sizeof(kCodeTrapHandlerV2_11), 2, 4}, // gfx11
{kCodeTrapHandlerV2_12, sizeof(kCodeTrapHandlerV2_12), 2, 4}, // gfx12
{kCodeTrapHandlerV2_1250, sizeof(kCodeTrapHandlerV2_1250), 2, 4}, // gfx1250
{kCodeTrapHandlerV2_1250, sizeof(kCodeTrapHandlerV2_1250), 2, 4}, // gfx1250 rev 1+
{kCodeTrapHandlerV2_1250_rev0, sizeof(kCodeTrapHandlerV2_1250_rev0), 2, 4}, // gfx1250 rev 0
}},
{"CopyAligned",
{
Expand Down Expand Up @@ -451,10 +459,27 @@ void GpuAgent::AssembleShader(const char* func_name, AssembleTarget assemble_tar
asic_shader = &compiled_shader_it->second.compute_11;
break;
case 12:
if(supported_isas()[0]->GetMinorVersion() >= 5)
if (supported_isas()[0]->GetMinorVersion() == 5 &&
properties_.Capability.ui32.ASICRevision == 0) {
// The trap-during-VOP3PX PC fixup in the second-level trap handler
// applies to minor version 5 revision 0 only. Every later revision,
// and every later minor version, uses the blob assembled without it.
// Shaders that do not vary by revision leave this slot zeroed.
ASICShader& rev0 = compiled_shader_it->second.compute_1250_rev0;
// An entry that supplies the later-revision blob must supply the
// revision 0 blob too. Falling back would hand a revision 0 part
// the blob assembled without the fixup, and the check below cannot
// catch it because the fallback pointer is valid.
assert((rev0.code != NULL ||
compiled_shader_it->second.compute_1250.code == NULL) &&
"Incomplete shader table entry for this revision");
asic_shader =
(rev0.code != NULL) ? &rev0 : &compiled_shader_it->second.compute_1250;
} else if (supported_isas()[0]->GetMinorVersion() >= 5) {
asic_shader = &compiled_shader_it->second.compute_1250;
else
} else {
asic_shader = &compiled_shader_it->second.compute_12;
}
break;
default:
assert(false && "Precompiled shader unavailable for target");
Expand Down
49 changes: 41 additions & 8 deletions projects/rocr-runtime/runtime/hsa-runtime/core/runtime/trap_handler/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,33 @@ cmake_minimum_required ( VERSION 3.7 )
find_package(Clang REQUIRED HINTS ${CMAKE_PREFIX_PATH}/llvm PATHS /opt/rocm/llvm )
find_package(LLVM REQUIRED HINTS ${CMAKE_PREFIX_PATH}/llvm PATHS /opt/rocm/llvm )

set (TARGET_DEVS "gfx900;gfx942;gfx950;gfx1010;gfx1030;gfx1100;gfx1200;gfx1250")
set (POSTFIX "9;942;950;1010;10;11;12;1250")
set (SOURCE_SUFFIX ";;;;;;_gfx12;_gfx12")
# gfx1250 is built twice from the same source. The default build takes no
# -defsym and so drops the trap-during-VOP3PX instruction fixup; it serves
# revision 1 and later. The _rev0 build opts the fixup back in with
# HAVE_VOP3PX_FIXUP=1. GpuAgent::AssembleShader picks between them using the
# ASIC revision reported by KFD. The opt-in direction is deliberate: a target
# added here without a matching ASM_DEFSYM entry gets the handler without the
# rewind, which is correct everywhere except revision 0.
set (TARGET_DEVS "gfx900;gfx942;gfx950;gfx1010;gfx1030;gfx1100;gfx1200;gfx1250;gfx1250")
set (POSTFIX "9;942;950;1010;10;11;12;1250;1250_rev0")
set (SOURCE_SUFFIX ";;;;;;_gfx12;_gfx12;_gfx12")
set (ASM_DEFSYM ";;;;;;;;HAVE_VOP3PX_FIXUP=1")

# The four lists above are indexed in parallel, so a miscounted separator in
# any one of them silently pairs a value with the wrong target rather than
# failing. Keep them the same length.
list (LENGTH TARGET_DEVS _n_devs)
list (LENGTH POSTFIX _n_postfix)
list (LENGTH SOURCE_SUFFIX _n_suffix)
list (LENGTH ASM_DEFSYM _n_defsym)
if (NOT _n_postfix EQUAL _n_devs OR
NOT _n_suffix EQUAL _n_devs OR
NOT _n_defsym EQUAL _n_devs)
message (FATAL_ERROR
"Trap handler target lists must be the same length: "
"TARGET_DEVS=${_n_devs} POSTFIX=${_n_postfix} "
"SOURCE_SUFFIX=${_n_suffix} ASM_DEFSYM=${_n_defsym}")
endif()

if(${CMAKE_VERBOSE_MAKEFILE})
get_property(clang_path TARGET clang PROPERTY LOCATION)
Expand All @@ -61,12 +85,20 @@ endif()
##==========================================
## Add custom command to generate a kernel code object file
##==========================================
function(gen_kernel_bc TARGET_ID INPUT_FILE OUTPUT_FILE)
function(gen_kernel_bc TARGET_ID INPUT_FILE OUTPUT_FILE DEFSYM)

set(CODE_OBJECT "${OUTPUT_FILE}.hsaco")

## Assembler symbol overrides, e.g. HAVE_VOP3PX_FIXUP=1. The source provides
## a default for every such symbol under .ifndef, so an empty DEFSYM yields
## the handler with every optional fixup left out.
set(DEFSYM_ARG "")
if(NOT "${DEFSYM}" STREQUAL "")
set(DEFSYM_ARG "-Wa,-defsym,${DEFSYM}")
endif()

separate_arguments(CLANG_ARG_LIST UNIX_COMMAND
"-x assembler -target amdgcn-amd-amdhsa -mcpu=${TARGET_ID} -o ${CODE_OBJECT} ${INPUT_FILE}")
"-x assembler -target amdgcn-amd-amdhsa -mcpu=${TARGET_ID} ${DEFSYM_ARG} -o ${CODE_OBJECT} ${INPUT_FILE}")

## Add custom command to produce a code object file.
add_custom_command(OUTPUT ${CODE_OBJECT} COMMAND clang ${CLANG_ARG_LIST}
Expand All @@ -93,12 +125,12 @@ endfunction(gen_kernel_bc)
##==========================================
## Find device code object name and forward to custom command
##==========================================
function(build_kernel TRAP_HANDLER_NAME TARGET_ID POSTFIX SOURCE_SUFFIX)
function(build_kernel TRAP_HANDLER_NAME TARGET_ID POSTFIX SOURCE_SUFFIX DEFSYM)

## generate trap handler object code files
set (CODE_OBJECT_FILE "${TRAP_HANDLER_NAME}_${POSTFIX}")
set (TRAP_FILE "${CMAKE_CURRENT_SOURCE_DIR}/trap_handler${SOURCE_SUFFIX}.s")
gen_kernel_bc(${TARGET_ID} ${TRAP_FILE} ${CODE_OBJECT_FILE})
gen_kernel_bc(${TARGET_ID} ${TRAP_FILE} ${CODE_OBJECT_FILE} "${DEFSYM}")

## Build a list of code object file names
## These will be target dependencies.
Expand All @@ -119,10 +151,11 @@ function(build_kernel_for_devices TRAP_HANDLER_NAME)
list(GET TARGET_DEVS ${ind} dev)
list(GET POSTFIX ${ind} post)
list(GET SOURCE_SUFFIX ${ind} suffix)
list(GET ASM_DEFSYM ${ind} defsym)
if(${CMAKE_VERBOSE_MAKEFILE})
message("\n Generating: ${dev} ...")
endif()
build_kernel(${TRAP_HANDLER_NAME} ${dev} ${post} "${suffix}")
build_kernel(${TRAP_HANDLER_NAME} ${dev} ${post} "${suffix}" "${defsym}")
endforeach(ind)

set(HSACO_TARG_LIST ${HSACO_TARG_LIST} PARENT_SCOPE)
Expand Down
15 changes: 15 additions & 0 deletions projects/rocr-runtime/runtime/hsa-runtime/core/runtime/trap_handler/trap_handler_gfx12.s
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,17 @@
////////////////////////////////////////////////////////////////////////////////

/// Trap Handler V2 source

// Trap-during-VOP3PX instruction fixup. Off unless a build opts in with
// -defsym,HAVE_VOP3PX_FIXUP=1, which only the gfx1250 revision 0 build does.
// Defaulting to off means a target added without a matching ASM_DEFSYM entry
// gets the unmodified handler rather than a rewind it must not perform.
// The VGPR MSB fixup below is deliberately not scoped - it applies to
// every revision.
.ifndef HAVE_VOP3PX_FIXUP
Comment thread
shwetagkhatri marked this conversation as resolved.
.set HAVE_VOP3PX_FIXUP , 0
.endif

.set DOORBELL_ID_SIZE , 10
.set DOORBELL_ID_MASK , ((1 << DOORBELL_ID_SIZE) - 1)
.set EC_QUEUE_WAVE_ABORT_M0 , (1 << (DOORBELL_ID_SIZE + 0))
Expand Down Expand Up @@ -1266,11 +1277,13 @@
s_cbranch_scc1 .fixup_three_dword // If VOPD3, 3 DWORD inst
// Not VOP1, VOP2, VOPC, VOP3, VOP3SD, VOPD, or VOPD3.
// Check if we are in the middle of VOP3PX.
.if HAVE_VOP3PX_FIXUP
s_and_b32 ttmp13, ttmp14, 0xffff0000 // Bits 31:16
s_cmp_eq_u32 ttmp13, 0xcc330000 // If 31:16 = 0xcc33, this is 8 bytes past VOP3PX
s_cbranch_scc1 .fixup_vop3px_middle
s_cmp_eq_u32 ttmp13, 0xcc880000 // If 31:16 = 0xcc88, this is 8 bytes past VOP3PX
s_cbranch_scc1 .fixup_vop3px_middle
.endif
// Might be in VOP3P, but we must ensure we are not VOP3PX2
s_and_b32 ttmp13, ttmp14, 0xffff0000 // Bits 31:16
s_cmp_eq_u32 ttmp13, 0xcc350000 // If 31:16 = 0xcc35, this is VOP3PX2
Expand Down Expand Up @@ -1331,10 +1344,12 @@
s_wait_kmcnt 0 // Wait for PC+2 and PC+3 to arrive in ttmp2 and ttmp3
s_mov_b32 ttmp15, ttmp3 // Move possible S_SET_VGPR_MSB into ttmp15
s_branch .fixup_one_dword // Go to common logic that checks if it is S_SET_VGPR_MSB
.if HAVE_VOP3PX_FIXUP
.fixup_vop3px_middle:
s_sub_co_u32 ttmp0, ttmp0, 8 // Rewind PC 8 bytes to beginning of instruction
s_sub_co_ci_u32 ttmp1, ttmp1, 0
s_branch .fixup_two_dword // 2 DWORD inst (2nd half of a 4 DWORD inst)
.endif
.fixup_done:

s_wait_idle // Required by SPG before reading/writing XNACK_STATE_PRIV
Expand Down
Loading