fix(codegen): float field read via implicit Self + float-returning method call
Two float-expression gaps, both fixed on native and QBE:
1. Implicit-Self float field READ (Result := FFloat inside a method).
- native: EmitExprToXmm0 had no implicit-Self field case, so the field fell
through to the plain VarOperand path and emitted `movsd FFloat(%rip)` — a
load from a global symbol named after the field (undefined in per-unit .o
codegen). Now loads from Self + field offset. (The store path was
already correct; only the read was missing it.)
- qbe: the implicit-Self field read in EmitExpr hardcoded `loadl` for any
non-word field, so a Double/Single field loaded with loadl into an l temp
and the following `stored`/`ret d` rejected it. Now uses the field type's
load opcode (loadd/loads) with the matching result type.
2. Qualified float-returning method call as a float expression
(d := Obj.GetF()). native EmitExprToXmm0 had no TMethodCallExpr case and
raised "unsupported float expression form"; now dispatches the method (the
SysV ABI leaves the float result in %xmm0).
e2e test in cp.test.e2e.sepcompile compiles a unit with a Double field read via
implicit Self to a .o and runs a consumer over it (the per-unit path that
exposed the native symbol bug), printing the float method-call result on both
backends. Full suite OK (3740); all four fixpoints pass on both build paths.
This commit is contained in:
parent
6d46af93de
commit
b074bc003a
|
|
@ -4704,6 +4704,22 @@ begin
|
|||
|
||||
if AExpr is TIdentExpr then
|
||||
begin
|
||||
{ Implicit-Self float field read (FFloat inside a method): load from the
|
||||
Self instance at the field offset, NOT from a global symbol named after
|
||||
the field. Without this the field reference falls through to the plain
|
||||
VarOperand path below and emits `movsd FFloat(%rip)` — an undefined
|
||||
symbol in per-unit (.o) codegen. (The store path already handles this;
|
||||
only the read was missing it.) }
|
||||
if TIdentExpr(AExpr).IsImplicitSelf and
|
||||
(TIdentExpr(AExpr).ImplicitFieldInfo <> nil) then
|
||||
begin
|
||||
Self.Emit(Format(#9'movq %s, %%rcx', [Self.VarOperand('Self')]));
|
||||
if TFieldInfo(TIdentExpr(AExpr).ImplicitFieldInfo).Offset > 0 then
|
||||
Self.Emit(Format(#9'addq $%d, %%rcx',
|
||||
[TFieldInfo(TIdentExpr(AExpr).ImplicitFieldInfo).Offset]));
|
||||
Self.EmitLoadFloat('(%rcx)', AExpr.ResolvedType);
|
||||
Exit;
|
||||
end;
|
||||
{ Float-typed named constant (const X = 6.28; ...): inline its literal
|
||||
value. The const has no storage — its ConstString holds the source
|
||||
text — so loading from a symbol named after it (VarOperand) would
|
||||
|
|
@ -4915,6 +4931,15 @@ begin
|
|||
Exit;
|
||||
end;
|
||||
|
||||
{ Qualified method call returning a float, e.g. d := Obj.GetF(). The method
|
||||
emitter performs vtable/static dispatch and the SysV ABI leaves the float
|
||||
result in %xmm0, so no further work is needed here. }
|
||||
if AExpr is TMethodCallExpr then
|
||||
begin
|
||||
Self.EmitMethodCallExpr(TMethodCallExpr(AExpr));
|
||||
Exit;
|
||||
end;
|
||||
|
||||
{ Float array element read A[I] (dynamic or static array of Double/Single).
|
||||
The element ADDRESS computation matches the integer EmitExprToEax subscript
|
||||
paths; only the final load differs — movsd/movss into %xmm0 rather than a
|
||||
|
|
|
|||
|
|
@ -11958,14 +11958,19 @@ begin
|
|||
if QType = 'w' then
|
||||
EmitLine(Format(' %s =w %s %s', [T, LoadInstrFor(ImplFld.TypeDesc), PtrT]))
|
||||
else
|
||||
EmitLine(Format(' %s =l loadl %s', [T, PtrT]));
|
||||
{ l / d / s — pick the load opcode by field type so a Double/Single
|
||||
field reads with loadd/loads (not loadl, which would mis-type the
|
||||
result for a later stored/ret). }
|
||||
EmitLine(Format(' %s =%s %s %s',
|
||||
[T, QType, LoadInstrFor(ImplFld.TypeDesc), PtrT]));
|
||||
end
|
||||
else
|
||||
begin
|
||||
if QType = 'w' then
|
||||
EmitLine(Format(' %s =w %s %s', [T, LoadInstrFor(ImplFld.TypeDesc), SelfT]))
|
||||
else
|
||||
EmitLine(Format(' %s =l loadl %s', [T, SelfT]));
|
||||
EmitLine(Format(' %s =%s %s %s',
|
||||
[T, QType, LoadInstrFor(ImplFld.TypeDesc), SelfT]));
|
||||
end;
|
||||
Exit(T);
|
||||
end;
|
||||
|
|
|
|||
|
|
@ -56,6 +56,13 @@ type
|
|||
runtime Makefile drives (every RTL unit is compiled `--source X.pas
|
||||
--output X.o`), so it broke `make` in runtime/. }
|
||||
procedure TestNativeIncremental_UnitUsesUnit_Compiles;
|
||||
{ Regression: a Double field read via implicit Self (Result := FFloat
|
||||
inside a method) compiled in unit-mode (.o) emitted `movsd FFloat(%rip)`
|
||||
— a load from a global symbol named after the field instead of from
|
||||
Self+offset — producing an undefined `FFloat` symbol. The store path was
|
||||
already correct; only the read was broken, and only in per-unit codegen
|
||||
(whole-program builds resolved it as a field offset). }
|
||||
procedure TestNativeIncremental_FloatFieldRead_InUnit;
|
||||
{ Regression: a unit that USES another unit only in its IMPLEMENTATION
|
||||
section. On a clean build all units are parsed, so the impl-only
|
||||
dependency's object is linked. But on an incremental REBUILD, the
|
||||
|
|
@ -680,6 +687,71 @@ begin
|
|||
AssertEquals('use_derived stdout', '42' + #10, Captured)
|
||||
end;
|
||||
|
||||
procedure TSepCompileTests.TestNativeIncremental_FloatFieldRead_InUnit;
|
||||
const
|
||||
UnitSrc =
|
||||
'''
|
||||
unit NumU;
|
||||
interface
|
||||
type
|
||||
TNum = class
|
||||
FFloat: Double;
|
||||
procedure SetF(v: Double);
|
||||
function GetF: Double;
|
||||
end;
|
||||
implementation
|
||||
procedure TNum.SetF(v: Double); begin FFloat := v; end;
|
||||
function TNum.GetF: Double; begin Result := FFloat; end;
|
||||
end.
|
||||
''';
|
||||
ProgSrc =
|
||||
'''
|
||||
program UseNum;
|
||||
uses NumU;
|
||||
var N: TNum;
|
||||
begin
|
||||
N := TNum.Create();
|
||||
N.SetF(3.5);
|
||||
WriteLn(N.GetF());
|
||||
N.Free()
|
||||
end.
|
||||
''';
|
||||
var
|
||||
UnitPas, ProgPas, ProgBin: string;
|
||||
Captured: string;
|
||||
Rc: Integer;
|
||||
begin
|
||||
if not ToolchainAvailable() then
|
||||
begin
|
||||
Fail('toolchain missing — qbe or RTL not found');
|
||||
Exit
|
||||
end;
|
||||
if not FileExists(BlaisePath()) then
|
||||
begin
|
||||
Fail('blaise binary missing at ' + BlaisePath());
|
||||
Exit
|
||||
end;
|
||||
|
||||
UnitPas := FScratch + '/NumU.pas';
|
||||
ProgPas := FScratch + '/use_num.pas';
|
||||
ProgBin := FScratch + '/use_num';
|
||||
|
||||
WriteFile(UnitPas, UnitSrc);
|
||||
WriteFile(ProgPas, ProgSrc);
|
||||
|
||||
{ Incremental (per-unit .o) is the default; NumU is compiled to numu.o, whose
|
||||
GetF must read FFloat from Self+offset, not a global symbol. }
|
||||
Rc := RunBlaise(['--source', ProgPas, '--output', ProgBin,
|
||||
'--backend', 'native',
|
||||
'--unit-path', FScratch], Captured);
|
||||
AssertEquals('blaise(use_num) exit code (out: ' + Captured + ')', 0, Rc);
|
||||
AssertTrue('use_num exists', FileExists(ProgBin));
|
||||
|
||||
Rc := RunBinary(ProgBin, Captured);
|
||||
AssertEquals('use_num exit code', 0, Rc);
|
||||
AssertEquals('use_num stdout', '3.5' + #10, Captured)
|
||||
end;
|
||||
|
||||
procedure TSepCompileTests.TestIncrementalRebuild_ImplOnlyUses_LinksDependency;
|
||||
const
|
||||
HelperSrc =
|
||||
|
|
|
|||
Loading…
Reference in a new issue