Page MenuHomePhabricator

[RISCV] Implement evaluateBranch
ClosedPublic

Authored by simoncook on Apr 6 2020, 9:29 AM.

Details

Summary

This implements the instruction analysis required to print branch
targets as part of llvm-objdump's disassembly.

Note, this only handles those branches which be analyzed in a single instruction, I'm working on a patch which handles multiple-instruction patterns where GNU objdump prints addresses (loads, stores, AUIPC/LUI+JALR pairs) which I will submit separately.

Diff Detail

Event Timeline

simoncook created this revision.Apr 6 2020, 9:29 AM
simoncook updated this revision to Diff 255400.Apr 6 2020, 10:33 AM

Resolve issue found by clang-tidy

simoncook edited the summary of this revision. (Show Details)Apr 7 2020, 2:22 AM
apazos added a comment.Apr 8 2020, 6:24 PM

Thanks, Simon for pushing this patch, it does help when debugging code and removes the dependence on binutils.

In the future patch for other instructionsm are you including lui/addi combo and addi with gp?

Thanks, Simon for pushing this patch, it does help when debugging code and removes the dependence on binutils.

In the future patch for other instructionsm are you including lui/addi combo and addi with gp?

Yes, my current WIP patch currently handles lui/addi. I need to work out how to feed in the value of GP into MIA, after which gp-relative addresses will work aswell. When that's a bit more polished I'll submit it here as a WIP patch even if its not feature complete

lenary accepted this revision.Apr 9 2020, 2:40 AM

This is a good first step. I'm happy with how well it is tested, and that it solves some common cases of the issue.

Happy for the follow-up supporting multiple instructions to land later, I don't think it should block this patch.

This revision is now accepted and ready to land.Apr 9 2020, 2:40 AM
asb accepted this revision.Apr 9 2020, 5:37 AM

Thanks Simon, LGTM. I noted a tiny nit re the test file.

llvm/test/MC/Disassembler/RISCV/branch-targets.txt
2

Even though it's fairly redundant, we tend to err on the side of testing both rv32 and rv64 at the same time. It would be nice to do so here.

This revision was automatically updated to reflect the committed changes.