Cool. Thanks for these opinions, Todd. I'll submit something in a day or so.
Matt

Todd Fiala wrote:
> 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?

Currently we have Linux/*BSD detection logic (i.e. OS detection code) within ObjectFileELF. Based on that precedent, I think having cpu_arch-specific detection in ObjectFileELF is fine. We could abstract that all and register them somehow to separate them out, but at this point I don't think the extra complexity is worth it. We can always revisit later.


On Tue, Aug 26, 2014 at 8:14 AM, Todd Fiala <[email protected] <mailto:[email protected]>> wrote:

    > 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?

    That's correct.  One of the community will fix it up.  Enough of
    us build with enough build systems that I'm sure we'll catch it
    relatively quickly.  (We ultimately expect build bots to catch
    these but we are still having to do this "manually" while we get
    that together).


    On Tue, Aug 26, 2014 at 5:56 AM, Matthew Gardiner <[email protected]
    <mailto:[email protected]>> 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
        <http://www.csr.com>. Keep up to date with CSR on our
        technical blog, www.csr.com/blog <http://www.csr.com/blog>,
        CSR people blog, www.csr.com/people
        <http://www.csr.com/people>, YouTube,
        www.youtube.com/user/CSRplc
        <http://www.youtube.com/user/CSRplc>, Facebook,
        www.facebook.com/pages/CSR/191038434253534
        <http://www.facebook.com/pages/CSR/191038434253534>, or follow
        us on Twitter at www.twitter.com/CSR_plc
        <http://www.twitter.com/CSR_plc>.
        New for 2014, you can now access the wide range of products
        powered by aptX at www.aptx.com <http://www.aptx.com>.
        _______________________________________________
        lldb-dev mailing list
        [email protected] <mailto:[email protected]>
        http://lists.cs.uiuc.edu/mailman/listinfo/lldb-dev




-- Todd Fiala | Software Engineer | [email protected]
    <mailto:[email protected]> |  650-943-3180





--
Todd Fiala | Software Engineer | [email protected] <mailto:[email protected]> | 650-943-3180




To report this email as spam click here <https://www.mailcontrol.com/sr/3Ur0tdkplDXGX2PQPOmvUuRWCrKzB2y3pAjLDnJQsUmOlvbfGangkBgscvXSUhTVvO2IMtgU0s45W2BPFL9JFA==>.


_______________________________________________
lldb-dev mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/lldb-dev

Reply via email to