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

[wamr] skip leading paddings in .aot section #380

Merged
merged 1 commit into from
Sep 11, 2024

Conversation

lum1n0us
Copy link
Contributor

@lum1n0us lum1n0us commented Jan 4, 2024

No description provided.

@lum1n0us lum1n0us changed the title Fr/padding in aot section [wamr] skip leading paddings in .aot section Jan 4, 2024
@shukitchan
Copy link

Perhaps we should separate this into 2 PRs?
one for wamr 1.3.0
another to do skip leading padding for aot?

@PiotrSikora
Copy link
Member

Perhaps we should separate this into 2 PRs? one for wamr 1.3.0 another to do skip leading padding for aot?

It seems that skipping is done only for AOT, so I don't see a reason for it to be 2 separate PRs.

@lum1n0us
Copy link
Contributor Author

This one is based on #378, and it will only contain "skip aot padding" after rebasing after #378 merged.

Copy link
Member

@PiotrSikora PiotrSikora left a comment

Choose a reason for hiding this comment

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

LGTM, sans comments.

src/wamr/wamr.cc Show resolved Hide resolved
bazel/repositories.bzl Outdated Show resolved Hide resolved
@lum1n0us lum1n0us force-pushed the fr/padding_in_aot_section branch 2 times, most recently from 69f34bf to 817b73a Compare August 18, 2024 03:06
@lum1n0us lum1n0us marked this pull request as draft August 19, 2024 00:46
@lum1n0us lum1n0us marked this pull request as ready for review August 19, 2024 07:44
@lum1n0us
Copy link
Contributor Author

lum1n0us commented Sep 9, 2024

@mpwarres @martijneken any update?

Copy link
Contributor

@mpwarres mpwarres left a comment

Choose a reason for hiding this comment

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

LGTM mod a few superficial nits. Thanks!

src/wamr/wamr.cc Outdated Show resolved Hide resolved
src/wamr/wamr.cc Outdated Show resolved Hide resolved
src/wamr/wamr.cc Outdated Show resolved Hide resolved
martijneken added a commit to martijneken/proxy-wasm-cpp-host that referenced this pull request Sep 10, 2024
Seen on proxy-wasm#380 CI:

Error: This request has been automatically failed because it uses a deprecated version of `actions/upload-artifact: v2`. Learn more: https://github.blog/changelog/2024-02-13-deprecation-notice-v1-and-v2-of-the-artifact-actions/

Signed-off-by: Martijn Stevenson <[email protected]>
bazel/external/wamr.BUILD Outdated Show resolved Hide resolved
src/wamr/wamr.cc Outdated Show resolved Hide resolved
src/wamr/wamr.cc Outdated Show resolved Hide resolved
src/wamr/wamr.cc Outdated Show resolved Hide resolved
src/wamr/wamr.cc Outdated Show resolved Hide resolved
mpwarres pushed a commit that referenced this pull request Sep 10, 2024
Seen on #380 CI:

Error: This request has been automatically failed because it uses a deprecated version of `actions/upload-artifact: v2`. Learn more: https://github.blog/changelog/2024-02-13-deprecation-notice-v1-and-v2-of-the-artifact-actions/

Signed-off-by: Martijn Stevenson <[email protected]>
Copy link
Contributor

@mpwarres mpwarres left a comment

Choose a reason for hiding this comment

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

Happy to merge this once @johnlanni 's open comments are addressed. Thanks!

src/wamr/wamr.cc Outdated Show resolved Hide resolved
Copy link
Contributor

@mpwarres mpwarres left a comment

Choose a reason for hiding this comment

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

LG, thanks @lum1n0us !

@mpwarres mpwarres merged commit 0699c05 into proxy-wasm:main Sep 11, 2024
30 checks passed
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.

5 participants