[Bug testsuite/80759] gcc.target/x86_64/abi/ms-sysv FAILs

["daniel.santos at pobox dot com" <gcc-bugzilla at gcc dot gnu dot org>]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80759

Daniel Santos <daniel.santos at pobox dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #41486|0                           |1
        is obsolete|                            |
  Attachment #41487|0                           |1
        is obsolete|                            |
  Attachment #41488|0                           |1
        is obsolete|                            |
  Attachment #41489|0                           |1
        is obsolete|                            |
  Attachment #41490|0                           |1
        is obsolete|                            |

--- Comment #31 from Daniel Santos <daniel.santos at pobox dot com> ---
Created attachment 41532
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=41532&action=edit
proposed fix v4

OK, so here goes version 4!  Hopefully we're getting closer.

(In reply to ro@CeBiTec.Uni-Bielefeld.DE from comment #30)
> Fine.  Imagine your formatting/coding style changes: when they are
> intermixed with functional changes, every reviewer has to check
> thoroughly which part is which.  If you separate them (which is easy for
> you to do) and submit them separately, the formatting stuff will be
> obvious or nearly so, and you save everyone time by having to review
> only the beef of the functional changes, in the end also giving you
> faster review.

Yes.  I had at least split out all of the formatting stuff into the first
patch, but I had attached them from git format-patch and selected the patch
checkbox in Bugzilla, so maybe it wasn't as obvious as it didn't show the
subject or text in the mbox file (it would have probably been better to copy &
paste the subject line of each patch).  I've separated out only what's relevant
to this bug report: what breaks on Solaris, the ugly hard-coded offsets and
replacing the custom parallelization code.  Note that I have included the
changes to split the test jobs into smaller pieces so that potential
RTL-checking timeout issues are resolved.

> > Please also note that I did seek guidance when putting this exp file together
> > (back in December)  I was following Mike Stump's direction, but you were
> > probably on vacation or something. :) 
> > https://gcc.gnu.org/ml/gcc/2016-12/msg00145.html
> 
> Might be.  However, I've been appointed testsuite maintainer at a time
> when nobody else was around.  Later, Mike stepped up again who's way
> more experienced here than I am.  In the case at hand, I happen to be
> both victim of your patches' fallout and testsuite maintainer ;-)

lol!!  Well I think you're a bit more knowledgeable to some parts of the test
harness as his original direction left all of my tests running one for each
-j<job>!  Once I figured that out, I tried to pester Mike and the mailing list
for a solution, but ended up just hacking out that custom parallelization code.
 But it's all good because I'm learning, and that keeps me happy. :)

> > I've also been motivated to expand the tests by a change somebody else made to
> > my original patch that I wasn't confident the original tests would fully check
> > (been worried about it, but it all looks good).  I'll get a cleaned up patch
> > for you soon.
> 
> Excellent, thanks for your patience.  I know the first times through the
> system can be hard and tedious...

HAH! Thanks for YOUR patience! :)  This patch doesn't include those extended
tests, but at least I have run them and feel more confident.  I'll submit those
changes separately.


> Here's the complete output from and amd64-pc-solaris2.12 build with your
> patch included:
> 
> spawn /var/gcc/regression/trunk/12-gcc-64/build/gcc/xgcc
> -B/var/gcc/regression/trunk/12-gcc-64/build/gcc/
> /vol/gcc/src/hg/trunk/local/gcc/testsuite/gcc.target/x86_64/abi/ms-sysv/ms-
> sysv.c -fno-diagnostics-show-caret -fdiagnostics-color=never -O2
> -DGEN_ARGS=-p1 -t64
> -I/var/gcc/regression/trunk/12-gcc-64/build/gcc/testsuite/gcc6/ms-sysv
> -I/vol/gcc/src/hg/trunk/local/gcc/testsuite/gcc.target/x86_64/abi/ms-sysv
> -Wall -Wall
> /vol/gcc/src/hg/trunk/local/gcc/testsuite/gcc.target/x86_64/abi/ms-sysv/do-
> test.S -lm -o ./ms-sysv.exe^M
> In file included from
> /vol/gcc/src/hg/trunk/local/gcc/testsuite/gcc.target/x86_64/abi/ms-sysv/ms-
> sysv.c:158:0:^M
> /var/gcc/regression/trunk/12-gcc-64/build/gcc/testsuite/gcc6/ms-sysv/ms-sysv-
> generated.h: In function 'msabi_02_1':^M
> /var/gcc/regression/trunk/12-gcc-64/build/gcc/testsuite/gcc6/ms-sysv/ms-sysv-
> generated.h:42:1: error: bp cannot be used in asm here^M
> compiler exited with status 1
> 
> I suspect the problem is not explicitly passing -fno-omit-frame-pointer,
> though.  gcc/config/i386/sol2.h has
> 
> #define USE_IX86_FRAME_POINTER 1
> #define USE_X86_64_FRAME_POINTER 1
> 
> which does this implicitly...
> 
> 	Rainer

Awesome!  This is what I've done in this patch

     # Detect when hard frame pointers are enabled (or required) so we know not
     # to generate bp clobbers.
-    if [regexp "^(.+ +| *)-(O0|fno-omit-frame-pointer|p|pg)( +.*)?$" \
-              $cflags match] then {
+    if { [regexp "(^| )-(O0|fno-omit-frame-pointer|p|pg)( |$)" \
+                "$TEST_ALWAYS_FLAGS $cflags" match]
+        || [istarget *-*-solaris*] } then {
        set generator_args "$generator_args --omit-rbp-clobbers"
     }

I went ahead with adding TEST_ALWAYS_FLAGS just in case, but hopefully the
istarget will fix this.  I haven't gotten to setup a Solaris VM yet, so I'll
have to ask you to please test this again for me and thank you!!

Also, if this works, should I still post it to the mailing list prior to
committing it?  The first time I did that, I didn't realize that you were the
other testsuite maintainer -- d'oh! :o

2017-06-10  Daniel Santos  <daniel.santos@pobox.com>

        * gcc.target/x86_64/abi/ms-sysv/do-test.S (test_data_save,
        test_data_input, test_data_output, test_data_fn,
        test_data_retaddr): Remove.
        (regs_to_mem, mem_to_regs): Make global.
        (do_test_aligned, do_test_unaligned): Gut and call do_test_body.
        * gcc.target/x86_64/abi/ms-sysv/ms-sysv.c: Add dg-* directives.
        (do_test_body0): New function.
        (do_test_body): New function added via inline assembly.
        * gcc.target/x86_64/abi/ms-sysv/ms-sysv.exp (runtest_ms_sysv): Replace
        custom parallelization code with dg-runtest, et. al.

Thanks!
Daniel

PS: I still need to create a local SVN repo and practice committing with
git-svn, because it's been a long time since I've done a push to SVN via git. 
If I screwed that up, they might revoke my commit privs!