Branch: refs/heads/bbiiggppiigg/amdgpu-getbaseregister-fix
Home: https://github.com/dyninst/dyninst
Commit: 7f3184752db6494bbeb5cca6ec6e52f691cbb4a3
https://github.com/dyninst/dyninst/commit/7f3184752db6494bbeb5cca6ec6e52f691cbb4a3
Author: wuxx1279 <bbiiggppiigg@xxxxxxxxx>
Date: 2026-07-07 (Tue, 07 Jul 2026)
Changed paths:
M common/src/registers/MachRegister.C
Log Message:
-----------
AMDGPU: treat vcc/exec/flat_scratch lo/hi halves as base registers
MachRegister::getBaseRegister() aliased the 32-bit halves to their combined
64-bit register on AMDGPU: vcc_lo/vcc_hi -> vcc, exec_lo/exec_hi -> exec, and
flat_scratch_lo/flat_scratch_hi -> flat_scratch_all. Those combined registers are
not present in the maps that consume base registers â the liveness index map
(machRegIndex_amdgpu_gfx908/90a/940 in dataflowAPI) and the register-conversion
maps (RegisterConversion-amdgpu.C) are all keyed on the _lo/_hi halves. As a
result:
* LivenessAnalyzer::calcRWSets computes getIndex(getBaseRegister(reg)); for
vcc_lo/exec_lo/flat_scratch_lo this returned getIndex(vcc/exec/
flat_scratch_all) = -1, and the `if (index >= 0)` guard silently discarded the
read AND the write. VCC/EXEC/FLAT_SCRATCH were therefore never tracked, so VCC
was reported dead even across a live carry chain (v_add_co_u32 defines VCC,
v_addc_co_u32 uses it). Any analysis or transformation that relies on liveness
would treat these live registers as free.
* convertRegID() looks up getBaseRegister(reg) in the reverse register map;
since the combined register is absent it returned the "ignored" register.
On AMDGPU each of vcc_lo/vcc_hi, exec_lo/exec_hi and flat_scratch_lo/
flat_scratch_hi is itself an independently addressable 32-bit base register (and
is exactly what the maps above are keyed on), so getBaseRegister now returns
*this for them on all three supported AMDGPU architectures. A 64-bit VCC/EXEC
operand is already decoded as a MultiRegister of its two halves, so the combined
form is not needed as an operand or base register.
Co-Authored-By: Claude Opus 4.8 <noreply@xxxxxxxxxxxxx>
Commit: 52cf1115f98bb7f75c48dbf39f99264493b0ce63
https://github.com/dyninst/dyninst/commit/52cf1115f98bb7f75c48dbf39f99264493b0ce63
Author: wuxx1279 <bbiiggppiigg@xxxxxxxxx>
Date: 2026-07-07 (Tue, 07 Jul 2026)
Changed paths:
M common/h/registers/AMDGPU/amdgpu_gfx908_regs.h
M common/h/registers/AMDGPU/amdgpu_gfx90a_regs.h
M common/h/registers/AMDGPU/amdgpu_gfx940_regs.h
M common/src/registers/MachRegister.C
M instructionAPI/src/AMDGPU/gfx908/decodeOperands.C
M instructionAPI/src/AMDGPU/gfx90a/decodeOperands.C
M instructionAPI/src/AMDGPU/gfx940/decodeOperands.C
M tests/unit/MachRegister/base_registers/amdgpu_gfx908.cpp
M tests/unit/MachRegister/base_registers/amdgpu_gfx90a.cpp
M tests/unit/MachRegister/base_registers/amdgpu_gfx940.cpp
M tests/unit/MachRegister/type_queries/amdgpu_gfx908.cpp
M tests/unit/MachRegister/type_queries/amdgpu_gfx90a.cpp
M tests/unit/MachRegister/type_queries/amdgpu_gfx940.cpp
Log Message:
-----------
AMDGPU: remove ambiguous combined vcc/exec/flat_scratch registers
Follow-on to representing vcc/exec/flat_scratch by their 32-bit halves. The
combined 64-bit registers (vcc, exec, flat_scratch_all) were a parallel encoding
of the same physical state as the _lo/_hi halves, distinguished only by width and
sharing the _lo half's index. They were absent from every map that consumes
registers (liveness index, register conversion, register space), so any use of
them silently misbehaved -- and the input data that feeds the register/decoder
generator was itself inconsistent across architectures:
* decodeOPR_VCC returned vcc_lo on gfx908 but the combined vcc on gfx90a/gfx940,
so a 64-bit VCC operand decoded to [vcc_lo,vcc_hi] on one arch and to an
unmapped combined register on the others (dropping VCC from liveness there).
* decodeOPR_FLAT_SCRATCH returned flat_scratch_all on all three arches.
* flat_scratch_lo was declared BITS_64 on gfx908/gfx940 but BITS_32 on gfx90a.
Changes:
* Decoders now emit the _lo half (appendOPR expands it to [lo,hi]) for VCC and
FLAT_SCRATCH on all three architectures -- uniform behavior.
* flat_scratch_lo is BITS_32 everywhere.
* Remove the vcc/exec/flat_scratch_all definitions and their now-dead cases in
MachRegister.C's type-query switches (the _lo/_hi halves already carry the
control-status classification).
* Unit tests: base_registers now expects each half to be its own base; the
combined-register checks are dropped.
With the combined registers gone, any future code (or generator data) that tries
to use a whole special register fails to compile instead of silently corrupting
liveness. Validated: dyninst builds clean, all six amdgpu MachRegister unit tests
pass, VCC liveness correct, instrumentation pipeline PASSES N=256/1024.
Co-Authored-By: Claude Opus 4.8 <noreply@xxxxxxxxxxxxx>
Compare: https://github.com/dyninst/dyninst/compare/7f3184752db6%5E...52cf1115f98b
To unsubscribe from these emails, change your notification settings at https://github.com/dyninst/dyninst/settings/notifications
|