It seems there is a bug in 64-bit compiler, affected all versions from XE2 to XE7 (maybe earlier too)

-
It seems there is a bug in 64-bit compiler, affected all versions from XE2 to XE7 (maybe earlier too)

const
  MaxKernelSize = 30;

type
  TKernelSize = 1..MaxKernelSize;
  X = array[-MaxKernelSize..MaxKernelSize] of cardinal;

procedure TForm1.Button1Click(Sender: TObject);
var
  K: X;
  KSize: TKernelSize;
  t: cardinal;
begin
 KSize:=10;
 t:=K[-KSize];
end;

Comments

  1. Alexander SviridenkovOctober 12, 2014 at 12:40 PM

    ASM when KSize is byte

    Unit72.pas.41: t:=K[-a];
    movzx rax,byte ptr [rbp+$03]
    neg eax
    movsxd rax,rax
    mov eax,[rbp+rax*4+$0084]
    mov [rbp+$04],eax

    And when KSize is TKernelSize

    Unit72.pas.41: t:=K[-a];
    movzx rax,byte ptr [rbp+$03]
    neg eax
    mov eax,eax
    mov eax,[rbp+rax*4+$0084]
    mov [rbp+$04],eax

    So here is unnecessary mov eax, eax instead of movsxd rax, rax

    ReplyDelete
  2. Sam Shaw (蕭沖)October 14, 2014 at 2:29 AM

    The workaroud is
    begin
     KSize:=10;
     t:=K[Integer(-KSize)];
    end; 

    or 
    var
     i:Integer;
    begin
     KSize:=10;
    i := -KSize;
     t:=K[i];
    end;

    force emitting movsxd rax, rax , which preserved signed number.
    Byte is unsigned, if let its value negative and move to larger type (for instance, 32bit integer), compiler always emits preserved signed instruction.  But if type is integer, it won't.  However, strictly speaking, I think this is still a bug, can report to QC, because in 64bit addressing mode, index register is 64bit, should consider small integer (32bit) move to 64bit to preserve signed when necessary.  
    movzx rax,byte ptr [rbp+$03] // fill zeros, so no signed (i.e., positive)
    neg EAX // only effect low 32 bit, but high 32bit still fill with zero
    move eax , eax  // correct one is movsxd rax,rax

    ReplyDelete
  3. Alexander SviridenkovOctober 14, 2014 at 4:27 AM

    Sam Shaw
    Thanks, reported
    https://quality.embarcadero.com/browse/RSP-9602

    ReplyDelete

Post a Comment