Re: [PATCH] [Microblaze]: PIC Data Text Relative

[Michael Eager <eager at eagerm dot com>]

On 02/25/2018 11:44 PM, Andrew Guirguis wrote:
> Dears,
>
> Kindly find attached the patch bundle for Microblaze 
> '-mpic-data-text-relative' feature.
>
> Description of the feature in the following link:
> https://github.com/andrewsadek/microblaze-pic-data-text-rel/blob/pic_data_text_rel/README.md 
> <https://github.com/andrewsadek/microblaze-pic-data-text-rel/blob/pic_data_text_rel/README.md>
>
> Bundle includes:
> 1) Change logs for GCC, binutils
> 2) GCC Test results and comparison with the original.
> 3) New Test case (picdtr.c)
> 4) The Patches (against current heads)

Hi Andrew --

Thanks for the submission.  I have the following recommendations:

Submit each patch to the appropriate project mailing list.  Only submit
the patch for the specific project, without patches for other projects.

Include a description of the changes with each patch as well as the
changelog.  Include the patch in your email or as an attachment.

It isn't clear why you sent your submission to the gdb-patches mailing
list, since there don't appear to be any GDB changes.  Conversely, it is
not clear why you did not include the binutils mailing list, since you
include a patch to that project.

Be sure to follow GNU coding conventions,  Check brace placement,
indent, maximum line length, if statements, etc.  I noticed a number
of places where these conventions are not followed in your patches.

GCC regression tests should include all tests (e.g., gcc.dg), not just 
the limited number of MicroBlaze-specific tests.

--
Michael Eager    eager@eagerm.com
1960 Park Blvd., Palo Alto, CA 94306