I don't have an M1 myself, but according to the data from the thread on the lazarus mail list, there is a bug in the 3.3.1 asm generator for M1

var pn8: pint8; // pointer signed byte

In the below expression ...(not pn8^)...

"pn8^" is loaded to w0 and sign extended. From this point onwards operations on the value should be 32 bits (the value has been extended, and the full 32 bits are later used).
but "not" only affects the lowest 8 bit.

Apparently in 3.2.2 (or was it 3.2.0) there was
mvn w0,w0

If someone can confirm tihs....

-------- Forwarded Message --------
Subject: [Lazarus] fpc bug with M1 [[was: Re: UTF8LengthFast returning incorrect results on AARCH64 (MacOS)]]
Date: Tue, 28 Dec 2021 22:47:53 +0100
From: Martin Frb via lazarus
Reply-To: Lazarus mailing list
To:
CC: Martin Frb

# [43] Result += (pn8^ shr 7) and ((not pn8^) shr 6);     ldr    x0,[sp]
    ldrsb    w0,[x0]         # <<<<< sign extend to a 32bit value (32bit register).
    eor    w0,w0,#255   # <<<<< But only "not" the lowest 8 bit. That is wrong. The calculation uses 32 bit at this point     lsr    w0,w0,#6
    ldr    x2,[sp]
    ldrsb    w2,[x2]
    lsr    w2,w2,#7
    and    w0,w2,w0    # <<<<<< here the full 32 bit are used.

It's probably caused by c90616944d3bde7b36e924d27a0790195d61f95c (Florian)

Jonas

Isn't the sign extension during the load wrong? Martin didn't post the whole assemble code but I would expect that 3.2.2 produced an uxtb instruction afterwards which hide the problem.

No, 3.2.2 afterwards sign-extendeds the result as a signed rather than unsigned value:

# [29] Result += (pn8^ shr 7) and ((not pn8^) shr 6); ldr x0,[sp]
ldrsb w0,[x0]
mvn w0,w0
sxtb w0,w0
lsr w1,w0,#6
ldr x0,[sp]
ldrsb w0,[x0]
lsr w0,w0,#7
and w0,w0,w1
sxtw x0,w0
ldr x1,[sp, #16]
add x0,x1,x0
str x0,[sp, #16]

I admit I don't know/remember where this is done though, as AArch64's a_load_ref_reg doesn't seem to have any special logic that would do this.

Jonas

Ah yes, or like this. Nevertheless, the question is whether the ldrsb w0,[x0] is correct or not.

The code is from the "old" LazUtils  Utf8LengthFast. "old" => about a week back, since it got recently changed to uint8.

function UTF8LengthFast(p: PChar; ByteCount: PtrInt): PtrInt;
var
   pnx: PPtrInt absolute p; // To get contents of text in PtrInt blocks.
x refers to 32 or 64 bits
   pn8: pint8 absolute pnx; // To read text as Int8 in the initial and
final loops
begin
....
     Result += (pn8^ shr 7) and ((not pn8^) shr 6);

It is about the  "((not pn8^) shr 6)" part.

For X86 the "not" is byte only, then sign extend, then shift (interesting, that the value for a logical shift is sign extended.)

Project1.pas:276                          Result += (pn8^ shr 7) and ((not pn8^) shr 6);
0000000100001B30 488b45f8                 mov    -0x8(%rbp),%rax 0000000100001B34 8a00                     mov    (%rax),%al 0000000100001B36 f6d0                     not    %al 0000000100001B38 0fbec0                   movsbl %al,%eax 0000000100001B3B c1e806                   shr    $0x6,%eax 0000000100001B3E 488b55f8                 mov    -0x8(%rbp),%rdx 0000000100001B42 0fbe12                   movsbl (%rdx),%edx 0000000100001B45 c1ea07                   shr    $0x7,%edx 0000000100001B48 21d0                     and    %edx,%eax 0000000100001B4A 4863c0                   movslq %eax,%rax 0000000100001B4D 480345e8                 add    -0x18(%rbp),%rax 0000000100001B51 488945e8                 mov    %rax,-0x18(%rbp)

Florian Klämpfl wrote

Ah yes, or like this. Nevertheless, the question is whether the ldrsb w0,[x0] is correct or not.

Yes, I was unclear: with the "I don't know/remember where this is done" I meant "changing the load of the unsigned byte type into a signed load". I can't think immediately of a reason either why this is done.

Jonas

"unsigned byte"? The pointer in the pascal code is a pint8 => signed.

Oh, I thought it was puint8. Then it makes sense.
c90616944d3bde7b36e924d27a0790195d61f95c applies both to OS_8 and OS_S8.

Jonas

It got changed to uint8 after the issue surfaced. From what the code does, uint8 makes more sense anyway.

But the asm code with the sign extension, is the one from signed int8.

Well at least according to the person who sent me this (they sent me both versions, and that one is supposed to be the int8 one)

Yes, but the question is: if we load a shortint into a register, do we need to sign extend it to 32/64 bit or not? I tend more and more to say that we shouldn’t require this.

Neither clang nor gcc seem to expect this for arguments/return values: https://godbolt.org/z/sv5fPP6GM

This is not related to arguments/return values. We do the same on on PPC, and afaik on all architectures that don't have 8/16 bit subregisters. I initially did it on PPC because it simplified code generation a lot and solved all kinds of small issues I got otherwise because non-cleared higher parts of registers were used. Maybe with our current code generators it would work better.

Jonas

Actually, the main reason may have been register variables: in the code generator you don't know whether a register is a register variable or not. So if you don't sign/zero extend values on initialising a register, then you have to zero/sign extend them every time you read them. And reading generally happens a lot more often than writing.

Jonas

Well the issue is, that at some time it needs to be extended, as the full expression is

  Result := Result + (pn8^ shr 7) and ((not pn8^) shr 6);

Result is either 64 or 32 depending on architecture. pn8 is pint8

On intel
- only the byte is loaded. "Not" applies to the byte only.
- Then on intel the value is sign extended, even though an (logical) "shr 6" could be done on the byte.
- For the addition, sign extension is needed.

That means that due to the early sign extension, the bytes 8..31 (or 8..64) before the shift (or 2..25 after) might be set to 1.

Since shr is not arithmetic, and does not keep/fill the "sign bit", I do not see why a sign extension is needed at that point. (But that is another issue)

It's because, as mentioned earlier, in Pascal virtually all operations are performed using the native integer type. Converting a signed byte to the native integer type requires a sign extension to keep the value equal.

Jonas

Yes and no.

Gcc compiles

unsigned char f(int i)
{

    return i;

}

into a single ret.

This means, that we cannot assume that the upper part of registers is cleared. Of course, we could limit this to function calls but I guess doing it consistently is easier to handle.