[x86] Bug fix to CMP/Jcc optimisation that erroneously changed the condition
Summary
This merge request fixes a jump-chain-shortcutting optimisation bug, first identified in #39851 (closed), where the first condition was changed to match the second condition, which is incorrect - once the first condition is determined to be a subset of (or equal to) the second condition, only the destination should be changed, not the actual condition.
Additionally, some reference count fixes were made in the same optimisation function (these are a separate commit).
System
- Operating system: Microsoft Windows, almost certainly others
- Processor architecture: i386, x86_64
What is the current bug behavior?
The following code causes an assertion failure because a conditional jump in the assembly language was changed from jl
to jle
to match the second jump (a variant of the code below is present in a commit that acts as a new regression test):
program test;
{$mode delphi}
{$OPTIMIZATION LEVEL1}
{$OPTIMIZATION PEEPHOLE}
{$C+}
function Fn1: Boolean;
begin
Result := True;
end;
procedure TestCmpErr;
var
I: Integer;
begin
I := 0;
if (I < 0) or (not Fn1()) then
begin // this branch should NOT be executed
ASSERT((I <= 0) and (not Fn1()));
ExitCode := 1;
end;
end;
begin
TestCmpErr;
end.
What is the behavior after applying this patch?
The above program should compile and run successfully without ExitCode
being set to 1.
Relevant logs and/or screenshots
The compiler and RTL sources received a lot of changes in the produced assembly language, showing that this bug is actually quite prevalent and may have been causing unexpected failures elsewhere. For example, in the bufdataset unit - before:
...
.Lj293:
cmpl $0,-60(%rbp)
# Peephole Optimization: CMP/Jcc/@Lbl/CMP/Jcc -> CMP/Jcc, redirecting first jump
jne .Lj296
cmpl $0,-64(%rbp)
jng .Lj310
...
After:
...
.Lj293:
cmpl $0,-60(%rbp)
# Peephole Optimization: CMP/Jcc/@Lbl/CMP/Jcc -> CMP/Jcc, redirecting first jump
jg .Lj296
cmpl $0,-64(%rbp)
jng .Lj310
...
Originally the jump was jg .Lj292
, and the code at .Lj292
is as follows:
.Lj292:
cmpl $0,-60(%rbp)
jne .Lj296
Additional Notes
The corrected reference counts permit some new optimisations since some new dead labels have appeared. For example, in cfileutl - before:
...
testl %eax,%eax
jne .Lj240
.Lj239:
jmp .Lj236
.p2align 4,,10
.p2align 3
.Lj240:
movq (%rbx),%rdx
...
After:
...
testl %eax,%eax
je .Lj236
movq (%rbx),%rdx
...
Similarly in convutils - before:
...
cmpb $0,34(%rcx,%rdx)
# Peephole Optimization: SETcc/TEST/Jcc -> SETcc/Jcc
seteb %dl
# Peephole Optimization: CMP/Jcc/@Lbl/CMP/Jcc -> CMP/Jcc, redirecting first jump
jne .Lj148
.Lj150:
addl $1,%edi
.Lj148:
...
After:
...
cmpb $0,34(%rcx,%rdx)
# Peephole Optimization: SETcc/TEST/Jcc -> SETcc/Jcc
seteb %dl
# Peephole Optimization: CMP/Jcc/@Lbl/CMP/Jcc -> CMP/Jcc, redirecting first jump
# Peephole Optimization: JccAdd2SetccAdd
sete %cl
movzbl %cl,%ecx
addl %ecx,%edi
.Lj148:
...