Hi All,
There is a small change that enables correct calculation of arm sub architectures while using the REPL on arm-linux. As you may of may or may not know, linux appends ‘l’ to arm architecture versions to denote little endian. This sometimes interferes with the determination of the architecture in the triple. I experimented with adding sub architecture entries for these within lldb, but I discovered a simpler (and less invasive) method. Because LLVM already knows how to handle some of these cases (I have a patch submitted for review that enables v6l; v7l already works), I am relying on llvm to clean it up. The gist of it is that the llvm constructor (when given a triple string) retains the provided string unless an accessor mutates it. Meanwhile, the accessors for the components go through the aliasing and parsing logic. This code detects whether the sub-architecture that armv6l or armv7l aliases to is detected, and re-sets the architecture in the triple. This overwrites the architecture that comes from linux, thus sanitizing it.
Some kind of solution is required for the REPL to on arm-linux.
Thoughts?
- Will
lldb.diff (934 Bytes)
Hi again, everyone
I’d like to ping on this patch now that the 3.8 branch is fairly new, and merging it over is fairly straight-forward.
Thanks in advance for your comments!
- Will
lldb.diff (956 Bytes)
This patch looks reasonable to me, but I don't know enough about LLDB
to actually review it.
+Renato or Pavel maybe?
+ Omair
I don't really understand arm (sub)-architectures or REPL. The patch
seems mostly harmless, but it also feels like a hack to me. A couple
of questions:
- why does this only pose a problem for REPL?
- If I understand correctly, the problem is that someone is looking at
the architecture string contained in the Triple, and not finding what
it expects. Is that so? Could you point me to (some of) the places
that do that.
Omair, any thoughts on this?
cheers,
pl
Hi Will,
I dont understand REPL and thus the benefits it will have by making
change to architecture name. I would not recommend to drop any
information that we get from the host operating system.
LLDB maintains core information alongwith triple in ArchSpec, may be
you can parse triple to reflect correct core and use core instead of
architecture name where needed.
Kindly elaborate in a bit detail what are we getting out of this
change for more accurate comments.
Thanks!
Hi Pavel,
Will is trying to get this working downstream of here IIRC.
Greg, can you have a look and see what you think of the patch? (Also see Pavel’s comments).
Thanks!
-Todd
Hi Omair,
In a very real sense, no information is lost here. The addition of the ‘l’ only indicates that the system is little endian. When the triple is created, the flag setting little endian is set (and defaults to little anyway). There is no other valid ARM sub architecture with ARMv6 or ARMv7 that ends with an ‘l’.
I based this change somewhat on the existing model for massaging LLVM triples such as in HostInfoLinux.cpp:
void
HostInfoLinux::ComputeHostArchitectureSupport(ArchSpec &arch_32, ArchSpec &arch_64)
{
HostInfoPosix::ComputeHostArchitectureSupport(arch_32, arch_64);
const char *distribution_id = GetDistributionId().data();
// On Linux, "unknown" in the vendor slot isn't what we want for the default
// triple. It's probably an artifact of config.guess.
if (arch_32.IsValid())
{
arch_32.SetDistributionId(distribution_id);
if (arch_32.GetTriple().getVendor() == llvm::Triple::UnknownVendor)
arch_32.GetTriple().setVendorName(llvm::StringRef());
}
if (arch_64.IsValid())
{
arch_64.SetDistributionId(distribution_id);
if (arch_64.GetTriple().getVendor() == llvm::Triple::UnknownVendor)
arch_64.GetTriple().setVendorName(llvm::StringRef());
}
}
Would it make you more comfortable is this patch was rewritten within HostInfoLinux::ComputeHostArchitectureSupport alongside the massaging of the vendor name for linux?
- Will
In a very real sense, no information is lost here.
That is exactly the reason why it looks very hackish. 
It looks to me like you are trying to work around some other bug,
probably in the code that consumes the triple.
Could you point me to the code that consumes that triple? Couldn't
that be fixed to look at triple.getSubArch() instead of it's string
representation?
Would it make you more comfortable is this patch was rewritten within HostInfoLinux::ComputeHostArchitectureSupport alongside the massaging of the vendor name for linux?
If this is Linux-specific, then HostInfoLinux is definitely a better
place for it. However, I would like to understand this a bit deeper
first...
pl