From: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
To: linux-sparse@vger.kernel.org
Cc: Christopher Li <sparse@chrisli.org>,
Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
Subject: [PATCH 4/8] avoid phisrc orphaned by simplify_loads()
Date: Thu, 13 Apr 2017 18:55:47 +0200 [thread overview]
Message-ID: <20170413165551.2785-5-luc.vanoostenryck@gmail.com> (raw)
In-Reply-To: <20170413165551.2785-1-luc.vanoostenryck@gmail.com>
simplify_loads() calls find_dominating_parents() which can
add an OP_PHISRC in each BB it visits and the corresponding
%phi are collected in a list.
Then, depending on find_dominating_parents()'s returned
value, either an OP_PHI is created with this list as phi_list,
or no such OP_PHI is created and the phi_list is discarded.
In the later case, the added OP_PHISRCs are of no use but are
left there nevertheless.
These orphaned OP_PHISRCs can only bring confusion later.
It seems also (but I can't strictly confirm this) that this can
sometimes happen at each CSE-simplification cycle, creating one
more such OP_PHISRC at each cycle, into each concerned BB.
Not good.
Change this by not creating these OP_PHISRC but instead just
collecting their source pseudo. And then only created them
together with the corresponding OP_PHI, or simply discarding
the list, depending on the returned value.
The situation can clearly be seen with the following code:
struct s { void *x, *z; };
extern void use(struct s *);
void *foo(struct s *s)
{
if (s->x == s->z)
use(s);
return s->x;
}
which was linearized as:
foo:
load.64 %r2 <- 0[%arg1]
load.64 %r4 <- 8[%arg1]
seteq.32 %r5 <- %r4, %r2
-> phisrc.64 %phi2 <- %r2
-> phisrc.64 %phi3 <- %r2
cbr %r5, .L1, .L2
.L1:
push.64 %arg1
call use
br .L2
.L2:
load.64 %r8 <- 0[%arg1]
ret.64 %r8
and is now simply linearized as:
foo:
load.64 %r2 <- 0[%arg1]
load.64 %r4 <- 8[%arg1]
seteq.32 %r5 <- %r4, %r2
cbr %r5, .L1, .L2
.L1:
push.64 %arg1
call use
br .L2
.L2:
load.64 %r8 <- 0[%arg1]
ret.64 %r8
Note: this situation seems to have existed since a long time
but have been made worse by the patch:
(5636cd5cb "missing load simplification")
Fixes: 5636cd5cbf816f30ee57d580ec4debd8e0bd7581
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
memops.c | 10 ++++++----
validation/linear/phisrc-orphan-ld.c | 22 ++++++++++++++++++++++
2 files changed, 28 insertions(+), 4 deletions(-)
create mode 100644 validation/linear/phisrc-orphan-ld.c
diff --git a/memops.c b/memops.c
index ac43b6a0a..39469e260 100644
--- a/memops.c
+++ b/memops.c
@@ -17,7 +17,7 @@
#include "flow.h"
static int find_dominating_parents(pseudo_t pseudo, struct instruction *insn,
- struct basic_block *bb, unsigned long generation, struct pseudo_list **dominators,
+ struct basic_block *bb, unsigned long generation, struct instruction_list **dominators,
int local)
{
struct basic_block *parent;
@@ -49,7 +49,7 @@ no_dominance:
continue;
found_dominator:
- add_dominator(dominators, insn, one, NULL);
+ add_instruction(dominators, one);
} END_FOR_EACH_PTR(parent);
return 1;
}
@@ -83,7 +83,7 @@ static void simplify_loads(struct basic_block *bb)
struct instruction *dom;
pseudo_t pseudo = insn->src;
int local = local_pseudo(pseudo);
- struct pseudo_list *dominators;
+ struct instruction_list *dominators;
unsigned long generation;
/* Check for illegal offsets.. */
@@ -115,6 +115,7 @@ static void simplify_loads(struct basic_block *bb)
bb->generation = generation;
dominators = NULL;
if (find_dominating_parents(pseudo, insn, bb, generation, &dominators, local)) {
+ struct pseudo_list *phi_list;
/* This happens with initial assignments to structures etc.. */
if (!dominators) {
if (local) {
@@ -123,7 +124,8 @@ static void simplify_loads(struct basic_block *bb)
}
goto next_load;
}
- rewrite_load_instruction(insn, dominators);
+ phi_list = add_load_dominators(insn, dominators, NULL);
+ rewrite_load_instruction(insn, phi_list);
}
}
next_load:
diff --git a/validation/linear/phisrc-orphan-ld.c b/validation/linear/phisrc-orphan-ld.c
new file mode 100644
index 000000000..a10aedba6
--- /dev/null
+++ b/validation/linear/phisrc-orphan-ld.c
@@ -0,0 +1,22 @@
+struct s {
+ void *x;
+ void *z;
+};
+
+extern void use(struct s *);
+
+void *foo(struct s *s)
+{
+ if (s->x == s->z)
+ use(s);
+
+ return s->x;
+}
+
+/*
+ * check-name: phisrc orphaned (loads)
+ * check-command: test-linearize -Wno-decl $file
+ *
+ * check-output-ignore
+ * check-output-excludes: phisrc
+ */
--
2.12.0
next prev parent reply other threads:[~2017-04-13 16:56 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-04-13 16:55 [PATCH 0/8] avoid creating orphaned OP_PHISRCs Luc Van Oostenryck
2017-04-13 16:55 ` [PATCH 1/8] extract add_dominator() from find_dominating_parents() Luc Van Oostenryck
2017-04-13 16:55 ` [PATCH 2/8] add helper add_load_dominators() Luc Van Oostenryck
2017-04-13 16:55 ` [PATCH 3/8] remove test on initial phi->ident Luc Van Oostenryck
2017-04-13 16:55 ` Luc Van Oostenryck [this message]
2017-04-13 16:55 ` [PATCH 5/8] avoid phisrc orphaned by find_dominating_stores() Luc Van Oostenryck
2017-04-13 16:55 ` [PATCH 6/8] integrate add_load_dominators() into rewrite_load_instruction() Luc Van Oostenryck
2017-04-13 16:55 ` [PATCH 7/8] check duplicated phi-nodes directly on dominators Luc Van Oostenryck
2017-04-13 16:55 ` [PATCH 8/8] avoid creating unneeded phi-sources Luc Van Oostenryck
2017-05-18 17:04 ` [PATCH 0/8] avoid creating orphaned OP_PHISRCs Luc Van Oostenryck
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=20170413165551.2785-5-luc.vanoostenryck@gmail.com \
--to=luc.vanoostenryck@gmail.com \
--cc=linux-sparse@vger.kernel.org \
--cc=sparse@chrisli.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;
as well as URLs for NNTP newsgroup(s).