[Patch] [x86] JccAdd2SetccAdd optimisation is faulty
Original Reporter info from Mantis: CuriousKit @CuriousKit
-
Reporter name: J. Gareth Moreton
Original Reporter info from Mantis: CuriousKit @CuriousKit
- Reporter name: J. Gareth Moreton
Description:
The JccAdd2SetccAdd, introduced in revision 48086 (and present as of 48115), sometimes produces bad code.
Steps to reproduce:
Build the following problem under i386-win32 with the -O3 option:
program test48086;
{$mode objfpc}{$H+}
function IsFontNameXLogicalFontDesc(const LongFontName: string): boolean;
var MinusCnt, p: integer;
begin
MinusCnt:=0;
for p:=1 to length(LongFontName) do
if LongFontName[p]='-' then inc(MinusCnt);
Result:=(MinusCnt=14);
end;
var
myfont:string;
begin
myfont:='Myfont--------------';
if IsFontNameXLogicalFontDesc(myfont) then
writeln('NO ERROR');
end.
Observe a run-time error upon execution of the generated EXE file.
Additional information:
In the assembly dump (generated with -alr):
...
.Lj8:
addl $1,%ebx
# [8] if LongFontName[p]='-' then inc(MinusCnt);
cmpb $45,-1(%eax,%ebx,1)
# Peephole Optimization: JccAdd2SetccAdd
sete %al
movzbl %al,%eax
# Register eflags released
addl %eax,%ecx
.Lj12:
cmpl %edx,%ebx
jnge .Lj8
# Register eflags,ebx,edx,eax released
.Lj7:
...
"JccAdd2SetccAdd" replaces "jne .Lj12; addl $1,%ecx" with "sete %al; movzbl %al,%eax; addl %eax,%ecx". The problem here is that it selects a register that is still in use, namely %al/%eax. If control flow returns to .Lj8 at the "jnge .Lj8" instruction, the "cmpl" instruction that follows attempts to dereference an address that uses %eax and now contains an invalid value (either 0 or 1).. - in the sample program, %eax contained an address, while %ebx is being used as an index.
Mantis conversion info:
- Mantis ID: 38334
- OS: Microsoft Windows
- OS Build: 10 Home
- Build: r48086
- Platform: i386
- Version: 3.3.1
- Fixed in version: 3.3.1
- Fixed in revision: 48124 (#eb81b981)