Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 5 additions & 1 deletion llvm/lib/Target/AIE/AIE2InstructionSelector.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -640,7 +640,11 @@ AIE2InstructionSelector::getCombinedOpcodeUNPACKLoad(
Intrinsic::aie2_unpack_I16_I8))
return {};

if (!MemOp.mayLoad())
// The unpack may be fed by an op that reports mayLoad() but is not an actual
// memory access (e.g. the SRS narrowing intrinsic that lowers
// acc.to_vector<int8>()). Such ops carry no memory operand, so bail out
// before getLoadStoreSize() dereferences a non-existent one.
if (!MemOp.mayLoad() || MemOp.memoperands_empty())

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you tried AIE2PS? If it doesn't occur there, it may mean that the memory operand modeling/handling is better there.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@martien-de-jong Verified — AIE2P crashes identically before this fix. The guard logic in AIE2PInstructionSelector::getCombinedOpcodeUNPACKLoad was if (!MemOp.mayLoad()) return {}; with no memoperands_empty() check, so the same null-deref SIGSEGV occurs. I confirmed this by running the new inst-select-unpack-non-load.mir test against an unpatched build (exit 139 / SIGSEGV) and a patched build (exit 0, selects VUNPACK_mv_unpack_w_unpackSign1).

return {};

assert(getLoadStoreSize(MemOp) == 256 && "Unexpected VLDA.UNPACK size");
Expand Down
6 changes: 5 additions & 1 deletion llvm/lib/Target/AIE/aie2p/AIE2PInstructionSelector.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2747,7 +2747,11 @@ AIE2PInstructionSelector::getCombinedOpcodeUNPACKLoad(
CombOpInstID != Intrinsic::aie2p_unpack_I1024_I8_I4))
return {};

if (!MemOp.mayLoad())
// The unpack may be fed by an op that reports mayLoad() but is not an actual
// memory access (e.g. the SRS narrowing intrinsic that lowers
// acc.to_vector<int8>()). Such ops carry no memory operand, so bail out
// before getLoadStoreSize() dereferences a non-existent one.
if (!MemOp.mayLoad() || MemOp.memoperands_empty())
return {};

assert(((getLoadStoreSize(MemOp) == 256 &&
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
#
# This file is licensed under the Apache License v2.0 with LLVM Exceptions.
# See https://llvm.org/LICENSE.txt for license information.
# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
#
# (c) Copyright 2026 Advanced Micro Devices, Inc. or its affiliates

# RUN: llc -mtriple aie2 -run-pass=instruction-select %s -verify-machineinstrs -o - | FileCheck %s

# The VLDB.UNPACK combine in selectG_AIE_LOAD_UNPACK must only fire when the
# def feeding the unpack is an actual load. Here the unpack consumes the result
# of an SRS narrowing intrinsic (the lowering of acc.to_vector<int8>()), which
# reports mayLoad() but carries no memory operand. Previously the combine path
# called getLoadStoreSize() on it and dereferenced a non-existent memoperand,
# crashing instruction-select. It must instead fall back to the normal VUNPACK
# selection.

---
name: unpack_of_srs_not_load
alignment: 16
legalized: true
regBankSelected: true
body: |
bb.1.entry:
liveins: $p0
; CHECK-LABEL: name: unpack_of_srs_not_load
; CHECK: liveins: $p0
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: [[COPY:%[0-9]+]]:ep = COPY $p0
; CHECK-NEXT: [[DEF:%[0-9]+]]:acc1024 = IMPLICIT_DEF
; CHECK-NEXT: [[MOV_RLC_imm10_pseudo:%[0-9]+]]:er = MOV_RLC_imm10_pseudo 8
; CHECK-NEXT: [[COPY1:%[0-9]+]]:mss = COPY [[MOV_RLC_imm10_pseudo]]
; CHECK-NEXT: [[VSRS_S8_S32_mv_w_srs:%[0-9]+]]:vec256 = VSRS_S8_S32_mv_w_srs [[DEF]], [[COPY1]], implicit-def dead $srsrs_of, implicit $crsat, implicit $crrnd
; CHECK-NEXT: [[VUNPACK_S16_S8_:%[0-9]+]]:vec512 = VUNPACK_S16_S8 [[VSRS_S8_S32_mv_w_srs]]
; CHECK-NEXT: [[COPY2:%[0-9]+]]:vec256 = COPY [[VUNPACK_S16_S8_]].sub_256_lo
; CHECK-NEXT: [[COPY3:%[0-9]+]]:vec256 = COPY [[VUNPACK_S16_S8_]].sub_256_hi
; CHECK-NEXT: VST_dmw_sts_w_ag_idx_imm [[COPY3]], [[COPY]], 32 :: (store (<16 x s16>) into unknown-address + 32)
; CHECK-NEXT: VST_dmw_sts_w_ag_idx_imm [[COPY2]], [[COPY]], 0 :: (store (<16 x s16>), align 64)
; CHECK-NEXT: PseudoRET implicit $lr
%0:ptrregbank(p0) = COPY $p0
%1:accregbank(<16 x s64>) = G_IMPLICIT_DEF
%2:gprregbank(s32) = G_CONSTANT i32 8
%3:gprregbank(s32) = G_CONSTANT i32 1
%4:vregbank(<32 x s8>) = G_INTRINSIC_W_SIDE_EFFECTS intrinsic(@llvm.aie2.I256.v32.acc32.srs), %1(<16 x s64>), %2(s32), %3(s32)
%5:vregbank(<32 x s16>) = G_INTRINSIC intrinsic(@llvm.aie2.unpack.I16.I8), %4(<32 x s8>), %3(s32)
G_STORE %5(<32 x s16>), %0(p0) :: (store (<32 x s16>))
PseudoRET implicit $lr
...
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
#
# This file is licensed under the Apache License v2.0 with LLVM Exceptions.
# See https://llvm.org/LICENSE.txt for license information.
# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
#
# (c) Copyright 2026 Advanced Micro Devices, Inc. or its affiliates

# RUN: llc -mtriple aie2p -run-pass=instruction-select %s -verify-machineinstrs -o - | FileCheck %s

# The VLDB.UNPACK combine in selectG_AIE_LOAD_UNPACK must only fire when the
# def feeding the unpack is an actual load. Here the unpack consumes the result
# of an SRS narrowing intrinsic (the lowering of acc.to_vector<int8>()), which
# reports mayLoad() but carries no memory operand. Previously the combine path
# called getLoadStoreSize() on it and dereferenced a non-existent memoperand,
# crashing instruction-select. It must instead fall back to the normal VUNPACK
# selection.

---
name: unpack_of_srs_not_load
alignment: 16
legalized: true
regBankSelected: true
body: |
bb.1.entry:
liveins: $p0
; CHECK-LABEL: name: unpack_of_srs_not_load
; CHECK: liveins: $p0
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: [[COPY:%[0-9]+]]:ep = COPY $p0
; CHECK-NEXT: [[DEF:%[0-9]+]]:acc1024 = IMPLICIT_DEF
; CHECK-NEXT: [[MOV_RLC_imm11_pseudo:%[0-9]+]]:er = MOV_RLC_imm11_pseudo 8
; CHECK-NEXT: $crsrsmode = MOV_scalar_imm11_pseudo 0
; CHECK-NEXT: [[COPY1:%[0-9]+]]:es = COPY [[MOV_RLC_imm11_pseudo]]
; CHECK-NEXT: [[VSRS_4x_mv_w_srs_cm_srsSign1_:%[0-9]+]]:vec256 = VSRS_4x_mv_w_srs_cm_srsSign1 [[DEF]], [[COPY1]], implicit-def dead $srsrs_of, implicit $crrnd, implicit $crsrsmode, implicit $crsat, implicit $srssign1
; CHECK-NEXT: $crunpacksize = MOV_scalar_imm11_pseudo 1
; CHECK-NEXT: [[VUNPACK_mv_unpack_w_unpackSign1_:%[0-9]+]]:vec512 = VUNPACK_mv_unpack_w_unpackSign1 [[VSRS_4x_mv_w_srs_cm_srsSign1_]], implicit $crunpacksize, implicit $unpacksign1
; CHECK-NEXT: VST_dmx_sts_x_idx_imm [[VUNPACK_mv_unpack_w_unpackSign1_]], [[COPY]], 0 :: (store (<32 x s16>))
; CHECK-NEXT: PseudoRET implicit $lr
%0:ptrregbank(p0) = COPY $p0
%1:accregbank(<32 x s32>) = G_IMPLICIT_DEF
%2:gprregbank(s32) = G_CONSTANT i32 8
%3:gprregbank(s32) = G_CONSTANT i32 1
%4:vregbank(<32 x s8>) = G_INTRINSIC_W_SIDE_EFFECTS intrinsic(@llvm.aie2p.I256.v32.acc32.srs), %1(<32 x s32>), %2(s32), %3(s32)
%5:vregbank(<32 x s16>) = G_INTRINSIC intrinsic(@llvm.aie2p.unpack.I512.I16.I8), %4(<32 x s8>), %3(s32)
G_STORE %5(<32 x s16>), %0(p0) :: (store (<32 x s16>))
PseudoRET implicit $lr
...
Loading