Re: [PATCH 2/2] [testsuite] PR 80759 Remove gas extensions from do-test.S, fix other problems

[Daniel Santos]

Thanks you for your assistance Rainer!

On 05/19/2017 04:03 AM, Rainer Orth wrote:
unfortunately, it still doesn't, as explained in the PR.  The multilib
support is still wrong/non-existant.

I guess I thought for some reason that would magically appear in 
TEST_ALWAYS_FLAGS. :)  I've explicitly added it for now, but I haven't 
yet found where -m64 gets fed in the normal flow of things and I would 
rather know I'm doing things as closely as possible to how the rest if 
the test harness does it.

(I have SVN write privs now, so I can even commit it myself).
Please always include ChangeLog entries with your patch submissions so
one can easily see what you've change
(cf. https://gcc.gnu.org/contribute.html).

Thanks.
         Rainer

I hate when I forget that!  I'll be sure to remember when I resubmit.

Use of .struct in do_test.S causes breakages when gas isn't the
assembler (e.g., Solaris).  I also wasn't including TEST_ALWAYS_FLAGS in
my CFLAGS resulting in super-ugly log files.  Finally, this patch
eliminates spam of "test unsupported" (limiting it to one printing).

Signed-off-by: Daniel Santos <daniel.san...@pobox.com>
---
  .../gcc.target/x86_64/abi/ms-sysv/do-test.S        | 26 +++++-----------------
  .../gcc.target/x86_64/abi/ms-sysv/ms-sysv.c        |  7 ++++++
  .../gcc.target/x86_64/abi/ms-sysv/ms-sysv.exp      | 24 ++++++++++++--------
  3 files changed, 27 insertions(+), 30 deletions(-)

diff --git a/gcc/testsuite/gcc.target/x86_64/abi/ms-sysv/do-test.S 
b/gcc/testsuite/gcc.target/x86_64/abi/ms-sysv/do-test.S
index 1395235fd1e..967eb959fbc 100644
--- a/gcc/testsuite/gcc.target/x86_64/abi/ms-sysv/do-test.S
+++ b/gcc/testsuite/gcc.target/x86_64/abi/ms-sysv/do-test.S
@@ -46,22 +46,6 @@ fn:
  #  define MOVAPS movaps
  # endif
  
-/* TODO: Is there a cleaner way to provide these offsets?  */
-       .struct 0
-test_data_save:
-
-       .struct test_data_save + 224
-test_data_input:
-
-       .struct test_data_save + 448
-test_data_output:
-
-       .struct test_data_save + 672
-test_data_fn:
-
-       .struct test_data_save + 680
-test_data_retaddr:
-
        .text
  
  regs_to_mem:
@@ -132,23 +116,23 @@ L0:
        call    regs_to_mem
  
  	# Load register with random data
-       lea     test_data + test_data_input(%rip), %rax
+       lea     test_data + 224(%rip), %rax
        call    mem_to_regs
  
  	# Save original return address
        pop     %rax
-       movq    %rax, test_data + test_data_retaddr(%rip)
+       movq    %rax, test_data + 680(%rip)
  
  	# Call the test function
-       call    *test_data + test_data_fn(%rip)
+       call    *test_data + 672(%rip)
  
  	# Restore the original return address
-       movq    test_data + test_data_retaddr(%rip), %rcx
+       movq    test_data + 680(%rip), %rcx
        push    %rcx
  
  	# Save test function return value and store resulting register values
        push    %rax
-       lea     test_data + test_data_output(%rip), %rax
+       lea     test_data + 448(%rip), %rax
        call    regs_to_mem
  
  	# Restore registers
diff --git a/gcc/testsuite/gcc.target/x86_64/abi/ms-sysv/ms-sysv.c 
b/gcc/testsuite/gcc.target/x86_64/abi/ms-sysv/ms-sysv.c
index 2a011f5103d..7cec312c386 100644
--- a/gcc/testsuite/gcc.target/x86_64/abi/ms-sysv/ms-sysv.c
+++ b/gcc/testsuite/gcc.target/x86_64/abi/ms-sysv/ms-sysv.c
@@ -346,6 +346,13 @@ int main (int argc, char *argv[])
    assert (!((long)&test_data.regdata[REG_SET_INPUT] & 15));
    assert (!((long)&test_data.regdata[REG_SET_OUTPUT] & 15));
  
+  /* Verify offsets hard-coded into assembly.  */
+  assert (__builtin_offsetof (struct test_data, regdata[REG_SET_SAVE]) == 0);
+  assert (__builtin_offsetof (struct test_data, regdata[REG_SET_INPUT]) == 
224);
+  assert (__builtin_offsetof (struct test_data, regdata[REG_SET_OUTPUT]) == 
448);
+  assert (__builtin_offsetof (struct test_data, fn) == 672);
+  assert (__builtin_offsetof (struct test_data, retaddr) == 680);
+
    while ((c = getopt (argc, argv, "s:f")) != -1)
      {
        switch (c)
while .struct is a gas extension and doesn't work with the Solaris/x86
/bin/as, having the same (mostly unexplained) constants hardcoded in two
places isn't exactly helpful.  I'd suggest moving them to (say)
ms-sysv.h and include that from both do-test.S (which is preprocessed
assembler after all) and ms-sysv.c.

        Rainer

Well, I don't have an ms-sysv.h, but I suppose I can add one.

I'm starting to lean more towards the idea of plucking out the portion 
of asm that uses these offsets, moving that to an inline asm function 
and having the code in do-test.S just jmp to it.  I wish there was some 
sort of "naked" attribute for x86 since I'm not well versed in every way 
that the compiler can change it in a way that wouldn't be friendly.

void __attribute__((optimize ("-O0 -fno-split-stack")))
do_test_body (void)
{
  __asm__ __volatile__ (
        "# Save registers\n"
"  lea     %0, %%rax\n"
"  call    regs_to_mem\n"
"\n"
"  # Load registers with random data\n"
"  lea     %1, %%rax\n"
"  call    mem_to_regs\n"
"\n"
"  # Save original return address\n"
"  pop     %%rax\n"
"  movq    %%rax, %4\n"
"\n"
"  # Call the test function\n"
"  call    *%3\n"
"\n"
"  # Restore the original return address\n"
"  movq    %4, %%rcx\n"
"  push    %%rcx\n"
"\n"
"  # Save test function return value and store resulting register values\n"
"  push    %%rax\n"
"  lea     %2, %%rax\n"
"  call    regs_to_mem\n"
"\n"
"  # Restore registers\n"
"  lea     %0, %%rax\n"
"  call    mem_to_regs\n"
"  pop     %%rax\n"
"  retq\n\t"
FUNC_END("do_test_unaligned")
FUNC_END("do_test_aligned")
        ::
        "m"(test_data.regdata[REG_SET_SAVE]),
        "m"(test_data.regdata[REG_SET_INPUT]),
        "m"(test_data.regdata[REG_SET_OUTPUT]),
        "m"(test_data.fn),
        "m"(test_data.retaddr):);
}

Any thoughts on that approach?  I haven't actually tested it yet. (It 
would probably nice if I put "memory" in the clobber.)

Thanks,
Daniel