Fixed miscompile bug.
This commit is contained in:
Scott Duensing
parent
52c4416b7e
commit
caeb78e0d9
3 changed files with 195 additions and 2 deletions
|
|
@ -1612,7 +1612,29 @@ EOF
|
|||
>/dev/null 2>&1; then
|
||||
die "soft-double MAME: 1.5+2.5 != 4.0 (bit pattern wrong)"
|
||||
fi
|
||||
rm -f "$cDblMame" "$oDblMame" "$oSdMame" "$binDblMame" "$oCrt0File"
|
||||
rm -f "$cDblMame" "$oDblMame" "$oSdMame" "$binDblMame"
|
||||
|
||||
# Regression: W65816SepRepCleanup's PHI-copy hoist must not move a
|
||||
# `STA slot` above an `AND/ORA/EOR/ADC/SBC/CMP_StackRel slot` read in
|
||||
# the gap. Before the fix (gap check only guarded LDA_StackRel),
|
||||
# course_support_piece miscompiled `best` to `y` at every optimized
|
||||
# -O level: the hoisted STA staged a select's TRUE value into the
|
||||
# slot that a predicate AND then read. Reported by an external SNES
|
||||
# user (gist gingerbeardman/74e25c6a). course_support_piece is
|
||||
# external + called twice so it stays out-of-line (a single inlined
|
||||
# call site hides the bug). Must return 0x00037FFF.
|
||||
log "check: MAME runs boolSelectHoist regression (SepRepCleanup gap check)"
|
||||
oBshFile="$(mktemp --suffix=.o)"
|
||||
binBshFile="$(mktemp --suffix=.bin)"
|
||||
"$CLANG" --target=w65816 -O2 -ffunction-sections -c \
|
||||
"$PROJECT_ROOT/tests/regress/boolSelectHoist.c" -o "$oBshFile"
|
||||
"$PROJECT_ROOT/tools/link816" -o "$binBshFile" --text-base 0x1000 \
|
||||
"$oCrt0File" "$oBshFile" "$oLibgccFile" 2>/dev/null
|
||||
if ! bash "$PROJECT_ROOT/scripts/runInMame.sh" "$binBshFile" --check \
|
||||
0x025000=7fff 0x025002=0003 >/dev/null 2>&1; then
|
||||
die "MAME: boolSelectHoist best != 0x7FFF (SepRepCleanup hoist regression)"
|
||||
fi
|
||||
rm -f "$oBshFile" "$binBshFile" "$oCrt0File"
|
||||
fi
|
||||
|
||||
# Fuzzer: generate 20 small random C programs and verify all compile.
|
||||
|
|
|
|||
|
|
@ -605,7 +605,22 @@ bool W65816SepRepCleanup::runOnMachineFunction(MachineFunction &MF) {
|
|||
int64_t off = Back->getOperand(0).getImm();
|
||||
if (ReadSlots.count(off)) { gapOK = false; break; }
|
||||
}
|
||||
if (BO == W65816::LDA_StackRel &&
|
||||
// Any *_StackRel that READS its slot operand — not just LDA.
|
||||
// AND/ORA/EOR/ADC/SBC/CMP_StackRel all take op0 as a stack
|
||||
// slot and read it. Hoisting a Block/Trailing STA above such
|
||||
// a read of a WriteSlot makes the read observe the hoisted NEW
|
||||
// value instead of the value live at its original position.
|
||||
// This was the course_support_piece miscompile: an
|
||||
// `AND_StackRel 47` (the boolean operand of a select predicate)
|
||||
// sat in the gap while the wrap's trailing `STA_StackRel 47`
|
||||
// staged the best-select TRUE value; hoisting that STA above
|
||||
// the AND fed the AND the true value (y) instead of the
|
||||
// boolean, so `best` wrongly became y at all optimized -O
|
||||
// levels. (Previously only LDA_StackRel was checked here.)
|
||||
if ((BO == W65816::LDA_StackRel || BO == W65816::AND_StackRel ||
|
||||
BO == W65816::ORA_StackRel || BO == W65816::EOR_StackRel ||
|
||||
BO == W65816::ADC_StackRel || BO == W65816::SBC_StackRel ||
|
||||
BO == W65816::CMP_StackRel) &&
|
||||
Back->getNumOperands() >= 1 && Back->getOperand(0).isImm()) {
|
||||
int64_t off = Back->getOperand(0).getImm();
|
||||
if (WriteSlots.count(off)) { gapOK = false; break; }
|
||||
|
|
|
|||
156
tests/regress/boolSelectHoist.c
Normal file
156
tests/regress/boolSelectHoist.c
Normal file
|
|
@ -0,0 +1,156 @@
|
|||
// Regression test for the W65816 SepRepCleanup PHI-copy-hoist fix
|
||||
// (W65816SepRepCleanup.cpp gap conflict-check).
|
||||
//
|
||||
// Reported by an external user targeting the SNES (gist gingerbeardman/
|
||||
// 74e25c6a). SepRepCleanup's "sink LDA;STA out of a PHP/PLP flag-wrap"
|
||||
// peephole hoisted a wrap's trailing `STA_StackRel N` (staging a select's
|
||||
// TRUE value `y`) ABOVE an `AND_StackRel N` in the gap (the select
|
||||
// predicate's boolean operand) -- its conflict check only treated
|
||||
// LDA_StackRel as a slot read, missing AND/ORA/EOR/ADC/SBC/CMP_StackRel.
|
||||
// The AND then read `y` instead of the boolean, so `best` wrongly became
|
||||
// `y` at every optimized -O level.
|
||||
//
|
||||
// course_support_piece(&gCourse, 32, 250, 8, 0, 0) must return 0x00037FFF
|
||||
// (best stays 0x7FFF, pid = 3). The buggy backend returned 0x00030082
|
||||
// (best clobbered with y = 130). course_support_piece MUST stay
|
||||
// out-of-line (external + 2 call sites) -- a single inlined call site
|
||||
// changes register allocation and hides the bug. Verified in MAME.
|
||||
typedef short fx_i16;
|
||||
typedef unsigned short fx_u16;
|
||||
typedef unsigned char fx_u8;
|
||||
typedef signed char fx_i8;
|
||||
typedef unsigned long fx_u32; // 32-bit on w65816
|
||||
|
||||
#define NO_GROUND 0x7FFF
|
||||
#define MASK_NONE 0x7FFF
|
||||
#define FLAT_SLOPED 0x7FFE
|
||||
#define CTILE 16
|
||||
#define CTSHIFT 4
|
||||
#define CTMASK 15
|
||||
#define PC_GAP 0
|
||||
#define PC_CRUMBLE 2
|
||||
#define PIECE_SHIFT 4
|
||||
|
||||
#define fx_shr16(n, k) ((fx_i16)((fx_i16)(n) >> (k)))
|
||||
#define fx_abs16(n) ((fx_i16)((n) < 0 ? -(n) : (n)))
|
||||
#define rd_i16(p, i) (((const volatile fx_i16 *)(p))[(i)])
|
||||
#define rd_u16(p, i) (((const volatile fx_u16 *)(p))[(i)])
|
||||
#define rd_u8(p, i) (((const volatile fx_u8 *)(p))[(i)])
|
||||
|
||||
typedef struct {
|
||||
fx_i16 start_tcol, end_tcol;
|
||||
fx_i16 start_col, end_col;
|
||||
const fx_u16 *tmask;
|
||||
const fx_i16 *tbase;
|
||||
const fx_u8 *ground_piece;
|
||||
const fx_i16 *col_surf_flat;
|
||||
} course_layer_t;
|
||||
|
||||
typedef struct {
|
||||
int nlayers;
|
||||
const course_layer_t *layers;
|
||||
const fx_i16 *masks;
|
||||
const fx_u8 *collapsed;
|
||||
fx_i16 max_px;
|
||||
} course_t;
|
||||
|
||||
static __attribute__((noinline)) fx_i16 wall_clamp_px(const course_t *c, fx_i16 px) {
|
||||
if (px < 0) { return 0; }
|
||||
if (px > c->max_px) { return c->max_px; }
|
||||
return px;
|
||||
}
|
||||
|
||||
static fx_i16 layer_probe(const course_t *c, const course_layer_t *L,
|
||||
fx_i16 px, fx_i16 col, int li, fx_u8 *piece_out) {
|
||||
(void)li;
|
||||
fx_i16 ret; fx_u8 outpiece;
|
||||
fx_i16 tcol = fx_shr16(px, CTSHIFT);
|
||||
if (tcol < L->start_tcol || tcol >= L->end_tcol) { ret = NO_GROUND; outpiece = PC_GAP; }
|
||||
else {
|
||||
int ti = tcol - L->start_tcol;
|
||||
int in_col = (col >= L->start_col && col < L->end_col);
|
||||
int gi = col - L->start_col;
|
||||
const fx_u16 *tmask = L->tmask; const fx_i16 *tbase = L->tbase;
|
||||
const fx_u8 *gp = L->ground_piece; const fx_i16 *masks = c->masks;
|
||||
const fx_u8 *collapsed = c->collapsed; const fx_i16 *csf = L->col_surf_flat;
|
||||
fx_u8 mi = (fx_u8)(px & CTMASK); fx_u8 gpv = PC_GAP;
|
||||
{ fx_i16 fy = rd_i16(csf, ti);
|
||||
if (fy != FLAT_SLOPED) {
|
||||
if (in_col) gpv = rd_u8(gp, gi);
|
||||
if (piece_out) *piece_out = in_col ? gpv : PC_GAP;
|
||||
return fy;
|
||||
} }
|
||||
if (in_col) gpv = rd_u8(gp, gi);
|
||||
fx_u16 id = rd_u16(tmask, ti);
|
||||
if (id == 0) ret = NO_GROUND;
|
||||
else if (collapsed && in_col && collapsed[col] && gpv == PC_CRUMBLE) ret = NO_GROUND;
|
||||
else {
|
||||
fx_i16 off = rd_i16(masks, (int)id * CTILE + mi);
|
||||
ret = (off == MASK_NONE) ? NO_GROUND : (fx_i16)(rd_i16(tbase, ti) + off);
|
||||
}
|
||||
outpiece = in_col ? gpv : PC_GAP;
|
||||
}
|
||||
if (piece_out) *piece_out = outpiece;
|
||||
return ret;
|
||||
}
|
||||
|
||||
fx_u32 course_support_piece(const course_t *c, fx_i16 px, fx_i16 contactY, fx_i8 grab,
|
||||
fx_i16 belowFrom, fx_i16 *below_out) {
|
||||
fx_i16 best = NO_GROUND, bestd = (fx_i16)(grab + 1), piece_bestd = 0x7FFF;
|
||||
fx_u8 pid = PC_GAP; fx_i16 below = NO_GROUND;
|
||||
fx_i16 qpx = wall_clamp_px(c, px);
|
||||
fx_i16 col = fx_shr16(qpx, PIECE_SHIFT);
|
||||
for (int li = 0; li < c->nlayers; li++) {
|
||||
const course_layer_t *L = &c->layers[li];
|
||||
fx_u8 pc; fx_i16 y = layer_probe(c, L, qpx, col, li, &pc);
|
||||
if (y != NO_GROUND) {
|
||||
fx_i16 d = fx_abs16((fx_i16)(y - contactY));
|
||||
if (d <= grab && d < bestd) { best = y; bestd = d; }
|
||||
if (d < piece_bestd) { piece_bestd = d; pid = pc; }
|
||||
if (below_out) { if (y >= belowFrom && y < below) below = y; }
|
||||
else if (d == 0) return (fx_u32)(fx_u16)y | ((fx_u32)pc << 16);
|
||||
}
|
||||
}
|
||||
if (below_out) *below_out = below;
|
||||
return (fx_u32)(fx_u16)best | ((fx_u32)pid << 16);
|
||||
}
|
||||
|
||||
#define FS FLAT_SLOPED
|
||||
static const fx_u16 tmask0[16] = {0,1,2,3, 1,2,3,0, 1,2,3,1, 2,3,0,1};
|
||||
static const fx_i16 tbase0[16] = {100,110,120,130, 140,150,160,170, 180,190,200,210, 220,230,240,250};
|
||||
static const fx_u8 gp0[16] = {1,2,3,4, 5,6,7,8, 1,2,3,4, 5,6,7,8};
|
||||
static const fx_i16 csf0[16] = {FS,250,FS,260, FS,FS,250,FS, 300,FS,FS,250, FS,FS,FS,FS};
|
||||
static const fx_u16 tmask1[16] = {1,0,2,0, 3,1,0,2, 0,3,1,0, 2,0,3,1};
|
||||
static const fx_i16 tbase1[16] = {200,205,210,215, 220,225,230,235, 240,245,250,255, 260,265,270,275};
|
||||
static const fx_u8 gp1[16] = {2,2,2,2, 2,2,2,2, 3,3,3,3, 1,1,1,1};
|
||||
static const fx_i16 csf1[16] = {FS,FS,FS,FS, 250,FS,FS,FS, FS,240,FS,FS, FS,FS,250,FS};
|
||||
static const fx_u16 tmask2[16] = {3,3,0,1, 2,3,0,1, 2,3,0,1, 2,3,0,0};
|
||||
static const fx_i16 tbase2[16] = {150,150,150,150, 160,160,160,160, 170,170,170,170, 180,180,180,180};
|
||||
static const fx_u8 gp2[16] = {4,4,4,4, 5,5,5,5, 6,6,6,6, 7,7,7,7};
|
||||
static const fx_i16 csf2[16] = {FS,FS,FS,FS, FS,FS,FS,FS, FS,FS,FS,FS, FS,FS,FS,FS};
|
||||
#undef FS
|
||||
static const fx_i16 masks[64] = {
|
||||
0,0,0,0, 0,0,0,0, 0,0,0,0, 0,0,0,0,
|
||||
0,1,2,3, 4,5,6,7, MASK_NONE,0,1,2, 3,4,5,6,
|
||||
10,11,MASK_NONE,13, 14,15,16,17, 18,MASK_NONE,20,21, 22,23,24,25,
|
||||
MASK_NONE,1,2,MASK_NONE, 4,5,6,7, 8,9,10,11, 12,13,14,15
|
||||
};
|
||||
static const fx_u8 collapsed[16] = {0,0,1,0, 0,1,0,0, 1,0,0,1, 0,0,1,0};
|
||||
static const course_layer_t layers[3] = {
|
||||
{ 0,16, 0,16, tmask0, tbase0, gp0, csf0 },
|
||||
{ 0,16, 0,16, tmask1, tbase1, gp1, csf1 },
|
||||
{ 0,16, 0,16, tmask2, tbase2, gp2, csf2 },
|
||||
};
|
||||
static const course_t gCourse = { 3, layers, masks, collapsed, 255 };
|
||||
|
||||
// Two call sites keep course_support_piece out-of-line (it is far over the
|
||||
// inline threshold), reproducing the real function's register pressure.
|
||||
int main(void) {
|
||||
fx_u32 r = course_support_piece(&gCourse, 32, 250, 8, 0, 0);
|
||||
fx_u32 r2 = course_support_piece(&gCourse, 48, 260, 8, 0, 0);
|
||||
*(volatile unsigned short *)0x025000 = (unsigned short)(r & 0xFFFF);
|
||||
*(volatile unsigned short *)0x025002 = (unsigned short)((r >> 16) & 0xFFFF);
|
||||
*(volatile unsigned short *)0x025004 = (unsigned short)(r2 & 0xFFFF);
|
||||
while (1) {}
|
||||
return 0;
|
||||
}
|
||||
Loading…
Add table
Reference in a new issue