Skip to content

Conversation

@kripken
Copy link
Member

@kripken kripken commented Jan 16, 2020

Update high_pc values. These are interesting as they
may be a relative offset compared to the low_pc.

For functions we already had both a start and an end. Add
such tracking for instructions as well.

This PR depends on #2594 (testcase updates).

@kripken kripken requested a review from dschuff January 16, 2020 18:18
Copy link
Member

@dschuff dschuff left a comment

Choose a reason for hiding this comment

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

Do we actually have any examples of DW_AT_high_pc [DW_FORM_addr] (i.e. the absolute form) in our test cases?

0x00000082: DW_TAG_subprogram [10] *
DW_AT_low_pc [DW_FORM_addr] (0x0000000000000006)
DW_AT_high_pc [DW_FORM_data4] (0x00000397)
DW_AT_high_pc [DW_FORM_data4] (0x00000383)
Copy link
Member

Choose a reason for hiding this comment

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

I guess this means that Binaryen shrank the function? Is that expected for this test?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, it shrinks it a little even without running optimizations, due to LEBs being compressed (which clang doesn't do in all places). I verified that size is correct.

@kripken
Copy link
Member Author

kripken commented Jan 16, 2020

We don't seem to have the absolute addr variant - perhaps llvm never emits it? (It does seem less efficient, so seems reasonable it wouldn't.

@kripken kripken changed the base branch from floc2-postupdate to master January 16, 2020 21:26
@kripken kripken merged commit 0ec999d into master Jan 16, 2020
@kripken kripken deleted the floc2-b branch January 16, 2020 23:51
awtcode pushed a commit to awtcode/binaryen that referenced this pull request Jan 23, 2020
Update high_pc values. These are interesting as they
may be a relative offset compared to the low_pc.

For functions we already had both a start and an end. Add
such tracking for instructions as well.
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.

3 participants