cse: update PHI users when throwing away an instruction

Commit Message

Kamil Dudka Aug. 27, 2011, 11:39 p.m. UTC

On Saturday 27 August 2011 17:53:23 Linus Torvalds wrote:
> phi2/phi3 are both dead, but theor 'phisrc' instructions haven't been
> killed. Ugly.

The attached patch solves the dangling PHI nodes and does not seem to break 
anything at first glance.  It is probably not the most efficient solution, 
but it might at least show where to look for the problem.

Kamil

Comments

Linus Torvalds Aug. 28, 2011, 12:34 a.m. UTC | #1

On Sat, Aug 27, 2011 at 4:39 PM, Kamil Dudka <[e-mail]> wrote:
>
> The attached patch solves the dangling PHI nodes and does not seem to break
> anything at first glance.  It is probably not the most efficient solution,
> but it might at least show where to look for the problem.

Hmm. From a quick look, looks fine to me. And checks the kernel, and
does indeed get rid of the extraneous phi nodes in the test-case.

Ack.

                       Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-sparse" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Christopher Li Aug. 28, 2011, 6:32 a.m. UTC | #2

On Sat, Aug 27, 2011 at 5:34 PM, Linus Torvalds
<[e-mail]> wrote:
> On Sat, Aug 27, 2011 at 4:39 PM, Kamil Dudka <[e-mail]> wrote:
>>
>> The attached patch solves the dangling PHI nodes and does not seem to break
>> anything at first glance.  It is probably not the most efficient solution,
>> but it might at least show where to look for the problem.
>
> Hmm. From a quick look, looks fine to me. And checks the kernel, and
> does indeed get rid of the extraneous phi nodes in the test-case.

Applied.

Chris
--
To unsubscribe from this list: send the line "unsubscribe linux-sparse" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Pekka Enberg Aug. 28, 2011, 6:33 a.m. UTC | #3

On Sat, Aug 27, 2011 at 4:39 PM, Kamil Dudka <[e-mail]> wrote:
>> The attached patch solves the dangling PHI nodes and does not seem to break
>> anything at first glance.  It is probably not the most efficient solution,
>> but it might at least show where to look for the problem.

On Sun, Aug 28, 2011 at 3:34 AM, Linus Torvalds
<[e-mail]> wrote:
> Hmm. From a quick look, looks fine to me. And checks the kernel, and
> does indeed get rid of the extraneous phi nodes in the test-case.
>
> Ack.

I applied both patches to sparse-llvm.git. Jeff, do they cure all loop
related issues?

                        Pekka
--
To unsubscribe from this list: send the line "unsubscribe linux-sparse" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Jeff Garzik Aug. 28, 2011, 8:53 a.m. UTC | #4

On 08/28/2011 02:33 AM, Pekka Enberg wrote:
> On Sat, Aug 27, 2011 at 4:39 PM, Kamil Dudka<[e-mail]>  wrote:
>>> The attached patch solves the dangling PHI nodes and does not seem to break
>>> anything at first glance.  It is probably not the most efficient solution,
>>> but it might at least show where to look for the problem.
>
> On Sun, Aug 28, 2011 at 3:34 AM, Linus Torvalds
> <[e-mail]>  wrote:
>> Hmm. From a quick look, looks fine to me. And checks the kernel, and
>> does indeed get rid of the extraneous phi nodes in the test-case.
>>
>> Ack.
>
> I applied both patches to sparse-llvm.git. Jeff, do they cure all loop
> related issues?

They look promising...  Currently loops are stuck due to issues 
unrelated to core sparse:  OP_PHI does not show successor phi's due to 
->priv==NULL.  Loops definitely will not work without both predecessors 
and successors both having a correct ->priv pointing to LLVM data.

	Jeff




--
To unsubscribe from this list: send the line "unsubscribe linux-sparse" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

From 7024fcdfba9fce20569f86e2a279c76cb94cbfd8 Mon Sep 17 00:00:00 2001
From: Kamil Dudka <[e-mail]>
Date: Sun, 28 Aug 2011 01:29:13 +0200
Subject: [PATCH] cse: update PHI users when throwing away an instruction


Signed-off-by: Kamil Dudka <[e-mail]>
---
 cse.c |   13 +++++++++++++
 1 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/cse.c b/cse.c
index 2a15745..1b63c0e 100644
--- a/cse.c
+++ b/cse.c
@@ -250,6 +250,19 @@  static void sort_instruction_list(struct instruction_list **list)
 static struct instruction * cse_one_instruction(struct instruction *insn, struct instruction *def)
 {
 	convert_instruction_target(insn, def->target);
+
+	if (insn->opcode == OP_PHI) {
+		/* Remove the instruction from PHI users */
+		pseudo_t phi;
+		FOR_EACH_PTR(insn->phi_list, phi) {
+			struct pseudo_user *pu;
+			FOR_EACH_PTR(phi->users, pu) {
+				if (pu->insn == insn)
+					DELETE_CURRENT_PTR(pu);
+			} END_FOR_EACH_PTR(pu);
+		} END_FOR_EACH_PTR(phi);
+	}
+
 	insn->opcode = OP_NOP;
 	insn->bb = NULL;
 	repeat_phase |= REPEAT_CSE;
-- 
1.7.6