* RFA: Fix mark_referenced_resources to handle COND_EXEC
@ 2013-09-06 10:22 Joern Rennecke
2013-09-06 17:15 ` Steven Bosscher
0 siblings, 1 reply; 2+ messages in thread
From: Joern Rennecke @ 2013-09-06 10:22 UTC (permalink / raw)
To: gcc-patches; +Cc: Jeff Law
[-- Attachment #1: Type: text/plain, Size: 1029 bytes --]
We found that gethostbyname_r from uClibc got miscompiled for ARC because
reorg thought a COND_EXEC unconditionally kills all the conditionally set
registers. Making it think that the registers are not killed is no good
either, because then we would miss anti-dependencies. So, short of adding
some complex predicate-tracing across all paths, it is best to consider
the result registers to be used before set.
bootstrapped / regtested on i686-pc-linux-gnu.
Well, that's not a particular relevant port since it doesn't combine
delay_slots with COND_EXEC, but then, only ARC does, and it's still
waiting for review. And an arc-linux-uclibc system can't boot properly
without this patch being applied to the compiler, and you have to boot
the system before doing a compiler bootstrap... and the later is also a
bit uncommon for embedded targets.
This is also the reason I placed the test into gcc.target; in principle
the test could also live in gcc.dg, but it would only be relevant to ARC.
OK to apply?
[-- Attachment #2: mark_set_resources-patch --]
[-- Type: text/plain, Size: 947 bytes --]
gcc:
2013-04-30 Joern Rennecke <joern.rennecke@embecosm.com>
* resource.c (mark_referenced_resources): Handle COND_EXEC.
Index: resource.c
===================================================================
--- resource.c (revision 202295)
+++ resource.c (working copy)
@@ -374,6 +374,16 @@ mark_referenced_resources (rtx x, struct
case INSN:
case JUMP_INSN:
+ if (GET_CODE (PATTERN (x)) == COND_EXEC)
+ /* In addition to the usual references, also consider all outputs
+ as referenced, to compensate for mark_set_resources treating
+ them as killed. This is similar to ZERO_EXTRACT / STRICT_LOW_PART
+ handling, execpt that we got a partial incidence instead of a partial
+ width. */
+ mark_set_resources (x, res, 0,
+ include_delayed_effects
+ ? MARK_SRC_DEST_CALL : MARK_SRC_DEST);
+
#ifdef INSN_REFERENCES_ARE_DELAYED
if (! include_delayed_effects
&& INSN_REFERENCES_ARE_DELAYED (x))
[-- Attachment #3: cond-set-use-test-patch --]
[-- Type: text/plain, Size: 3133 bytes --]
gcc/testsuite:
2013-08-31 Joern Rennecke <joern.rennecke@embecosm.com>
* gcc.target/arc/cond-set-use.c: New test.
diff --git a/gcc/testsuite/gcc.target/arc/cond-set-use.c b/gcc/testsuite/gcc.target/arc/cond-set-use.c
new file mode 100644
index 0000000..aee2725
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arc/cond-set-use.c
@@ -0,0 +1,128 @@
+/* { dg-do run } */
+/* { dg-options "-Os" } */
+
+/* Based on gethostbyname_r,
+ * Copyright (C) 2000-2006 Erik Andersen <andersen@uclibc.org>
+ *
+ * Licensed under the LGPL v2.1, see the file COPYING.LIB
+ *
+ * Extraction / wrapping as test by
+ * Joern Rennecke <joern.rennecke@embecosm.com>
+ * Copyright (C) 2013 Free Software Foundation, Inc.
+ */
+
+typedef unsigned size_t;
+typedef int ssize_t;
+typedef unsigned uint32_t;
+struct resolv_answer {
+ char *dotted;
+ int atype;
+ int aclass;
+ int ttl;
+ int rdlength;
+ const unsigned char *rdata;
+ int rdoffset;
+ char* buf;
+ size_t buflen;
+ size_t add_count;
+};
+struct hostent
+{
+ char *h_name;
+ char **h_aliases;
+ int h_addrtype;
+ int h_length;
+ char **h_addr_list;
+};
+
+int *__attribute__ ((noinline,weak)) nop (void * p) { return p; };
+void __attribute__ ((noinline,weak)) seta (struct resolv_answer * p)
+{ p->atype = 1;}
+
+int ghostbyname_r(
+ struct hostent *result_buf,
+ char *buf,
+ size_t buflen,
+ struct hostent **result,
+ int *h_errnop)
+{
+ char **addr_list;
+ char **alias;
+ char *alias0;
+ int i0;
+ struct resolv_answer a;
+ int i;
+
+ *result = ((void *)0);
+
+ *h_errnop = -1;
+
+ if ((ssize_t)buflen <= 5)
+ return 34;
+
+ alias = (char **)buf;
+ addr_list = (char **)buf;
+
+ /* This got turned into branch with conditional move in delay slot. */
+ if ((ssize_t)buflen < 256)
+ return 34;
+
+
+ {
+ if (!nop(&i0)) {
+ result_buf->h_aliases = alias;
+ result_buf->h_addrtype = 2;
+ result_buf->h_length = 4;
+ result_buf->h_addr_list = addr_list;
+ *result = result_buf;
+ *h_errnop = 0;
+ return 0;
+ }
+ }
+
+
+ seta (&a);
+
+ if (a.atype == 1) {
+
+ int need_bytes = sizeof(addr_list[0]) * (a.add_count + 1 + 1);
+
+ int ips_len = a.add_count * a.rdlength;
+
+ buflen -= (need_bytes + ips_len);
+ if ((ssize_t)buflen < 0) {
+ i = 34;
+ goto free_and_ret;
+ }
+
+ result_buf->h_addrtype = 2;
+ *result = result_buf;
+ *h_errnop = 0;
+ i = 0;
+ goto free_and_ret;
+ }
+
+ /* For cse, the 1 was is loaded into a call-saved register;
+ the load was hoisted into a delay slot before the conditional load,
+ clobbering result_buf, which (conditionally) lived in the same
+ call-saved register, because mark_referenced_resources considered the
+ destination of the COND_EXEC only clobbered, but not used. */
+ *h_errnop = 1;
+ *nop(&i0) = 1;
+ i = 2;
+
+ free_and_ret:
+ nop (&i0);
+ return i;
+}
+
+int
+main ()
+{
+ struct hostent buf, *res;
+ int i;
+ char c;
+ ghostbyname_r (&buf, &c, 1024, &res, &i);
+ ghostbyname_r (&buf, 0, 1024, &res, &i);
+ return 0;
+}
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: RFA: Fix mark_referenced_resources to handle COND_EXEC
2013-09-06 10:22 RFA: Fix mark_referenced_resources to handle COND_EXEC Joern Rennecke
@ 2013-09-06 17:15 ` Steven Bosscher
0 siblings, 0 replies; 2+ messages in thread
From: Steven Bosscher @ 2013-09-06 17:15 UTC (permalink / raw)
To: Joern Rennecke; +Cc: GCC Patches, Jeff Law
On Fri, Sep 6, 2013 at 12:22 PM, Joern Rennecke wrote:
> 2013-04-30 Joern Rennecke <...>
>
> * resource.c (mark_referenced_resources): Handle COND_EXEC.
This is OK.
Ciao!
Steven
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2013-09-06 17:15 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-09-06 10:22 RFA: Fix mark_referenced_resources to handle COND_EXEC Joern Rennecke
2013-09-06 17:15 ` Steven Bosscher
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).