Page MenuHomePhabricator

[LLDB][RISCV] Add RISC-V ArchSpec and rv32/rv64 variant detection
ClosedPublic

Authored by luismarques on Aug 20 2020, 7:57 AM.

Details

Summary

Adds the RISC-V ArchSpec bits contributed by @simoncook as part of D62732, plus logic to distinguish between riscv32 and riscv64 based on ELF class.

The patch follows the implementation approach previously used for MIPS. It defines RISC-V architecture subtypes and inspects the ELF header, namely the ELF class, to detect the right subtype. At the moment this is slightly silly, as the subtypes coincide with the architecture types (rv32 vs rv64), but given how the existing code was structured this seemed like a straightforward and future-proof solution.

Diff Detail

Event Timeline

luismarques created this revision.Aug 20 2020, 7:57 AM
luismarques requested review of this revision.Aug 20 2020, 7:57 AM
clayborg requested changes to this revision.Aug 20 2020, 2:46 PM

Patch looks good. Just need to test 32 bit RISC as well with a "lldb/test/Shell/ObjectFile/ELF/riscv32-arch.yaml" to verify 32 bit is working.

lldb/test/Shell/ObjectFile/ELF/riscv64-arch.yaml
1–11 ↗(On Diff #286815)

We should test 32 bit as well right? Or do we already have one?

This revision now requires changes to proceed.Aug 20 2020, 2:46 PM

Add riscv32 test.

clayborg accepted this revision.Aug 21 2020, 10:46 AM
This revision is now accepted and ready to land.Aug 21 2020, 10:46 AM

Not so silly; gdb (well, the names are inherited from bfd) has set architecture riscv:rv32/set architecture riscv:rv64 :)

Not so silly; gdb (well, the names are inherited from bfd) has set architecture riscv:rv32/set architecture riscv:rv64 :)

Ha! We're in good company then. Thanks for sharing, Jessica.

luismarques retitled this revision from [LLDB][RISCV] Distinguish between riscv32 and riscv64 based on ELF class to [LLDB][RISCV] Add RISC-V ArchSpec and rv32/rv64 variant detection.
luismarques edited the summary of this revision. (Show Details)
luismarques added a reviewer: labath.
luismarques added a subscriber: labath.

Moved some of the ArchSpec/core bits from D62732 to here, per @labath's suggestion.

labath accepted this revision.Nov 2 2020, 1:16 AM

This looks fine.

lldb/test/Shell/ObjectFile/ELF/riscv64-arch.yaml
1 ↗(On Diff #302190)

Consider using yaml2obj's --docnum functionality to merge these tests into one.

clayborg accepted this revision.Nov 2 2020, 3:44 PM

Merge tests, using --docnum.

luismarques requested review of this revision.Nov 24 2020, 7:53 AM
labath accepted this revision.Nov 25 2020, 4:43 AM
This revision is now accepted and ready to land.Nov 25 2020, 4:43 AM
abidh added a subscriber: abidh.Nov 27 2020, 9:24 AM
clayborg accepted this revision.Nov 30 2020, 2:02 PM
This revision was landed with ongoing or failed builds.Jan 7 2021, 3:03 PM
This revision was automatically updated to reflect the committed changes.