Home Original page

[RFA] PowerPC VLE port

Catherine Moore catherine_moore@mentor.com
Fri Feb 24 16:44:00 GMT 2012 
Hi Alan,

Well, it's taken a while, but I am now resubmitting the patch for the 
PPC VLE port.  I believe that your comments have all been addressed and 
the additional testing that you suggested has been successfully 
completed.  Does this now look okay to commit?

Thanks,
Catherine

On 08/13/2011 07:47 AM, Alan Modra wrote:

> On Fri, Aug 12, 2011 at 02:05:46PM -0400, Catherine Moore wrote:
>> Tests for binutils/gas/ld testsuites for the ppc-eabi, power64,
>> powerpc-aix, powerpc-linux, powerpc-rtems, and powerpc-sysv4
>> configurations reported no regressions.  If I've missed an important
>> configuration for testing, please let me know.
> I'd like to see you do a 32-bit powerpc-linux gcc bootstrap and
> regression test using binutils with your patch applied, and compare
> results with the same using baseline binutils.  Comparing all object
> files in the trees would also give me a feeling of security.
>
>> 	(DWARF2_LINE_MIN_INSN_LENGTH): Redefine to 2.
> Of course, comparing object files won't work with the current patch.
> This needs fixing.  I'm not sure, but I think
> DWARF2_LINE_MIN_INSN_LENGTH is only used after gas has finished
> reading the input file, so you can select 2 if a 2 byte VLE insn has
> been used anywhere in the file.
>
> -/* This bit is reserved by BFD for processor specific stuff.  Name
> -   it properly so that we can easily stay consistent elsewhere.  */
> -#define SEC_PPC_VLE		SEC_TIC54X_BLOCK
>
> Please revert this accidental commit now rather than waiting until the
> whole patch is approved.
>
> +/* Processor specific program headers, p_flags field.  */
> +#define PF_PPC_VLE		0x10000000	/* PowerPC VLE.  */
>
> Used where?
>
>> -      table_op = PPC_OP (opcode->opcode);
>> -      if (op<  table_op)
>> +      mask = opcode->mask;
>> +      table_opcode = opcode->opcode;
>> +      table_op_is_short = PPC_OP_SE_VLE (mask);
>> +      table_op = PPC_OP (table_opcode, PPC_OP_SA (mask));
>> +
>> +      masked_op = op;
>> +      if (table_op_is_short)
>> +        {
>> +          /* Some short instructions only have 4 or 5 opcode bits.  */
>> +          if ((mask&  0xfff) == 0)
>> +            masked_op&= 0x3c;
>> +          else if ((mask&  0x7ff) == 0)
>> +            masked_op&= 0x3e;
>> +        }
>> +      if (masked_op<  table_op)
>>   	break;
>> -      if (op>  table_op)
>> +      if (masked_op>  table_op)
>>   	continue;
> Hmm.  How much does this patch slow down objdump -d of large powerpc
> object files?  If significant, can you do something about it?
>
>> +          /* Skip se_bc in favor of simplified mnemonics.  */
>> +          if (table_opcode == 0xe000&&  !strcmp (opcode->name, "se_bc"))
>> +            continue;
> Sort the opcode table differently instead?
>
>> +      /* Skip e_bc or e_bcl in favor of simplified mnemonics.  */
>> +      else if (table_opcode == 0x7a000000&&  !strcmp (opcode->name, "e_bc"))
>> +          continue;
>> +      else if (table_opcode == 0x7a000001&&  !strcmp (opcode->name, "e_bcl"))
>> +          continue;
> Ditto.
>
>> -	    (*info->fprintf_func) (info->stream, "%ld", value);
>> +	    (*info->fprintf_func) (info->stream, "%d", value);
> value is a long isn't it?
>
>> +  /* A relative 8 bit branch.  */
>> +  HOWTO (R_PPC_VLE_REL8,	/* type */
>> +	 1,			/* rightshift */
>> +	 1,			/* size (0 = byte, 1 = short, 2 = long) */
>> +	 8,			/* bitsize */
>> +	 TRUE,			/* pc_relative */
>> +	 0,			/* bitpos */
>> +	 complain_overflow_bitfield, /* complain_on_overflow */
> complain_overflow_signed?  Ditto other relative branches.
>
>> +static bfd_reloc_status_type
>> +ppc_elf_vle_addr16_split16a (bfd *abfd ATTRIBUTE_UNUSED,
> Unfinished?  I don't see anything to insert the split field, and the
> function is identical to..
>
>> +static bfd_reloc_status_type
>> +ppc_elf_vle_addr16_split16d (bfd *abfd ATTRIBUTE_UNUSED,
> ..this one.
>
>> @@ -3502,6 +3903,9 @@
>>
>>         switch (r_type)
>>   	{
>> +	default:
>> +	  break;
>> +
> Please don't add default cases to switch statements that previously
> lacked them.  I like a warning if some relocation type is missing
> in check_relocs.
>
>> +.  {*  Indicate that the section has the VLE bit set. *}
>> +.#define SEC_PPC_VLE 0x80000000
>> +.
> Where is this used?  If needed, I'd rather see one of the sec_flg[0-6]
> bits used instead for backend specific flags.
>

-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: bfd.cl
URL: <https://sourceware.org/pipermail/binutils/attachments/20120224/aa4ae873/attachment.ksh>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: bfd.patch
Type: text/x-patch
Size: 29389 bytes
Desc: not available
URL: <https://sourceware.org/pipermail/binutils/attachments/20120224/aa4ae873/attachment.bin>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: binutils.cl
URL: <https://sourceware.org/pipermail/binutils/attachments/20120224/aa4ae873/attachment-0001.ksh>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: binutils.patch
Type: text/x-patch
Size: 473 bytes
Desc: not available
URL: <https://sourceware.org/pipermail/binutils/attachments/20120224/aa4ae873/attachment-0001.bin>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: include.cl
URL: <https://sourceware.org/pipermail/binutils/attachments/20120224/aa4ae873/attachment-0002.ksh>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: include.patch
Type: text/x-patch
Size: 5889 bytes
Desc: not available
URL: <https://sourceware.org/pipermail/binutils/attachments/20120224/aa4ae873/attachment-0002.bin>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: ld.cl
URL: <https://sourceware.org/pipermail/binutils/attachments/20120224/aa4ae873/attachment-0003.ksh>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ld.patch
Type: text/x-patch
Size: 28642 bytes
Desc: not available
URL: <https://sourceware.org/pipermail/binutils/attachments/20120224/aa4ae873/attachment-0003.bin>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: opcodes.cl
URL: <https://sourceware.org/pipermail/binutils/attachments/20120224/aa4ae873/attachment-0004.ksh>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: opcodes.patch
Type: text/x-patch
Size: 144524 bytes
Desc: not available
URL: <https://sourceware.org/pipermail/binutils/attachments/20120224/aa4ae873/attachment-0004.bin>
More information about the Binutils mailing list