Re: [PATCH] [PowerPC] VLE update

[Alexander Fedotov <alfedotov at gmail dot com>]

> You could reuse the SEC_MEP_VLIW bit, I guess.   The section flags
> weren't really supposed to be used for target-specific purposes, but
> it seems pointless trying to be a purist now.

Okay, so I will set
+#define SEC_PPC_VLE SEC_MEP_VLIW


> That doesn't look elegant at all.  So you are using R_PPC_PLTREL24 and
> R_PPC_LOCAL24PC relocations in VLE code but giving them a new meaning.
> Is that documented in the ABI?
>
> I think it would be better to define new R_PPC_VLE_PLTREL24 and
> R_PPC_VLE_LOCAL24 relocs, and make the assembler generate them when
> assembling VLE code.

Nope, they are not defined by ABI at all. I agree it would be better
to rework. I will remove it from current patch set.

> As I understand it, SHF_PPC_VLE and PF_PPC_VLE are set on sections or
> segments containing instructions that can only be executed by a
> processor in VLE mode.  PPC_OPCODE_VLE is set for such opcodes too.
> So, do the LSP instructions only execute in VLE mode?  I suspect not,
> from their encoding.
>
> Do you have the LSP instructions in the correct opcode table?

I see that LSP instructions are available only on the cores with VLE ISA only.


Alex

On Mon, Aug 28, 2017 at 4:14 PM, Alan Modra <amodra@gmail.com> wrote:
> On Mon, Aug 28, 2017 at 01:11:28PM +0300, Alexander Fedotov wrote:
>> >> +  /* PowerPC VLE code.  */
>> >> +#define SEC_PPC_VLE 0x100000000
>> >
>> > How can this possibly work?  The field is an unsigned int.
>>
>> You are right. In original patch it was just reuse:
>> +#define SEC_PPC_VLE SEC_TIC54X_BLOCK
>>
>> Well what the best way to deal with this ? Extend flagword to long ?
>> Or keep reuse ?
>
> You could reuse the SEC_MEP_VLIW bit, I guess.   The section flags
> weren't really supposed to be used for target-specific purposes, but
> it seems pointless trying to be a purist now.
>
>>
>>
>> >> +     (R_PPC_VLE_PLTREL24): New relocation.
>> >> +     (R_PPC_VLE_ADDR20): Likewise.
>> >
>> >How are these generated?  I see code that processes them, but nothing
>> > to create them.
>>
>> Regarding to R_PPC_VLE_PLTREL24 I forgot to apply patch:
>>
>> @@ -9686,8 +9845,17 @@ ppc_elf_relocate_section (bfd *output_bfd,
>>             }
>>         }
>>        else
>> -       r = _bfd_final_link_relocate (howto, input_bfd, input_section, contents,
>> +       {
>> +         if ((elf_section_flags (input_section) & SHF_PPC_VLE) != 0)
>> +           {
>> +             if (howto == ppc_elf_howto_table[R_PPC_PLTREL24])
>> +               howto = ppc_elf_howto_table[R_PPC_VLE_PLTREL24];
>> +             else if (howto == ppc_elf_howto_table[R_PPC_LOCAL24PC])
>> +               howto = ppc_elf_howto_table[R_PPC_VLE_REL24];
>> +           }
>> +         r = _bfd_final_link_relocate (howto, input_bfd,
>> input_section, contents,
>>                                       rel->r_offset, relocation, addend);
>> +       }
>
> That doesn't look elegant at all.  So you are using R_PPC_PLTREL24 and
> R_PPC_LOCAL24PC relocations in VLE code but giving them a new meaning.
> Is that documented in the ABI?
>
> I think it would be better to define new R_PPC_VLE_PLTREL24 and
> R_PPC_VLE_LOCAL24 relocs, and make the assembler generate them when
> assembling VLE code.
>
>> Yes, R_PPC_VLE_ADDR20 is not used yet. But it is defined by Power
>> Architecture 32-bit Application Binary Interface Supplement 1.0 -
>> Linux & Embedded. So it looks like just missing.
>
> OK.
>
>> >> @@ -253,12 +253,13 @@ get_powerpc_dialect (struct disassemble_info *info)
>> >>    dialect = POWERPC_DIALECT (info);
>> >>
>> >>    /* Disassemble according to the section headers flags for VLE-mode.  */
>> >> -  if (dialect & PPC_OPCODE_VLE
>> >> -      && info->section != NULL && info->section->owner != NULL
>> >> +  if (dialect & PPC_OPCODE_VLE)
>> >> +    return dialect;
>> >> +  else if (info->section != NULL && info->section->owner != NULL
>> >>        && bfd_get_flavour (info->section->owner) == bfd_target_elf_flavour
>> >>        && elf_object_id (info->section->owner) == PPC32_ELF_DATA
>> >>        && (elf_section_flags (info->section) & SHF_PPC_VLE) != 0)
>> >> -    return dialect;
>> >> +    return PPC_OPCODE_VLE;
>> >>    else
>> >>      return dialect & ~ PPC_OPCODE_VLE;
>> >
>> >Can you explain why this change is necessary?
>>
>> As I can see it resolves problem with LSP disassembling on lsp.s test.
>> Unfortunately I have no more history for this one. Drop it from the
>> patch ?
>
> As I understand it, SHF_PPC_VLE and PF_PPC_VLE are set on sections or
> segments containing instructions that can only be executed by a
> processor in VLE mode.  PPC_OPCODE_VLE is set for such opcodes too.
> So, do the LSP instructions only execute in VLE mode?  I suspect not,
> from their encoding.
>
> Do you have the LSP instructions in the correct opcode table?
>
> --
> Alan Modra
> Australia Development Lab, IBM



-- 
Best regards,
AF