Greg Clayton wrote:
The patch looks fine and yes it is ok to have Kalimba specific stuff in the 
ObjectFileELF as it is specific to ELF and you are only checking it if the 
machine type is set to Kalimba.

So the changes look good, go ahead and check it in, we can fix the Xcode 
project breakage, but your patch below doesn't seem to have any since it 
doesn't require an extra file.

Thanks for that. There was no extra file in the diff I floated up, since I was only going to separate the kalimba e_flags extraction stuff to a separate module, if the community reckoned it was necessary.

Matt



On Aug 26, 2014, at 5:56 AM, Matthew Gardiner <m...@csr.com> wrote:

Hi folks,

The kalimba architecture has a number of variants. The ones of interest to me 
being 3, 4 and 5. I have added some additional entries to the 
g_elf_arch_entries to describe these variants. However, up until now, no 
subtypes (architecture variants), for ELF objectfiles exist in lldb. So I've 
found it necessary to modify the invocation of ArchSpec.SetArchitecture from 
ObjectFileELF to deal with this.

Now the kalimba variant specification can be deduced by parsing the e_flags 
field from the ELF header. So I've written a specific routine which extracts 
the variant spec from the e_flags. What I'm not too happy about, and would 
appreciate lldb-dev's opinion on, is if it is ok for the extraction routine 
(kalimbaVariantFromElfFlags) to remain in ObjectFileELF.cpp? If not, where 
should the routine go? Furthermore how do I get any new files into the xcode 
project file, given that I have no xcode system - shall I just change the make 
and cmake build and rely on an Apple person to fix any xcode build breakage?

Below is a diff of what I have done recognize kalimba variants, basically:

1. add new entries to ArchSpec::enum Core, and g_elf_arch_entries
2. supply routine to extract the kalimba variant from the e_flags
3. if e_machine == EM_CSR_KALIMBA, extract the kalimba variant and pass it to 
SetArchitecture as the cpu sub type.

--- a/include/lldb/Core/ArchSpec.h    2014-08-18
+++ b/include/lldb/Core/ArchSpec.h    2014-08-18
@@ -103,6 +103,9 @@
         eCore_uknownMach64,

         eCore_kalimba,
+        eCore_kalimba3,
+        eCore_kalimba4,
+        eCore_kalimba5,

         kNumCores,

--- a/source/Core/ArchSpec.cpp    2014-08-05
+++ b/source/Core/ArchSpec.cpp    2014-08-05
@@ -115,7 +115,10 @@
     { eByteOrderLittle, 4, 4, 4 , llvm::Triple::UnknownArch , 
ArchSpec::eCore_uknownMach32  , "unknown-mach-32" },
     { eByteOrderLittle, 8, 4, 4 , llvm::Triple::UnknownArch , 
ArchSpec::eCore_uknownMach64  , "unknown-mach-64" },

-    { eByteOrderLittle, 4, 1, 1 , llvm::Triple::kalimba , ArchSpec::eCore_kalimba  , 
"kalimba" }
+    { eByteOrderLittle, 4, 1, 1 , llvm::Triple::kalimba , ArchSpec::eCore_kalimba  , 
"kalimba" },
+    { eByteOrderLittle, 4, 1, 1 , llvm::Triple::kalimba , ArchSpec::eCore_kalimba3  , 
"kalimba3" },
+    { eByteOrderLittle, 4, 1, 1 , llvm::Triple::kalimba , ArchSpec::eCore_kalimba4  , 
"kalimba4" },
+    { eByteOrderLittle, 4, 1, 1 , llvm::Triple::kalimba , ArchSpec::eCore_kalimba5  , 
"kalimba5" }
};

// Ensure that we have an entry in the g_core_definitions for each core. If you 
comment out an entry above,
@@ -257,7 +260,10 @@
     { ArchSpec::eCore_x86_64_x86_64   , llvm::ELF::EM_X86_64 , 
LLDB_INVALID_CPUTYPE, 0xFFFFFFFFu, 0xFFFFFFFFu }, // AMD64
     { ArchSpec::eCore_mips64          , llvm::ELF::EM_MIPS   , 
LLDB_INVALID_CPUTYPE, 0xFFFFFFFFu, 0xFFFFFFFFu }, // MIPS
     { ArchSpec::eCore_hexagon_generic , llvm::ELF::EM_HEXAGON, 
LLDB_INVALID_CPUTYPE, 0xFFFFFFFFu, 0xFFFFFFFFu }, // HEXAGON
-    { ArchSpec::eCore_kalimba , llvm::ELF::EM_CSR_KALIMBA, 
LLDB_INVALID_CPUTYPE, 0xFFFFFFFFu, 0xFFFFFFFFu }  // KALIMBA
+    { ArchSpec::eCore_kalimba , llvm::ELF::EM_CSR_KALIMBA, 
LLDB_INVALID_CPUTYPE, 0xFFFFFFFFu, 0xFFFFFFFFu },  // KALIMBA
+    { ArchSpec::eCore_kalimba3 , llvm::ELF::EM_CSR_KALIMBA, 3, 0xFFFFFFFFu, 
0xFFFFFFFFu },  // KALIMBA
+    { ArchSpec::eCore_kalimba4 , llvm::ELF::EM_CSR_KALIMBA, 4, 0xFFFFFFFFu, 
0xFFFFFFFFu },  // KALIMBA
+    { ArchSpec::eCore_kalimba5 , llvm::ELF::EM_CSR_KALIMBA, 5, 0xFFFFFFFFu, 
0xFFFFFFFFu }  // KALIMBA

};

--- a/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp 2014-07-22
+++ b/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp 2014-07-22
@@ -257,6 +257,27 @@
     return true;
}

+static uint32_t
+kalimbaVariantFromElfFlags(const elf::elf_word e_flags)
+{
+    const uint32_t dsp_rev = e_flags & 0xFF;
+    uint32_t kal_arch_variant = LLDB_INVALID_CPUTYPE;
+    switch(dsp_rev)
+    {
+        // TODO(mg11) Support more variants
+        case 10:
+            kal_arch_variant = 3;
+            break;
+        case 14:
+            kal_arch_variant = 4;
+            break;
+        default:
+            break;
+    }
+    return kal_arch_variant;
+}
+
+
// Arbitrary constant used as UUID prefix for core files.
const uint32_t
ObjectFileELF::g_core_uuid_magic(0xE210C);
@@ -544,9 +565,15 @@
             {
                 ModuleSpec spec;
                 spec.GetFileSpec() = file;
+
+                const uint32_t sub_type =
+                    llvm::ELF::EM_CSR_KALIMBA == header.e_machine ?
+ kalimbaVariantFromElfFlags(header.e_flags) :
+                    LLDB_INVALID_CPUTYPE;
spec.GetArchitecture().SetArchitecture(eArchTypeELF,
header.e_machine,
- LLDB_INVALID_CPUTYPE);
+ sub_type);
+
                 if (spec.GetArchitecture().IsValid())
                 {
                     llvm::Triple::OSType ostype;
@@ -1269,7 +1296,12 @@
     // We'll refine this with note data as we parse the notes.
     if (arch_spec.GetTriple ().getOS () == llvm::Triple::OSType::UnknownOS)
     {
-        arch_spec.SetArchitecture (eArchTypeELF, header.e_machine, 
LLDB_INVALID_CPUTYPE);
+        const uint32_t sub_type =
+            llvm::ELF::EM_CSR_KALIMBA == header.e_machine ?
+            kalimbaVariantFromElfFlags(header.e_flags) :
+            LLDB_INVALID_CPUTYPE;
+        arch_spec.SetArchitecture (eArchTypeELF, header.e_machine, sub_type);
+
         switch (arch_spec.GetAddressByteSize())
         {
         case 4:

I'd appreciate people's opinion on this diff, before I commit anything. (The reason I 
need to know the variant type, is because some kalimba variants have a different notion 
of "byte-size" i.e. minimum addressable unit. For example for kalimba3 the 
minimum addressable unit from the data bus is 24-bits, whereas for kalimba4 it is the 
more conventional 8-bits. I'd like to reserve the problems/challenges this presents for 
me, regarding my kalimba lldb port, to a future email).

thanks
Matt


Member of the CSR plc group of companies. CSR plc registered in England and 
Wales, registered number 4187346, registered office Churchill House, Cambridge 
Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom
More information can be found at www.csr.com. Keep up to date with CSR on our 
technical blog, www.csr.com/blog, CSR people blog, www.csr.com/people, YouTube, 
www.youtube.com/user/CSRplc, Facebook, 
www.facebook.com/pages/CSR/191038434253534, or follow us on Twitter at 
www.twitter.com/CSR_plc.
New for 2014, you can now access the wide range of products powered by aptX at 
www.aptx.com.
_______________________________________________
lldb-dev mailing list
lldb-dev@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/lldb-dev


  To report this email as spam click 
https://www.mailcontrol.com/sr/TwfF5sSdNNXGX2PQPOmvUihCLhWP7Wj92Ow1ZOSAWBCcBwzg!Mexio6r!ia3fcc2vO2IMtgU0s6W4Hi4JZPuPA==
 .

_______________________________________________
lldb-dev mailing list
lldb-dev@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/lldb-dev

Reply via email to