This is an archive of the discontinued LLVM Phabricator instance.

[ELF] Restore arm-branch.s test
ClosedPublic

Authored by ikudrin on Jun 15 2021, 3:33 AM.

Details

Summary

After D77330, the comments are inconsistent with the disassembled code. As the value of far has been changed, a thunk to reach it is now generated, and target addresses of branch instructions are different from what was initially expected.

The patch fixes that and makes the test closer to what it was originally.

Diff Detail

Event Timeline

ikudrin created this revision.Jun 15 2021, 3:33 AM
ikudrin requested review of this revision.Jun 15 2021, 3:33 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 15 2021, 3:33 AM
peter.smith accepted this revision.Jun 15 2021, 6:28 AM
peter.smith added a subscriber: peter.smith.

LGTM. We could probably improve the test further with some stable addresses in the linker script, and a max range backwards branch. However that's probably overdoing it for this test as we've gots lots of others that will cover that.

This revision is now accepted and ready to land.Jun 15 2021, 6:28 AM
MaskRay accepted this revision.Jun 15 2021, 10:25 AM

LGTM.

lld/test/ELF/Inputs/far-long-arm-abs.s
10

Add a newline

lld/test/ELF/arm-branch.s
8–9

add a file-level comment what this test does. it is not obvious.

ikudrin added inline comments.Jun 15 2021, 8:45 PM
lld/test/ELF/arm-branch.s
8–9

Well, as I am not the author of the test, I cannot tell for sure the initial intentions; I merely restored its original functionality and fixed comments. Do you have any suggestions about the description? @peter.smith, maybe you, as the first committer, could help with that?

I've made a suggestion for a comment.

lld/test/ELF/arm-branch.s
8–9

I think this was one of the first patches I wrote which introduced just enough Arm support to link hello world. As an introductory comment how about.

/// Test the Arm state R_ARM_CALL and R_ARM_JUMP24 relocation to Arm state destinations.
/// R_ARM_CALL is used for branch and link (BL)
/// R_ARM_JUMP24 is used for unconditional and conditional branches (B and B<cc>)
/// Relocations defined in https://github.com/ARM-software/abi-aa/blob/main/aaelf32/aaelf32.rst
/// Addend A is always -8 to cancel out Arm state PC-bias of 8 bytes
ikudrin updated this revision to Diff 352434.Jun 16 2021, 7:50 AM
ikudrin marked an inline comment as done.
  • Added a missing newline in far-long-arm-abs.s
  • Added a description for arm-branch.s
lld/test/ELF/arm-branch.s
8–9

Thank you for the description!

This revision was automatically updated to reflect the committed changes.