[PATCH 0/2] normalize bb's label names for testing

[PATCH 0/2] normalize bb's label names for testing

[Luc Van Oostenryck]

These two patches solves the testing problem of the label names
as emitted by test-linearize and others being not comparable
between runs.

Luc Van Oostenryck (2):
  testsuite: allow commands to use pipes
  testsuite: add a script to normalize label names

 validation/normalize-bb        | 16 ++++++++++++++++
 validation/test-normalize-bb.c | 31 +++++++++++++++++++++++++++++++
 validation/test-pipe-cmd.c     | 13 +++++++++++++
 validation/test-suite          |  6 +++---
 4 files changed, 63 insertions(+), 3 deletions(-)
 create mode 100755 validation/normalize-bb
 create mode 100644 validation/test-normalize-bb.c
 create mode 100644 validation/test-pipe-cmd.c

-- 
2.10.2

[PATCH 1/2] testsuite: allow commands to use pipes

[Luc Van Oostenryck]

This can be usefull for filtering, transforming the normal
output in order to normalize it so that it can be compared
or to ignore things irrelevant to the test.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 validation/test-pipe-cmd.c | 13 +++++++++++++
 validation/test-suite      |  6 +++---
 2 files changed, 16 insertions(+), 3 deletions(-)
 create mode 100644 validation/test-pipe-cmd.c

diff --git a/validation/test-pipe-cmd.c b/validation/test-pipe-cmd.c
new file mode 100644
index 00000000..8e86e9c3
--- /dev/null
+++ b/validation/test-pipe-cmd.c
@@ -0,0 +1,13 @@
+3
+2
+1
+/*
+ * check-name: test-pipe-cmd
+ * check-command: sparse $file | sort
+ * check-output-start
+
+1
+2
+3
+ * check-output-end
+ */
diff --git a/validation/test-suite b/validation/test-suite
index df5a7c60..9f4974c7 100755
--- a/validation/test-suite
+++ b/validation/test-suite
@@ -106,8 +106,8 @@ do_test()
 	cmd=`eval echo $default_path/$default_cmd`
 	get_value "check-command" $file
 	if [ "$?" -eq "0" ]; then
-		last_result=`echo $last_result | sed -e 's/^ *//'`
-		cmd=`eval echo $default_path/$last_result`
+		last_result=`echo "$last_result" | sed -e 's/^ *//'`
+		cmd=`eval echo "\"$default_path/$last_result\""`
 	fi
 
 	# check for disabled commands
@@ -142,7 +142,7 @@ do_test()
 
 
 	# grab the actual output & exit value
-	$cmd 1> $file.output.got 2> $file.error.got
+	eval $cmd 1> $file.output.got 2> $file.error.got
 	actual_exit_value=$?
 
 	for stream in output error; do
-- 
2.10.2

[PATCH 2/2] testsuite: add a script to normalize label names

[Luc Van Oostenryck]

test-linearize displays basic block's labels by using
'.L0x' + plus the address of the bb struct. This is certainly
convenient as an UID but it has the disadvantage that these
labels names are not comparable between runs.

This complicate testing quite a bit.

This script more or less solve this by filtering the output of
test-linearize and change these names to some nice sequential
and comparable '.L1', '.L2', ...

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 validation/normalize-bb        | 16 ++++++++++++++++
 validation/test-normalize-bb.c | 31 +++++++++++++++++++++++++++++++
 2 files changed, 47 insertions(+)
 create mode 100755 validation/normalize-bb
 create mode 100644 validation/test-normalize-bb.c

diff --git a/validation/normalize-bb b/validation/normalize-bb
new file mode 100755
index 00000000..a7c7b73b
--- /dev/null
+++ b/validation/normalize-bb
@@ -0,0 +1,16 @@
+#!/bin/sh
+
+out=$(mktemp)
+sed=$(mktemp)
+
+n=0
+tee $out | grep '^\.L0x' | sort -u | \
+	sed 's/\(.L0x[0-9a-f]*\).*/\1/'	| \
+	while read l; do
+		n=$(($n + 1))
+		echo "s/$l/.L$n/"
+	done > $sed
+
+cat $out | sed -f $sed
+
+rm -f $out $sed
diff --git a/validation/test-normalize-bb.c b/validation/test-normalize-bb.c
new file mode 100644
index 00000000..57a5886e
--- /dev/null
+++ b/validation/test-normalize-bb.c
@@ -0,0 +1,31 @@
+/*
+ * test the normalization of basic block's names
+ */
+
+void use(int);
+void foo(int p);
+void foo(int p)
+{
+	if (p)
+		use(1);
+}
+
+/*
+ * check-name: test-normalize-bb
+ * check-command: test-linearize $file | ./normalize-bb
+ * check-output-start
+foo:
+.L1:
+	<entry-point>
+	br          %arg1, .L2, .L3
+
+.L2:
+	call        use, $1
+	br          .L3
+
+.L3:
+	ret
+
+
+ * check-output-end
+ */
-- 
2.10.2

Re: [PATCH 0/2] normalize bb's label names for testing

[Christopher Li]

On Mon, Nov 21, 2016 at 12:14 PM, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
> These two patches solves the testing problem of the label names
> as emitted by test-linearize and others being not comparable
> between runs.

I think it is much easier just let test-linearize print out label in
a predictable way. e.g. In the "struct basic_block", there is this
two fields "generation" and "priv". The "generation" is used by
flow analyze to store some state. Same as the "priv", which is
used by the back end to store some back end related state.

Since test-linearize can treat as a back end, it is perfectly safe
to store some allocation number into the "priv".

Than we don't need to complicate the test-suit script to do the
label normalization.

Chris

Re: [PATCH 0/2] normalize bb's label names for testing

[Luc Van Oostenryck]

On Tue, Nov 22, 2016 at 06:02:04PM +0800, Christopher Li wrote:
> On Mon, Nov 21, 2016 at 12:14 PM, Luc Van Oostenryck
> <luc.vanoostenryck@gmail.com> wrote:
> > These two patches solves the testing problem of the label names
> > as emitted by test-linearize and others being not comparable
> > between runs.
> 
> I think it is much easier just let test-linearize print out label in
> a predictable way.

Absolutely. It's what I'm doing here on my working tree.

> e.g. In the "struct basic_block", there is this
> two fields "generation" and "priv". The "generation" is used by
> flow analyze to store some state. Same as the "priv", which is
> used by the back end to store some back end related state.
> 
> Since test-linearize can treat as a back end, it is perfectly safe
> to store some allocation number into the "priv".

The problem with this 'priv' is that it's, well ... private, if we
reuse it to store labels IDs it possibly can't be used anymore for
this private usage.
I would be more tempted to reuse the 'generation' field which is a long
and don't need much bits.
Of course, the real question is if that's such important to not add a new
field to struct bb.

> Than we don't need to complicate the test-suit script to do the
> label normalization.

Sure.
But it should be noted that this filtering stuff *could* be useful for other
things to (but I have no such uses).

Luc

[PATCH] give comparable label's names to basic blocks

[Luc Van Oostenryck]

test-linearize displays basic block's labels by using
'.L0x' plus the address of the bb struct. This is certainly
convenient as an UID but it has the disadvantage that these
names change at each run and are thus not comparable.

This complicate testing quite a bit.

Let change this by giving a simple sequential number to each bb
and simply display them as: '.L1', '.L2', ...

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 linearize.c  | 30 ++++++++++++++++--------------
 linearize.h  |  5 ++++-
 test-unssa.c |  2 +-
 3 files changed, 21 insertions(+), 16 deletions(-)

diff --git a/linearize.c b/linearize.c
index c6ada1e8..cb252282 100644
--- a/linearize.c
+++ b/linearize.c
@@ -67,10 +67,12 @@ static struct entrypoint *alloc_entrypoint(void)
 
 static struct basic_block *alloc_basic_block(struct entrypoint *ep, struct position pos)
 {
+	static int nr;
 	struct basic_block *bb = __alloc_basic_block(0);
 	bb->context = -1;
 	bb->pos = pos;
 	bb->ep = ep;
+	bb->nr = nr++;
 	return bb;
 }
 
@@ -109,7 +111,7 @@ const char *show_pseudo(pseudo_t pseudo)
 		struct expression *expr;
 
 		if (sym->bb_target) {
-			snprintf(buf, 64, ".L%p", sym->bb_target);
+			snprintf(buf, 64, ".L%u", sym->bb_target->nr);
 			break;
 		}
 		if (sym->ident) {
@@ -303,10 +305,10 @@ const char *show_instruction(struct instruction *insn)
 		break;
 	case OP_BR:
 		if (insn->bb_true && insn->bb_false) {
-			buf += sprintf(buf, "%s, .L%p, .L%p", show_pseudo(insn->cond), insn->bb_true, insn->bb_false);
+			buf += sprintf(buf, "%s, .L%u, .L%u", show_pseudo(insn->cond), insn->bb_true->nr, insn->bb_false->nr);
 			break;
 		}
-		buf += sprintf(buf, ".L%p", insn->bb_true ? insn->bb_true : insn->bb_false);
+		buf += sprintf(buf, ".L%u", insn->bb_true ? insn->bb_true->nr : insn->bb_false->nr);
 		break;
 
 	case OP_SYMADDR: {
@@ -314,7 +316,7 @@ const char *show_instruction(struct instruction *insn)
 		buf += sprintf(buf, "%s <- ", show_pseudo(insn->target));
 
 		if (sym->bb_target) {
-			buf += sprintf(buf, ".L%p", sym->bb_target);
+			buf += sprintf(buf, ".L%u", sym->bb_target->nr);
 			break;
 		}
 		if (sym->ident) {
@@ -348,7 +350,7 @@ const char *show_instruction(struct instruction *insn)
 			buf += sprintf(buf, "%s", show_ident(expr->symbol->ident));
 			break;
 		case EXPR_LABEL:
-			buf += sprintf(buf, ".L%p", expr->symbol->bb_target);
+			buf += sprintf(buf, ".L%u", expr->symbol->bb_target->nr);
 			break;
 		default:
 			buf += sprintf(buf, "SETVAL EXPR TYPE %d", expr->type);
@@ -360,11 +362,11 @@ const char *show_instruction(struct instruction *insn)
 		buf += sprintf(buf, "%s", show_pseudo(insn->target));
 		FOR_EACH_PTR(insn->multijmp_list, jmp) {
 			if (jmp->begin == jmp->end)
-				buf += sprintf(buf, ", %d -> .L%p", jmp->begin, jmp->target);
+				buf += sprintf(buf, ", %d -> .L%u", jmp->begin, jmp->target->nr);
 			else if (jmp->begin < jmp->end)
-				buf += sprintf(buf, ", %d ... %d -> .L%p", jmp->begin, jmp->end, jmp->target);
+				buf += sprintf(buf, ", %d ... %d -> .L%u", jmp->begin, jmp->end, jmp->target->nr);
 			else
-				buf += sprintf(buf, ", default -> .L%p", jmp->target);
+				buf += sprintf(buf, ", default -> .L%u", jmp->target->nr);
 		} END_FOR_EACH_PTR(jmp);
 		break;
 	}
@@ -372,7 +374,7 @@ const char *show_instruction(struct instruction *insn)
 		struct multijmp *jmp;
 		buf += sprintf(buf, "%s", show_pseudo(insn->target));
 		FOR_EACH_PTR(insn->multijmp_list, jmp) {
-			buf += sprintf(buf, ", .L%p", jmp->target);
+			buf += sprintf(buf, ", .L%u", jmp->target->nr);
 		} END_FOR_EACH_PTR(jmp);
 		break;
 	}
@@ -473,7 +475,7 @@ void show_bb(struct basic_block *bb)
 {
 	struct instruction *insn;
 
-	printf(".L%p:\n", bb);
+	printf(".L%u:\n", bb->nr);
 	if (verbose) {
 		pseudo_t needs, defines;
 		printf("%s:%d\n", stream_name(bb->pos.stream), bb->pos.line);
@@ -481,7 +483,7 @@ void show_bb(struct basic_block *bb)
 		FOR_EACH_PTR(bb->needs, needs) {
 			struct instruction *def = needs->def;
 			if (def->opcode != OP_PHI) {
-				printf("  **uses %s (from .L%p)**\n", show_pseudo(needs), def->bb);
+				printf("  **uses %s (from .L%u)**\n", show_pseudo(needs), def->bb->nr);
 			} else {
 				pseudo_t phi;
 				const char *sep = " ";
@@ -489,7 +491,7 @@ void show_bb(struct basic_block *bb)
 				FOR_EACH_PTR(def->phi_list, phi) {
 					if (phi == VOID)
 						continue;
-					printf("%s(%s:.L%p)", sep, show_pseudo(phi), phi->def->bb);
+					printf("%s(%s:.L%u)", sep, show_pseudo(phi), phi->def->bb->nr);
 					sep = ", ";
 				} END_FOR_EACH_PTR(phi);		
 				printf(")**\n");
@@ -503,7 +505,7 @@ void show_bb(struct basic_block *bb)
 		if (bb->parents) {
 			struct basic_block *from;
 			FOR_EACH_PTR(bb->parents, from) {
-				printf("  **from %p (%s:%d:%d)**\n", from,
+				printf("  **from .L%u (%s:%d:%d)**\n", from->nr,
 					stream_name(from->pos.stream), from->pos.line, from->pos.pos);
 			} END_FOR_EACH_PTR(from);
 		}
@@ -511,7 +513,7 @@ void show_bb(struct basic_block *bb)
 		if (bb->children) {
 			struct basic_block *to;
 			FOR_EACH_PTR(bb->children, to) {
-				printf("  **to %p (%s:%d:%d)**\n", to,
+				printf("  **to .L%u (%s:%d:%d)**\n", to->nr,
 					stream_name(to->pos.stream), to->pos.line, to->pos.pos);
 			} END_FOR_EACH_PTR(to);
 		}
diff --git a/linearize.h b/linearize.h
index 61fbd831..f2ae140d 100644
--- a/linearize.h
+++ b/linearize.h
@@ -233,7 +233,10 @@ struct basic_block {
 	struct basic_block_list *children; /* destinations */
 	struct instruction_list *insns;	/* Linear list of instructions */
 	struct pseudo_list *needs, *defines;
-	void *priv;
+	union {
+		unsigned int nr;	/* unique id for label's names */
+		void *priv;
+	};
 };
 
 static inline int is_branch_goto(struct instruction *br)
diff --git a/test-unssa.c b/test-unssa.c
index 88ce025f..f7a5c3a1 100644
--- a/test-unssa.c
+++ b/test-unssa.c
@@ -12,7 +12,7 @@ static void output_bb(struct basic_block *bb, unsigned long generation)
 	struct instruction *insn;
 
 	bb->generation = generation;
-	printf(".L%p\n", bb);
+	printf(".L%u\n", bb->nr);
 
 	FOR_EACH_PTR(bb->insns, insn) {
 		if (!insn->bb)
-- 
2.10.2

Re: [PATCH 0/2] normalize bb's label names for testing

[Christopher Li]

On Tue, Nov 22, 2016 at 9:27 PM, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
>
> Absolutely. It's what I'm doing here on my working tree.
>

Great.

> The problem with this 'priv' is that it's, well ... private, if we
> reuse it to store labels IDs it possibly can't be used anymore for
> this private usage.
> I would be more tempted to reuse the 'generation' field which is a long
> and don't need much bits.
> Of course, the real question is if that's such important to not add a new
> field to struct bb.

It is private to the back end. If you consider test-linearize as one of
the back end, it compile the C code into readable text form. it is fine
to use it. I assume the label ID is only make sense for test-linearize,
not for other back end?

I am also OK with just introduce one more field ID for basic blocks.
There are far less basic blocks than instructions. So even size of basic
block bump up by a integer is not a big deal.

> Sure.
> But it should be noted that this filtering stuff *could* be useful for other
> things to (but I have no such uses).

We can always deal with it when such usage actually come up.

Chris

Re: [PATCH 0/2] normalize bb's label names for testing

[Luc Van Oostenryck]

On Wed, Nov 23, 2016 at 12:44:36AM +0800, Christopher Li wrote:
> 
> It is private to the back end. If you consider test-linearize as one of
> the back end, it compile the C code into readable text form. it is fine
> to use it. I assume the label ID is only make sense for test-linearize,
> not for other back end?

I use it for test-linearize, test-unssa and others stuff I'll coming up
with in the coming weeks I hope. So nothing I consider as 'private',
external or out-of-tree.
 
> I am also OK with just introduce one more field ID for basic blocks.
> There are far less basic blocks than instructions. So even size of basic
> block bump up by a integer is not a big deal.
Yes, I think so too.
But anyway, I already sent a patch which reuse the 'priv' member with
an union. It will always be very easy to remove the union later if needed.
 
> > Sure.
> > But it should be noted that this filtering stuff *could* be useful for other
> > things to (but I have no such uses).
> 
> We can always deal with it when such usage actually come up.
Absolutely.

Luc