Re: [PATCH 00/12] Xilinx MicroBlaze Support Update

[David Holsgrove <david dot holsgrove at xilinx dot com>]

Hi Joseph,

On 11 October 2012 01:09, Joseph S. Myers <joseph@codesourcery.com> wrote:
> On Wed, 10 Oct 2012, Joseph S. Myers wrote:
>
>> On Wed, 10 Oct 2012, david.holsgrove@xilinx.com wrote:
>>
>> > This patch series updates binutils support for Xilinx MicroBlaze by
>> > adding support for new instructions, little endian target (microblazeel)
>> > and TLS relocations.
>>
>> These patches all seem to be missing testcases for the new or changed or
>> bug-fixed features, which should be added to the assembler and linker
>> testsuites.
>
>  ... and I see that actually the assembler and linker testsuites are
> completely missing any tests for MicroBlaze, indicating an unfortunate
> failure of patch review when the port went in originally.  So the lack of
> testcases probably isn't a reason to prevent these patches going in - but

Thanks for the review. I've amended my patches with respect the
comments on each, and will be able to follow up with a v2 of the patch
series shortly.

> I strongly advise adding reasonable testsuite coverage (testing each
> instruction / relocation in the assembler, each relocation in the linker,
> each architecture-specific error message, etc.), as otherwise it's quite
> likely MicroBlaze could accidentally get broken without this readily being
> noticed.
>

We appreciate your pragmatic approach here, and can address the
testcase coverage as a high priority followup to this series.

> --
> Joseph S. Myers
> joseph@codesourcery.com

thanks,
David