From: Doug Evans <dje@google.com>
To: gdb-patches@sourceware.org
Subject: Re: [PATCH] Show registers as live in better way in amd64-pseudo.c
Date: Fri, 29 Aug 2014 13:46:00 -0000 [thread overview]
Message-ID: <21503.31094.810730.706239@ruffy2.mtv.corp.google.com> (raw)
In-Reply-To: <yjt2ppfoqq5w.fsf@ruffy.mtv.corp.google.com>
Doug Evans writes:
> Hi.
>
> This test is failing on clang because clang uses $eax as a temp register here
> (grep for <<<<):
>
> (gdb) disas /m main
> ...
> 66 /* amd64-dword.exp writes eax-edi here.
> 67 Tell gcc they're clobbered so it doesn't try to keep "data" in
> 68 one of them. */
> 69 asm (""
>
> 70 : /* no outputs */
> 71 : /* no inputs */
> 72 : "eax", "ebx", "ecx", "edx", "esi", "edi");
> 73
> 74 asm ("mov %%eax, 0(%0)\n\t"
> 0x00000000004006c7 <+119>: mov $0x402020,%eax <<<<
> 0x00000000004006cc <+124>: mov %eax,%r8d
> 0x00000000004006cf <+127>: mov %eax,(%r8)
> 0x00000000004006d2 <+130>: mov %ebx,0x4(%r8)
> 0x00000000004006d6 <+134>: mov %ecx,0x8(%r8)
> 0x00000000004006da <+138>: mov %edx,0xc(%r8)
> 0x00000000004006de <+142>: mov %esi,0x10(%r8)
> 0x00000000004006e2 <+146>: mov %edi,0x14(%r8)
>
> FAIL: gdb.arch/amd64-word.exp: check contents of %ax
> FAIL: gdb.arch/amd64-word.exp: check contents of %r8w
> FAIL: gdb.arch/i386-word.exp: check contents of %ax
> [the {amd64,i386}-byte.exp and amd64-dword.exp tests fail similarly]
>
> This patch uses a different mechanism to indicate that eax,etc.
> are live across the two asms.
>
> Regression tested with gcc 4.8.x-google and clang (trunk r215195).
>
> 2014-08-25 Doug Evans <dje@google.com>
>
> * gdb.arch/amd64-pseudo.c (main): Rewrite to better specify when
> eax,etc. are live with values set by gdb and thus the compiler can't
> use them.
> * gdb.arch/i386-pseudo.c (main): Ditto.
Hi.
I checked in a slight variation of the patch.
The original code had, effectively,
asm ("nop"); /* set breakpoint here */
asm ("" /* inform compiler of register usage */);
and I just went with that.
It's slightly cleaner to indicate the register usage at the nop
since that is where the .exp file will be setting the registers.
commit fb0576e98388c6f4585b94684cea8d18c97a91aa
Author: Doug Evans <dje@google.com>
Date: Thu Aug 28 11:38:22 2014 -0700
Rewrite {amd64,i386}-pseudo.c to better specify register liveness.
clang was using eax to construct %0 here:
asm ("mov %%eax, 0(%0)\n\t"
"mov %%ebx, 4(%0)\n\t"
"mov %%ecx, 8(%0)\n\t"
"mov %%edx, 12(%0)\n\t"
"mov %%esi, 16(%0)\n\t"
"mov %%edi, 20(%0)\n\t"
: /* no output operands */
: "r" (data)
: "eax", "ebx", "ecx", "edx", "esi", "edi");
which caused amd64-word.exp (and others similarly) to fail.
It's a perfectly legit thing for clang to do given the available data.
The patch fixes this by marking the registers as live from the
time of the preceding breakpoint.
gdb/testsuite/ChangeLog:
* gdb.arch/amd64-pseudo.c (main): Rewrite to better specify when
eax,etc. are live with values set by gdb and thus the compiler can't
use them.
* gdb.arch/i386-pseudo.c (main): Ditto.
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 4a65fef..d996b7c 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,10 @@
+2014-08-28 Doug Evans <dje@google.com>
+
+ * gdb.arch/amd64-pseudo.c (main): Rewrite to better specify when
+ eax,etc. are live with values set by gdb and thus the compiler can't
+ use them.
+ * gdb.arch/i386-pseudo.c (main): Ditto.
+
2014-08-27 Doug Evans <dje@google.com>
* lib/gdb.exp (gdb_compile_shlib): Add support for clang.
diff --git a/gdb/testsuite/gdb.arch/amd64-pseudo.c b/gdb/testsuite/gdb.arch/amd64-pseudo.c
index 4dac1f1..f6fd0e7 100644
--- a/gdb/testsuite/gdb.arch/amd64-pseudo.c
+++ b/gdb/testsuite/gdb.arch/amd64-pseudo.c
@@ -39,6 +39,21 @@ int data[] = {
int
main (int argc, char **argv)
{
+ register int eax asm ("eax");
+ register int ebx asm ("ebx");
+ register int ecx asm ("ecx");
+ register int edx asm ("edx");
+ register int esi asm ("esi");
+ register int edi asm ("edi");
+ register long r8 asm ("r8");
+ register long r9 asm ("r9");
+ register long r10 asm ("r10");
+ register long r11 asm ("r11");
+ register long r12 asm ("r12");
+ register long r13 asm ("r13");
+ register long r14 asm ("r14");
+ register long r15 asm ("r15");
+
asm ("mov 0(%0), %%eax\n\t"
"mov 4(%0), %%ebx\n\t"
"mov 8(%0), %%ecx\n\t"
@@ -61,15 +76,13 @@ main (int argc, char **argv)
: /* no output operands */
: "r" (data)
: "r8", "r9", "r10", "r11", "r12", "r13", "r14", "r15");
- asm ("nop"); /* second breakpoint here */
- /* amd64-dword.exp writes eax-edi here.
- Tell gcc they're clobbered so it doesn't try to keep "data" in
- one of them. */
- asm (""
- : /* no outputs */
- : /* no inputs */
- : "eax", "ebx", "ecx", "edx", "esi", "edi");
+ asm ("nop" /* second breakpoint here */
+ /* amd64-{byte,word,dword}.exp write eax-edi here.
+ Tell gcc/clang they're live. */
+ : "=r" (eax), "=r" (ebx), "=r" (ecx),
+ "=r" (edx), "=r" (esi), "=r" (edi)
+ : /* no inputs */);
asm ("mov %%eax, 0(%0)\n\t"
"mov %%ebx, 4(%0)\n\t"
@@ -78,17 +91,18 @@ main (int argc, char **argv)
"mov %%esi, 16(%0)\n\t"
"mov %%edi, 20(%0)\n\t"
: /* no output operands */
- : "r" (data)
- : "eax", "ebx", "ecx", "edx", "esi", "edi");
- asm ("nop"); /* third breakpoint here */
+ : "r" (data),
+ /* Mark these as inputs so that gcc/clang won't try to use them as
+ a temp to build %0. */
+ "r" (eax), "r" (ebx), "r" (ecx),
+ "r" (edx), "r" (esi), "r" (edi));
- /* amd64-dword.exp writes r8-r15 here.
- Tell gcc they're clobbered so it doesn't try to keep "data" in
- one of them. */
- asm (""
- : /* no outputs */
- : /* no inputs */
- : "r8", "r9", "r10", "r11", "r12", "r13", "r14", "r15");
+ asm ("nop" /* third breakpoint here */
+ /* amd64-{byte,word,dword}.exp write r8-r15 here.
+ Tell gcc/clang they're live. */
+ : "=r" (r8), "=r" (r9), "=r" (r10), "=r" (r11),
+ "=r" (r12), "=r" (r13), "=r" (r14), "=r" (r15)
+ : /* no inputs */);
asm ("mov %%r8d, 24(%0)\n\t"
"mov %%r9d, 28(%0)\n\t"
@@ -99,8 +113,11 @@ main (int argc, char **argv)
"mov %%r14d, 48(%0)\n\t"
"mov %%r15d, 52(%0)\n\t"
: /* no output operands */
- : "r" (data)
- : "r8", "r9", "r10", "r11", "r12", "r13", "r14", "r15");
+ : "r" (data),
+ /* Mark these as inputs so that gcc/clang won't try to use them as
+ a temp to build %0. */
+ "r" (r8), "r" (r9), "r" (r10), "r" (r11),
+ "r" (r12), "r" (r13), "r" (r14), "r" (r15));
puts ("Bye!"); /* forth breakpoint here */
return 0;
diff --git a/gdb/testsuite/gdb.arch/i386-pseudo.c b/gdb/testsuite/gdb.arch/i386-pseudo.c
index 11cbcd5..34f6b9b 100644
--- a/gdb/testsuite/gdb.arch/i386-pseudo.c
+++ b/gdb/testsuite/gdb.arch/i386-pseudo.c
@@ -29,6 +29,11 @@ int data[] = {
int
main (int argc, char **argv)
{
+ register int eax asm ("eax");
+ register int ebx asm ("ebx");
+ register int ecx asm ("ecx");
+ register int edx asm ("edx");
+
asm ("mov 0(%0), %%eax\n\t"
"mov 4(%0), %%ebx\n\t"
"mov 8(%0), %%ecx\n\t"
@@ -36,15 +41,22 @@ main (int argc, char **argv)
: /* no output operands */
: "r" (data)
: "eax", "ebx", "ecx", "edx");
- asm ("nop"); /* first breakpoint here */
+
+ asm ("nop" /* first breakpoint here */
+ /* i386-{byte,word}.exp write eax-edx here.
+ Tell gcc/clang they're live. */
+ : "=r" (eax), "=r" (ebx), "=r" (ecx), "=r" (edx)
+ : /* no inputs */);
asm ("mov %%eax, 0(%0)\n\t"
"mov %%ebx, 4(%0)\n\t"
"mov %%ecx, 8(%0)\n\t"
"mov %%edx, 12(%0)\n\t"
: /* no output operands */
- : "r" (data)
- : "eax", "ebx", "ecx", "edx");
+ : "r" (data),
+ /* Mark these as inputs so that gcc/clang won't try to use them as
+ a temp to build %0. */
+ "r" (eax), "r" (ebx), "r" (ecx), "r" (edx));
puts ("Bye!"); /* second breakpoint here */
return 0;
prev parent reply other threads:[~2014-08-29 13:46 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-08-25 20:18 Doug Evans
2014-08-28 21:58 ` Doug Evans
2014-08-29 13:46 ` Doug Evans [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=21503.31094.810730.706239@ruffy2.mtv.corp.google.com \
--to=dje@google.com \
--cc=gdb-patches@sourceware.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox