I just updated to fpc trunk 45658

It seems sometime in the last 3 or 4 month a change was made that leads to "parentfp" being optimized away.
Even with -O- or -O1 (many users do O1 for debugging)

This happens if the nested proc does not access data in the outer procedure. So the code indeed does not need it.
But a user debugging the code may well want to see the content of those vars.

There still is dwarf info, though it points to some register (I have seen rax and rdx).
While normally it would point to an offset in the current frame, where the outer frame is stored.

I think fpc should keep the parentfp for debugging (in O1)? If agreed, I report a bug.
If not agreed what alternatives are proposed?

Or ideally (but that would also need the parentfp's value) provide dwarf entries for each of outer variable that is in scope, using location expressions to directly point to the date.

Regards
Martin

Martin wrote

I just updated to fpc trunk 45658 It seems sometime in the last 3 or 4 month a change was made that leads to "parentfp" being optimized away. Even with -O- or -O1 (many users do O1 for debugging)

I've turned off optimization of parentfp when -O- in r45661.

Yuriy.

Maybe it can be tied to the -Oostackframe optimisation setting.

Jonas

Some CPU targets - jvm at least - doesn't support cs_opt_stackframe, but the parentfp optimization applies for any CPU target.
The parentfp optimization need to be tied to some generic optimization. cs_opt_remove_empty_proc looks good for this.

Yuriy.

At first sight, the only thing that needs to be done to "support" cs_opt_stackframe for the JVM target is to always set pi_has_stackparameter there.

The parentfp optimization need to be tied to some generic optimization. cs_opt_remove_empty_proc looks good for this.

That feels rather unrelated to me.

Jonas

Yes, but currently it is not supported. :)

> The parentfp optimization need to be tied to some generic optimization. > cs_opt_remove_empty_proc looks good for this. That feels rather unrelated to me.

cs_opt_stackframe also is only vaguely related to the parentfp optimization.

Maybe implement this in a clean way by adding a new generic optimization cs_opt_hiddenpara? Enable it for -O2. It may be used in future to optimize unused $self.

Yuriy.

Yuriy Sydorov wrote

Maybe implement this in a clean way by adding a new generic optimization cs_opt_hiddenpara?

Maybe cs_opt_unusedpara. It doesn't have to be limited to hidden parameters.

Enable it for -O2. It may be used in future to optimize unused $self.

Yes.

Jonas

IMO there is even nothing wrong to have a completely separate switch like cs_opt_unusedparentfp.

I've added the generic cs_opt_unused_para optimization option. In future, if needed, more fine-grained related options can be introduced.

So now the parentfp optimization is enabled for -O2 and higher.

Yuriy.

Hi,

Yuriy Sydorov wrote

I've added the generic cs_opt_unused_para optimization option. In future, if needed, more fine-grained related options can be introduced. So now the parentfp optimization is enabled for -O2 and higher.

Yuriy, your changes seem to have broken ppdump.pp. For all platforms.

Charlie

I have fixed it.

It broke compiling packages/ide/src/fpcodcmp.pp with the LLVM backend. The issue is when compiling line 234 ("InsertInS(P^.At(I))"). It's trying to load the framepointer parameter there from a register that was never initialised.

I think that InsertItemsInS' parentfp got optimised away (because that procedure does not access parent variables/parameters), but it gets loaded anyway due when calling InsertInS because that procedure does need it.

It may also be also broken with FPC's code generator, but simply not get detected at compile time.

Jonas

Thank you.
I haven't been aware that optimization options are stored in ppu and ppudump need to be updated.

Yuriy.

I'll take a look at this tomorrow.
Thanks for reporting.

Yuriy.

Fixed.

Yuriy.

Yuriy Sydorov wrote

> It may also be also broken with FPC's code generator, but simply not get > detected at compile time. Fixed.

Thanks!

Jonas

Just tested with 3.3.1 from 17.08.2020.

There are still cases where it goes wrong. (tested on win 64 / with -O- and -O1)
components\lazdebuggers\lazdebugtestbase\testapps\WatchesScopePrg.lpi

FuncFooNestedTwice and FuncFooNestedTwice2
Both do not actually access any variables from the parent.

The value for parentfp is passed in register rcx
And the debug info, correctly says that it is in that register.

This also works fine, if stopped at the first line (after "begin") in either of those 2 procedures.
But stepping through the procedure
   "Result := 'abc2';"
uses rcx
and the value is lost.

---------------
when compiling with -al  neither procedure has
# Var $parentfp located at rbp-16, size=OS_64
because the variable is not stored.

Best regards
Martin

Fixed in r46514.

Yuriy.

Yuriy Sydorov wrote

Fixed in r46514.

Thanks, looks ok now.