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

[J. Gareth Moreton]

I have a suspicion as to what it might be.  Can you produce the faulty 
assembly language with DEBUG_AOPTCPU so it shows the comments?  Does it 
say "Mov2Nop 3" where the missing instruction lies?

Gareth aka. Kit

P.S. Good job in spotting the fault.

On 04/10/2021 13:16, Yuriy Sydorov via fpc-devel wrote:
> 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.
> _______________________________________________
> fpc-devel maillist  -  fpc-devel at lists.freepascal.org
> https://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-devel
>

-- 
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus