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

[Michael Eager <eager@eagerm.com>]

On 03/02/2018 08:12 AM, Andrew Sadek wrote:
> Hello Michael,
> 
> I tried running the whole GCC test suite on the current head (without my 
> patch) along with 'microblaze-qemu' but I have the following problems:
> 
> 1) The test is hanging at 'gcc.c-torture/string-large-1.c' , the gcc is 
> making a 100% CPU usage and the machine stucks.
> I tried compiling the file alone, it generated a couple of warnings than 
> it hangs.
>   warning: '__builtin_memchr' specified size 4294967295 exceeds maximum 
> object size 2147483647 [-Wstringop-overflow=]
>     vp1 = __builtin_memchr (a, b, SIZE1);
> 
> Is it a bug? Is there something wrong with my configuration ?
> GCC configured with options :  --with-newlib --enable-threads=no 
> --disable-shared --with-pkgversion='crosstool-NG 
> crosstool-ng-1.23.0-280-g01e3290' --enable-__cxa_atexit 
> --disable-libgomp --disable-libmudflap --disable-libmpx --disable-libssp 
> --disable-libquadmath --disable-libquadmath-support --enable-lto 
> --with-host-libstdcxx='-static-libgcc -Wl,-Bstatic,-lstdc++,-Bdynamic 
> -lm' --enable-target-optspace --disable-nls --enable-multiarch 
> --enable-languages=c,c++

Your configuration is more complex than my hard-metal target version,
but it looks reasonable.

The problem with string-large-1.c does appear to be a bug.  You can
add a line to the test case which will mark it as known failure for MB:

   /* { dg-xfail-if "exceeds maximum" { microblaze-*-* } } */

(I have not tested this, but it should work.  Compare with other
xfail's.)

> 2) For running QEMU, I have no problem with elf execution. But I do not 
> know how to auto terminate the QEMU itself  as it remains up even after 
> program execution.
> Is there some command to be passed to QEMU in order make system shut 
> down after program termination with its exit code ?

Yes, this is a problem.  For other targets using QEMU I have added dummy
HLT instructions to terminate QEMU, or used a wrapper around QEMU which
sets breakpoints at exit (or _exit) and stops the simulator when hit.

If you are running Linux on QEMU, instead of using QEMU's built-in gdb
interface you might use the Linux system as the target for the test
suite, running gdbserver on the target.

> 
> 
> On Tue, Feb 27, 2018 at 10:13 AM, Andrew Sadek 
> <andrew.sadek.se@gmail.com <mailto:andrew.sadek.se@gmail.com>> wrote:
> 
>     Thanks Micheal for your response.
>     I shall re-submit patches separately after re-running the whole GCC
>     Test suite and re-checking code conventions.
>     For sending to gdb-patches, it was a conflict from my side as
>     actually I thought it is also for binutils.
> 
>     On Tue, Feb 27, 2018 at 2:07 AM, Michael Eager <eager@eagerm.com
>     <mailto:eager@eagerm.com>> wrote:
> 
>         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>
>             <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 <mailto:eager@eagerm.com>
>         1960 Park Blvd., Palo Alto, CA 94306
> 
> 
> 
> 
>     -- 
> 
>     Andrew
> 
> 
> 
> 
> -- 
> 
> Andrew

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