Inconsistent AlignToPtr use

[User4martin]

Getting the next opcode is sometimes done with PRENextOff(AlignToPtr(scan + REOpSz))^ and sometimes Inc(s, REOpSz + RENextOffSz)

It doesn't fail on many arch:

function AlignToPtr(const p: Pointer): Pointer; {$IFDEF InlineFuncs}inline;{$ENDIF}
begin
  {$IFDEF FPC_REQUIRES_PROPER_ALIGNMENT}
  Result := Align(p, SizeOf(Pointer));
  {$ELSE}
  Result := p;
  {$ENDIF}
end;

But compile on an arch that requires align, and it should fail

[Alexey-T]

Correct note. But i don't have the hardware with required align, so my fix will be blind, w/o tests... so I don't want to fix it.

[User4martin]

Just remove the IFDEF (for testing).

Your intel/amd will not complain about the extra alignment.

[Alexey-T]

I will try it after #303 is merged...

[Alexey-T]

@User4martin Will you help me with this issue, please? to do it you need to enable the $define here:

// Alexey T.: handling of that define FPC_REQUIRES_PROPER_ALIGNMENT was present even 15 years ago,
// but with it, we have failing of some RegEx tests, on ARM64 CPU.
// If I undefine FPC_REQUIRES_PROPER_ALIGNMENT, all tests run OK on ARM64 again.
{$undef FPC_REQUIRES_PROPER_ALIGNMENT}

after fix done, that block needs to be removed.

[User4martin]

Disclaimer: Someone with background on different CPU/arch should maybe double check some of my statements....

---

Well, right now, with that define enabled, it will fail on all platforms.

My understanding is that there are some CPU that can not access a longint unless it is 32 bit aligned (or a int64 unless it is 64bit aligned.)

Currently I would expect TRegExpr to fail on such CPU. Code like

      OpKind_Char:
        begin
          Inc(ABuffer);
          N := PLongInt(ABuffer)^;

is likely to fail, because ABuffer may not be aligned.

It could also be that on some CPU such code runs slower, due to the non align (though then, its a gamble versus needing more cache lines).

Those CPU are probably mostly embedded/RISC. (Afaik they may cause an exception, if the address is not aligned / so I really am not sure) So in order to know, if this define is needed after all => it would need to be tested on those cpu.

Afaik, fpc has a copy of this, does it have testcases? I would imagine the fpc team running there tests on various cpu.

---

As for the question if it may have a speed impact => maybe a CPU expert from the fpc team knows. (just testing is likely not revealing much, as things like cache hits may cause timings to vary to much.)

---

---

So first thing is to find out:

• which target arch are currently broken (if any)
• maybe, if it could have a speed impact (though I would defer that part)
• If then fixing is urgent or not.

If it isn't urgent, more than half of the code that needs to be fixed could probably be replaced by code that doesn't need alignment on any architecture. (E.g. with a few exceptions the "next pointer" can be ReChar)

---

In either case, fixing the alignment code seems to be closer to rewriting it. So I wont be able to squeeze that in right now.

[Alexey-T]

@User4martin Any ideas for future PRs? besides the AlignToPtr. If no, let's suggest our new version to FPC bugtracker?

[User4martin]

I think this issue can be deferred.

I may have other stuff later this year... Not sure yet.

[Alexey-T]

Let's suggest our new version to FPC bugtracker? who should do it: you / me?

[User4martin]

if you would.