Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Operator '__addressof' #445

Merged
merged 5 commits into from
Nov 7, 2019
Merged

Operator '__addressof' #445

merged 5 commits into from
Nov 7, 2019

Conversation

Daniel-Cortez
Copy link
Contributor

@Daniel-Cortez Daniel-Cortez commented Sep 15, 2019

What this PR does / why we need it:

This PR implements operator __addressof that allows to obtain the address of the operand as a compile-time constant value.
Example:

Func1() { print("Func1()"); }
Func2() { print("Func2()"); }
Func3() { print("Func3()"); }

new const functions[3] = // array of function addresses
{
    __addressof(Func1),
    __addressof(Func2),
    __addressof(Func3)
};

main()
{
    for (new i = 0; i < sizeof(functions); ++i)
    {
        // call all functions from the array
        __emit load.u.pri functions[i];
        __emit push.c 0;
        __emit push.c :lbl_ret;
        __emit sctrl 6;
        __emit { lbl_ret: }
    }
}

The operator can be used on:

  • single variables;
  • arrays;
  • array cells;
  • functions (the scripted ones, not the natives);
  • labels.

NOTE: Functions and labels should be defined before they are used via __addressof, because the compiler can't know their address in advance.

While implementing this operator, I also made the following tweaks:

  • Made the symbol suggestion system distinguish native functions and Pawn functions.
    There's no point in suggesting native functions for the use in __addressof.
  • Allowed symbols to be referenced from outside functions.
    This should be useful not only for __addressof, but also for __emit and #emit.
  • Simplified function count_referers() (sc1.c) and renamed it to has_referers().
    When reducing the referers tree, the compiler doesn't really need to count all the referers for each symbol, it only needs to check if the symbol has been referenced at least once.
    This change isn't really necessary for __addressof to function properly, but I already had to modify count_referers() to take in account the newly added "referenced globally" symbol flag (see above), so I also simplified the function while I was at it. This should slightly speed up the compilation process.

Which issue(s) this PR fixes:

Fixes #

What kind of pull this is:

  • A Bug Fix
  • A New Feature
  • Some repository meta (documentation, etc)
  • Other

Additional Documentation:

@Daniel-Cortez Daniel-Cortez requested a review from a team as a code owner September 15, 2019 12:07
@Y-Less
Copy link
Member

Y-Less commented Sep 15, 2019

There are two systems (PawnPlus and amx-assembly) that already implement this to some extent. Obviously compiler support is better, but both existing versions use the full name addressof. So I'd (as always) recommend going with the existing naming schemes and use __addressof.

@Y-Less
Copy link
Member

Y-Less commented Sep 15, 2019

How does it work with otherwise unused symbols? Does using this commit the symbol to the AMX?

@Daniel-Cortez
Copy link
Contributor Author

I'd (as always) recommend going with the existing naming schemes and use __addressof.

Done.

How does it work with otherwise unused symbols? Does using this commit the symbol to the AMX?

Yes it does, as it marks them as uREAD.

source/compiler/tests/__addrof.pwn Outdated Show resolved Hide resolved
source/compiler/tests/__addrof.meta Outdated Show resolved Hide resolved
@Southclaws
Copy link
Collaborator

Nice, this will be very useful!

source/compiler/sc5.c Outdated Show resolved Hide resolved
@Daniel-Cortez
Copy link
Contributor Author

Daniel-Cortez commented Sep 18, 2019

Oops, I think I mistapped and re-requested review (sorry!)

@YashasSamaga I think there shouldn't be anything that would prevent implementing address calculation for local variables at run time; I'll try to finish this today.
EDIT: OK, done.

@YashasSamaga
Copy link
Member

YashasSamaga commented Sep 22, 2019

I think a pcode_check test or runtime test is more suitable for this PR. The current test only checks if the code successfully compiles and throws errors correctly. If you're fine with it, adding some tests for checking if the values returned by __addressof operator is correct. This can be done with a pcode check or runtime test. The runtime test would require __emit code to manually obtain the addresses and match against what's returned by __addressof. The runtime test is probably better as it doesn't assume about the addresses of the symbols and would work even if a future PR changes the layout of the symbols in the binary.

@Daniel-Cortez
Copy link
Contributor Author

Daniel-Cortez commented Sep 23, 2019

Unlike __emit, __addressof utilizes the standard code generation mechanism by calling function address() from sc4.c for stack variables (that's also the reason why __emit can reference functions and labels defined after the point of their use and __addressof can't). While I'm not really against adding P-code tests (in fact, I already have some of them done in a local repo), do we really need to test the stanard code generator?

@@ -0,0 +1,48 @@
#include <console>

const func1_addr = __addressof(Func1); // error
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a little surprising. Functions can otherwise be used before declaration. Would this work if it were forwarded?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Functions can otherwise be used before declaration.

This is because in order to call functions the compiler outputs call .funcname, so it could look up the function and obtain its address later, when assembling the code. But __addressof works as an expression, and expressions are handled at the first two compilation stages (parsing), and the compiler can't know a function/label address until it parses its definition at the 2'nd pass; it doesn't keep function addresses obtained at the 1'st pass since at the 2'nd one the generated code can change (e.g. because of #if defined <func> being used for conditional compilation). I think the only solution for this is to add an ability to force a "second" second pass, but this sounds more like a work for a separate PR, IMO.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could build up a list of unresolved addressof calls, just as CONST.pri locations (or however it is actually implemented), then iterate through them once the function is resolved. A third compiler pass would very probably break a lot of stuff.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or, as I said, would forward fix this? I somewhat doubt it as address isn't in the metadata that could be stored in that way. But maybe the pending addresses could be added to the metadata for a function, thus require forward for this case as well (for something like consistency).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just realised this works:

main()
{
    new a = __emit(CONST.pri Func);
}

Func()
{
}

Copy link
Contributor Author

@Daniel-Cortez Daniel-Cortez Sep 24, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just realised this works:
new a = __emit(CONST.pri Func);

Indeed. As I already mentioned, __emit implements its own code generation mechanism, so it isn't bound by the limits of the standard codegen. But either way it generates code, so it can't be used to initialize constants and/or global variables.

@YashasSamaga YashasSamaga changed the title Operator '__addrof' Operator '__addressof' Sep 25, 2019
@Y-Less Y-Less merged commit 8850316 into pawn-lang:dev Nov 7, 2019
@Daniel-Cortez Daniel-Cortez deleted the __addrof branch December 1, 2019 13:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants