Skip to content

Conversation

@Lukasdoe
Copy link
Contributor

Following #4460, I also implemented call targets hints in WAMR. Support in LLVM is tbd.

This also includes the generalization of our metadata parsing mechanism so that it'll be very easy to integrate new types of compilation metadata.

Basis: https://github.com/WebAssembly/compilation-hints/blob/main/proposals/compilation-hints/Overview.md

struct WASMCompilationHint *next;
enum WASMCompilationHintType type;
uint32 offset;
bool used;
Copy link
Collaborator

Choose a reason for hiding this comment

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

i feel it's simpler to have struct WASMCompilationHint common instead of duplicating individual fields.

hint = hint->next;
}
if (hint != NULL) {
((struct WASMCompilationHintBranchHint *)hint)->used = true;
Copy link
Collaborator

Choose a reason for hiding this comment

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

no need to cast as the used field is in WASMCompilationHint as well.

while (curr != NULL) {
if (!curr->used) {
printf("Unused hint for function %u, offset: %x\n",
i + module->import_count, curr->offset);
Copy link
Collaborator

Choose a reason for hiding this comment

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

we usually don't use printf.
maybe LOG_WARNING?

uint64_t
aot_func_name_hash(const char *name)
{
return MD5Hash(StringRef(name));
Copy link
Collaborator

Choose a reason for hiding this comment

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

is this function ubiquitously available?
i suspect it isn't.

struct WASMCompilationHint *next;
enum WASMCompilationHintType type;
uint32 offset;
bool used;
Copy link
Collaborator

Choose a reason for hiding this comment

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

is this used field only for diagnostic purposes?

return true;
fail:
return false;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

put a blank line between functions

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.

2 participants