<!-- See available text formatting: https://gitlab.com/help/user/markdown.md --> ## Summary TryConstMerge causes a bug by incorrect checking variable. ## System Information <!-- The more information are provided the easier it is to replicate the bug --> - **Operating system:** Windows - **Processor architecture:** X86-64 - **Compiler version:** trunk - **Device:** PC ## Steps to reproduce Compile and test from O- to O4 using the example below. ## Example Project ```pascal { Regression test for x86 OptPass1MOV / TryConstMerge (compiler/x86/aoptx86.pas). Constant stores to the stack that are NOT adjacent must not be merged into one wider store, because that overwrites the bytes in between. TryConstMerge selected the merge width from the outer instruction (p) instead of the pair being merged (p1/p2). Run with -O- -O2 -O3 -O4. Exit code 0 = OK, otherwise number of the failed test. } {$mode objfpc} {$R-}{$Q-} var failed: longint = 0; procedure check(const a: array of byte; const expect: array of byte; id: longint); noinline; var i: longint; begin for i := 0 to high(expect) do if a[i] <> expect[i] then begin writeln('FAIL ', id, ': byte ', i, ' is ', a[i], ', expected ', expect[i]); if failed = 0 then failed := id; exit; end; end; { byte stores at 0,1 (merged to a word), then at 4 and 6 (must not become a dword store) } procedure t1; noinline; var r: array[0..7] of byte; begin r[5] := $55; r[7] := $77; r[2] := $22; r[3] := $33; r[0] := 1; r[1] := 2; r[4] := 3; r[6] := 4; check(r, [1, 2, $22, $33, 3, $55, 4, $77], 1); end; { word stores at 0,2 (merged to a dword), then at 8 and 12 (second one is 0, so the merged constant would still fit into 32 bits) } procedure t2; noinline; var r: array[0..15] of byte; begin r[4] := 0; r[5] := 0; r[6] := 0; r[7] := 0; r[10] := $AA; r[11] := $BB; r[14] := $CC; r[15] := $DD; PWord(@r[0])^ := $0304; PWord(@r[2])^ := $0102; PWord(@r[8])^ := $1111; PWord(@r[12])^ := 0; check(r, [4, 3, 2, 1, 0, 0, 0, 0, $11, $11, $AA, $BB, 0, 0, $CC, $DD], 2); end; { the intended merges of directly adjacent stores must still give the right bytes } procedure t3; noinline; var r: array[0..7] of byte; begin r[0] := 1; r[1] := 2; r[2] := 3; r[3] := 4; r[4] := 5; r[5] := 6; r[6] := 7; r[7] := 8; check(r, [1, 2, 3, 4, 5, 6, 7, 8], 3); end; begin t1; t2; t3; if failed = 0 then writeln('OK'); halt(failed); end. ``` [constmerge_bug.pp](/uploads/7e59096fd911db82a6e063300de4a1e0/constmerge_bug.pp) ## What is the current bug behavior? ``` ./tryconstmerge_bug OK ./tryconstmerge_bug FAIL 1: byte 5 is 0, expected 85 ./tryconstmerge_bug FAIL 1: byte 5 is 0, expected 85 FAIL 2: byte 10 is 0, expected 170 ./tryconstmerge_bug FAIL 1: byte 5 is 0, expected 85 FAIL 2: byte 10 is 0, expected 170 ./tryconstmerge_bug FAIL 1: byte 5 is 0, expected 85 FAIL 2: byte 10 is 0, expected 170 ``` ## What is the expected (correct) behavior? ``` ./tryconstmerge_bug OK ./tryconstmerge_bug OK ./tryconstmerge_bug OK ./tryconstmerge_bug OK ./tryconstmerge_bug OK ``` ## Relevant logs and/or screenshots <!-- Paste any relevant logs - please use code blocks (```) to format console output, logs, and code --> ## Possible fixes ```patch --- a/compiler/x86/aoptx86.pas +++ b/compiler/x86/aoptx86.pas @@ -3349,7 +3349,9 @@ (ThisRef.base = current_procinfo.framepointer) ) then begin - case taicpu(p).opsize of + { Must use p1, not p: when called as TryConstMerge(hp1, hp2) the + size of the instructions being merged is not that of p } + case taicpu(p1).opsize of S_B: begin { Word writes must be on a 2-byte boundary } ``` [aoptx86_TryConstMerge_fix.diff](/uploads/df87267502ab8e45ac55a73241c37e45/aoptx86_TryConstMerge_fix.diff)