This patch implements the ABI Plugin for PPC64le. It was based on the
ABI for PPC64. It also enables LLDB to evaluate expressions using JIT.
Details
Diff Detail
- Repository
- rL LLVM
Event Timeline
Is the only difference between ppc64 and ppc64le ABIs in the endianness of the values?
If so, could we make one unified ABI which takes the endianness as an argument (in the constructor, or as a template argument, or deduces it from target endiannes, ...) ?
The ABIs have some other differences. The largest difference between the ABIs is how indirect-calls (and, thus, function pointers) work. There are some other more-minor differences, for example, some of the call-frame offsets are different. It still might be possible to unify the support (we certainly have one backend in LLVM for both), but it's a bit more involved than just switching the endianness.
Thanks for the explanation. These don't sound like major differences. Could you take a look the possibility of merging these two plugins? I scanned through the source code, and it looks like a large part of it is the same, so I'm hoping that we can make like 80% of this code go away with a unified plugin.
If you hit some major roadblock which would prevent doing that, then that's fine, but I would at least like to know what the blocker is.
(And sorry for the delays, I've been OOO last week.)
Thanks, @labath .
The ABI plugin for PPC64be is not working: https://reviews.llvm.org/D5988 .
It was implemented based on the x86_64 plugin and needs to be fixed.
We could commit this patch and merge the plugins when both plugins work properly.
Hmm.. that complicates things a bit. How badly is it broken, do you estimate? Would it be any better than if we just made your new plugin handle the the big-endian target as well (just a best effort, I'm not asking you to actually test it on that target)?
If the ppc64be FreeBSD target is really broken, and there is no desire to fix it, maybe we should just delete the relevant plugin.
@emaste, @jhibbits: do you know what's the state of ppc64 lldb support in freebsd?
It's broken because LLDB doesn't yet handle function descriptors, and I haven't yet made the effort (ENOTIME) to fix that. I think our eventual goal is to move even BE to ELFv2 on FreeBSD, to do away with function descriptors, and it should then Just Work, as powerpc (32-bit) works fine for most cases right now.
That said, if anyone knows enough to add function descriptor knowledge to LLDB, I think that's the only missing piece, or the biggest missing piece. I do want to finish the effort, since I started it 3.5 years ago, but I do need help with understanding that ABI detail and how it fits with LLDB.
Thanks for the explanation. How badly is it broken would you say? Do you think that using this ABI plugin for the big endian target (instead of the one from 2014) would make things any worse? Could you take a quick look to see if there is anything in this plugin that would be completely wrong for the big endian target ? (I couldn't find anything, apart from the obvious things like register constants in CreateDefaultUnwindPlan, but then again, I don't know the ABIs).
I'm trying to avoid making bad design decisions now because of legacy code that is known to be broken and has no clear maintainer nor a plan to fix it.
I think the stack frame structure is not correct for the PPC64be plugin.
But it is hard to say how bad it is broken and fix the plugin without testing it.
The register numbers and stack frame structures are different from the ABI 1.9 to the ABI 2.0.
I don't see anything glaring. I also cross-referenced with the 64-bit BE ABI reference (http://refspecs.linuxfoundation.org/ELF/ppc64/PPC-elf64abi.html) and didn't see anything obvious that would hinder using your plugin as a replacement to mine (as mentioned before, symbol handling is the big problem right now, and that's not handled within the ABI plugin), if yours is fully tested.
Thank you for looking at this @chmeee. @alexandreyy, could you update this plugin to handle big-endian as well, and then delete the old plugin?
Looks nice. Only nit is we probably don't need the m_endian member variable. See inlined comment.
source/Plugins/ABI/SysV-ppc64/ABISysV_ppc64.h | ||
---|---|---|
114 ↗ | (On Diff #130180) | Most other code uses "m_byte_order" as the name. That being said, you can always just ask the process since it is store in the lldb_private::ABI class so you really don't need to store it here if you don't want to, you could add an accessor: lldb::ByteOrder GetByteOrder() const { return GetProcessSP()->GetByteOrder(); } |
Thank you for taking the time to do this.
@chmeee, do you want to take a quick look at this?
Well.. whether this plugin is tested depends on what machine you're running on. If you're running on a ppc machine, plenty of tests will exercise this code (TestReturnValue, and pretty much any test doing expression evaluation). Correct me if I am wrong, but I am assuming you guys are running the lldb testsuite as a part of ppc64le bringup.
Given that our entire test suite is based on compiling things and running them on the host, I don't think it's fair to ask a random contributor to come up with a new test strategy.
(Of course, I would still very much like to have tests which are not host-dependent).
We are running the lldb suite as part of our development process.
And we know that there are still some issues with the PPC64le code.
But we are already working on that.
Later, we will send other patches to fix them.
Thanks!
Thanks.
We are running the default tests of the lldb suite.
Currently, we have 48 failures that we are working on.
Could we proceed with this patch?
We will send fixes for these issues soon.