[fpc-devel] Anyone an idea were/how to look for the missing merge in 3.2.0 [[peephole / fixed]]

[Yuriy Sydorov]

On 04.10.2021 4:44, Martin Frb via fpc-devel wrote:
> Update to my original mail: https://lists.freepascal.org/pipermail/fpc-devel/2021-June/043884.html
> I found the needle in the haystack.
> 
> It appears fixed in current (2 day old) fixes 3.3.1. / I have not tested trunk.
> -------------------------
> 
> The code is in: components\lazutils\lazcollections.pas
> 
> procedure TLazFifoQueue.Grow(ADelta: integer);
> var
>    NewList: array of T;
>    c: Integer;
>    i: QWord;
> begin
>    c:=Max(FQueueSize + ADelta, Integer(FTotalItemsPushed - FTotalItemsPopped));
>    setlength(NewList{%H-}, c);
>    i:=FTotalItemsPopped;
>    while i < FTotalItemsPushed do begin
>      NewList[i mod c] := FList[i mod FQueueSize];
>      inc(i);
>    end;
> 
>    FList := NewList;
>    FQueueSize:=c;
> end;
> 
> Without peephole (the "max" is inlined, below is the compare code)
> # Var c located in register ebx
> 
>      cmpq    %rax,%rcx
>      jl    .Lj37
>      jmp    .Lj38
> .Lj37:
>      jmp    .Lj39
> .Lj38:
>      movq    %rcx,%rax
> .Lj39:
>      movl    %eax,%ebx
>      movslq    %ebx,%rax
> 
> The 2 moves at the end do 2 things
> 1) extend 32 to 64 bit (afaik)
> 2) move the result to ebx
> 
> Compiled with peephole the code looks like this
>      cmpq    %rax,%rcx
>      cmovnlq    %rcx,%rax
>      movslq    %eax,%rax
> 
> The 2 moves have been combined => ebx is no longer loaded.
> 
> But ebx is used for c. And hence c has the wrong value.
> 
> <little rant>Because for some mystery SetLength does not use the register value, but loads from the stackframe, the 
> array is correctly sized. Therefore the code above never crashes. But it does not move all data to the new array. Other 
> code then accesses random memory. So my crashes always happened elsewhere. Really not helpful</little rant>
> 
> In 3.3.1 the result is
>      cmpq    %rax,%rcx
> # Peephole Optimization: JccMov2CMov
>      cmovnlq    %rcx,%rax
>      movl    %eax,%ebx
> # Peephole Optimization: %ebx = %eax; changed to minimise pipeline stall (MovXXX2MovXXX)
> .Ll23:
>      movslq    %eax,%rax
> 

Nice catch!
Indeed this code reproduces the bug consistently. I'll figure out what is needed to merge to the 3.2 branch in order to 
fix it.

Yuriy.