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

Add all missing ewasm methods #2

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Add all missing ewasm methods #2

wants to merge 3 commits into from

Conversation

axic
Copy link
Member

@axic axic commented Jun 29, 2018

No description provided.

@lrettig
Copy link
Member

lrettig commented Jun 29, 2018

I'm already working on this

@lrettig lrettig assigned lrettig and unassigned lrettig Jun 29, 2018
@axic
Copy link
Member Author

axic commented Jun 29, 2018

Can we merged this though? It is done, just needs to be reviewed :)

@MaxGraey
Copy link

MaxGraey commented Jun 29, 2018

It seems addressOffset, dataOffset and resultOffset is actually pointers, so better use usize type instead i32

@lrettig
Copy link
Member

lrettig commented Jun 29, 2018

I'll review

@axic
Copy link
Member Author

axic commented Jun 29, 2018

@MaxGraey good to know, what are the differences? Would usize be something like C's intptr? We need to make sure though that usize always translates to the wasm i32 type.

@MaxGraey
Copy link

MaxGraey commented Jun 29, 2018

@axic usize good to fit pointers in C indeed. Main advantage of usize type is dependent on target. If target is wasm32 usize equal to u32 (address range 0x0 - 0xFFFFFFFF), if target is wasm64 usize equal to u64 (address range 0x0 - 0xFFFFFFFFFFFFFFFF). i32 can addressed only 0x0 - 0x7FFFFFFF.

@lrettig
Copy link
Member

lrettig commented Jun 29, 2018

LGTM, back to you @axic. I updated the pointers to use usize (thanks @dcodeIO).

export declare function selfDestruct(addressOffset: usize): void;

@external("debug", "print32")
export declare function print32(value: i32): void;
Copy link

@MaxGraey MaxGraey Jun 29, 2018

Choose a reason for hiding this comment

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

Just note

instead repeated @external("debug", "...") you can simplified and grouped this to:

export declare namespace debug {
  function print32(value: i32): void;
  function print64(value: i64): void;
  function printMem(dataOffset: usize, length: i32): void;
  function printMemHex(dataOffset: usize, length: i32): void;
  ...
}

this behave and generate exactly the same as "atomic" external decorator.

But required call like this: debug.print32(0x80). So if this not what you want leave this as is

Copy link
Collaborator

@dcodeIO dcodeIO Jun 29, 2018

Choose a reason for hiding this comment

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

Another option could be to move all of the debug functions to a file named debug.ts, and rexport its elements from lib/ethereum.ts. By default, the file name without extension is used as the import's module name. If debug.ts resides within lib/, it's not necessary to re-export its elements because standard library exports become globals by default.

@axic
Copy link
Member Author

axic commented Jun 29, 2018

@axic usize good to fit pointers in C indeed. Main advantage of usize type is dependent on target.

Thanks @MaxGraey that is good to know.

Unfortunately in this case it seems i32 makes more sense because we have to enforce the pointer size is always 32 bit. There is no i64 equivalent methods exported by ewasm and by the nature of blockchain (and actual hard limits) it is ensured a wasm binary will never grow past 4GBs, thus eliminating the need for i64.

However, perhaps the cleanest way is to force compilation for 32-bit targets only. Is there a way to do that?

@MaxGraey
Copy link

MaxGraey commented Jun 29, 2018

Currently wasm support only 32-bit addressing mode. This may change in the future (but there are opinions that it is not in such a near future and will require explicit flag). Anyway unsigned type definitely better for reflect pointer type. AS internally always use usize for typeless references.


export declare function getTxOrigin(resultOffset: usize): void;

export declare function log(dataOffset: usize, length: i32, numberOfTopics: i32, topic1: i32, topic2: i32, topic3: i32, topic4: i32): void;

Choose a reason for hiding this comment

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

export declare function log(dataOffset: usize, length: i32, numberOfTopics: i32, topic1: usize, topic2: usize, topic3: usize, topic4: usize): void;

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.

None yet

4 participants