Testing for Static Data relocations for both ARM and Thumb targets.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
This is looking very good already! Nice idea to check ARM and Thumb in one test. I am not sure, however, if it's worth the effort. They both produce R_ARM_ABS32 and R_ARM_REL32. The only difference is the alignment and here Thumb alone would be sufficient:
➜ llvm-objdump -r ... RELOCATION RECORDS FOR [.text]: RELOCATION RECORDS FOR [.text]: OFFSET TYPE VALUE OFFSET TYPE VALUE 00000004 R_ARM_ABS32 target 00000002 R_ARM_ABS32 target 00000008 R_ARM_REL32 target 00000006 R_ARM_REL32 target ^ ARM is on a 4-byte boundary ^ Thumb is 2-byte
What we should do at some point (not necessarily in this patch) is to check whether it works on big-endian targets. Add a eb suffix to the arch in order to test it, e.g. thumbebv7-none-linux-gnueabihf
llvm/test/ExecutionEngine/JITLink/AArch32/ELF_static_data_reloc.s | ||
---|---|---|
17 | Yes, but it's no data-relocation, so it should be out-of-scope for this test. |
llvm/test/ExecutionEngine/JITLink/AArch32/ELF_static_data_reloc.s | ||
---|---|---|
34 | Is your intention to distinguish command-lines from comments by using // and # respectively? Because then it seems you are rotating them here? All test I know use the same comment marker for both. |
Agree, I'd say it is more a proof of concept. These provide the scaffolding to add tests for varying targets.
Thanks for the updates! Can we align the assembly in the test a little more with the (now) existing ELF_static_thumb_reloc.s? That may avoid future divergence. Otherwise, this looks good to me.
llvm/test/ExecutionEngine/JITLink/AArch32/ELF_static_data_reloc.s | ||
---|---|---|
14 | Function addresses are not in scope for this test, so we can drop the comment as well |
The disassembly of both ARM and Thumb turned out to be matching but CHECK-INSTR-{ARM,THM} can be added in the future for the lines that diverge.