From 073c93eebf6b197c13342a63a3ad322b058b0656 Mon Sep 17 00:00:00 2001
From: Bill Currie <bill@taniwha.org>
Date: Wed, 21 Feb 2024 20:43:33 +0900
Subject: [PATCH] [qfcc] Don't split basic blocks on function calls

This allows the dags code to optimize the return values, and when I make
the node killing by function calls less aggressive, should make for many
more potential CSE optimizations.
---
 tools/qfcc/include/dags.h      |   5 +-
 tools/qfcc/source/dags.c       | 100 ++++++++++++++++++---------------
 tools/qfcc/source/dot_dag.c    |   4 +-
 tools/qfcc/source/flow.c       |   5 +-
 tools/qfcc/source/statements.c |   3 +-
 5 files changed, 65 insertions(+), 52 deletions(-)

diff --git a/tools/qfcc/include/dags.h b/tools/qfcc/include/dags.h
index 44c17a183..b435cc5b2 100644
--- a/tools/qfcc/include/dags.h
+++ b/tools/qfcc/include/dags.h
@@ -62,7 +62,7 @@ typedef struct dagnode_s {
 	struct dagnode_s *killed;	///< node is unavailable for cse (by node)
 	st_type_t   type;			///< type of node (st_none = leaf)
 	daglabel_t *label;			///< ident/const if leaf node, or operator
-	const struct type_s *tl;
+	const struct type_s *vtype;	///< operand type
 	struct operand_s *value;	///< operand holding the value of this node
 	/// \name child nodes
 	/// if \a children[0] is null, the rest must be null as well. Similar for
@@ -89,11 +89,12 @@ typedef struct dag_s {
 	struct dag_s *next;
 	dagnode_t **nodes;			///< array of all dagnodes in this dag
 	int         num_nodes;
+	int         killer_node;	///< last mass-killer node
 	int        *topo;			///< nodes in topological sort order
 	int         num_topo;		///< number of nodes in topo (may be <
 								///< num_nodes after dead node removal)
-	daglabel_t **labels;		///< array of all daglabels in this dag
 	int         num_labels;
+	daglabel_t **labels;		///< array of all daglabels in this dag
 	struct set_s *roots;		///< set of root nodes
 	struct flownode_s *flownode;///< flow node this dag represents
 } dag_t;
diff --git a/tools/qfcc/source/dags.c b/tools/qfcc/source/dags.c
index a698f3777..ea361affc 100644
--- a/tools/qfcc/source/dags.c
+++ b/tools/qfcc/source/dags.c
@@ -333,7 +333,7 @@ leaf_node (dag_t *dag, operand_t *op, const expr_t *expr)
 	if (!op)
 		return 0;
 	node = new_node (dag);
-	node->tl = op->type;
+	node->vtype = op->type;
 	label = operand_label (dag, op);
 	label->dagnode = node;
 	label->expr = expr;
@@ -458,6 +458,9 @@ dag_make_child (dag_t *dag, operand_t *op, statement_t *s)
 		// No valid node found (either first reference to the value,
 		// or the value's node was killed).
 		node = leaf_node (dag, op, s->expr);
+		if (node && dag->killer_node >= 0) {
+			set_add (node->edges, dag->killer_node);
+		}
 	}
 	if (killer) {
 		// When an operand refers to a killed node, it must be
@@ -507,6 +510,9 @@ dagnode_add_children (dag_t *dag, dagnode_t *n, operand_t *operands[4],
 			set_add (child->parents, n->number);
 		}
 	}
+	if (operands[0]) {
+		n->vtype = operands[0]->type;
+	}
 }
 
 static int
@@ -891,7 +897,7 @@ dag_sort_nodes (dag_t *dag)
 }
 
 static void
-dag_kill_nodes (dag_t *dag, dagnode_t *n)
+dag_kill_nodes (dag_t *dag, dagnode_t *n, bool is_call)
 {
 	int         i;
 	dagnode_t  *node;
@@ -908,14 +914,14 @@ dag_kill_nodes (dag_t *dag, dagnode_t *n)
 			// point to itself (the required type is recursive).
 			continue;
 		}
-		if (op_is_constant (node->label->op)) {
+		if (!is_call && op_is_constant (node->label->op)) {
 			// While constants in the Quake VM can be changed via a pointer,
 			// doing so would cause much more fun than a simple
 			// mis-optimization would, so consider them safe from pointer
 			// operations.
 			continue;
 		}
-		if (op_is_temp (node->label->op)) {
+		if (!is_call && op_is_temp (node->label->op)) {
 			// Assume that the pointer cannot point to a temporary variable.
 			//  This is reasonable as there is no programmer access to temps.
 			continue;
@@ -923,6 +929,7 @@ dag_kill_nodes (dag_t *dag, dagnode_t *n)
 		node->killed = n;
 	}
 	n->killed = 0;
+	dag->killer_node = n->number;
 }
 
 static __attribute__((pure)) int
@@ -1017,6 +1024,7 @@ dag_create (flownode_t *flownode)
 	num_lables = num_statements * (FLOW_OPERANDS + 1 + 8) + num_aux;
 	dag->labels = alloca (num_lables * sizeof (daglabel_t));
 	dag->roots = set_new ();
+	dag->killer_node = -1;
 
 	// actual dag creation
 	for (s = block->statements; s; s = s->next) {
@@ -1067,7 +1075,10 @@ dag_create (flownode_t *flownode)
 			}
 		}
 		if (n->type == st_ptrassign) {
-			dag_kill_nodes (dag, n);
+			dag_kill_nodes (dag, n, false);
+		}
+		if (s->type == st_func && strcmp (s->opcode, "call") == 0) {
+			dag_kill_nodes (dag, n, true);
 		}
 	}
 
@@ -1289,43 +1300,6 @@ generate_memsetps (dag_t *dag, sblock_t *block, dagnode_t *dagnode)
 	return dst;
 }
 
-static operand_t *
-generate_call (dag_t *dag, sblock_t *block, dagnode_t *dagnode)
-{
-	set_iter_t *var_iter;
-	daglabel_t  *var = 0;
-	operand_t   *operands[3] = {0, 0, 0};
-	statement_t *st;
-	operand_t   *dst;
-
-	operands[0] = make_operand (dag, block, dagnode, 0);
-	if (dagnode->children[1]) {
-		operands[1] = make_operand (dag, block, dagnode, 1);
-	}
-	dst = operands[0];
-	for (var_iter = set_first (dagnode->identifiers); var_iter;
-		 var_iter = set_next (var_iter)) {
-		if (var) {
-			internal_error (var->expr, "more than one return value for call");
-		}
-		var = dag->labels[var_iter->element];
-		operands[2] = var->op;
-		dst = operands[2];
-		st = build_statement ("call", operands, var->expr);
-		sblock_add_statement (block, st);
-	}
-	if (var_iter) {
-		set_del_iter (var_iter);
-	}
-	if (!var) {
-		// void call or return value ignored, still have to call
-		operands[2] = make_operand (dag, block, dagnode, 2);
-		st = build_statement ("call", operands, dagnode->label->expr);
-		sblock_add_statement (block, st);
-	}
-	return dst;
-}
-
 static operand_t *
 generate_assignments (dag_t *dag, sblock_t *block, operand_t *src,
 					  set_iter_t *var_iter, const type_t *type)
@@ -1362,6 +1336,43 @@ generate_assignments (dag_t *dag, sblock_t *block, operand_t *src,
 	return dst;
 }
 
+static operand_t *
+generate_call (dag_t *dag, sblock_t *block, dagnode_t *dagnode)
+{
+	set_iter_t *var_iter;
+	daglabel_t  *var = 0;
+	operand_t   *operands[3] = {0, 0, 0};
+	statement_t *st;
+	operand_t   *dst = nullptr;
+	const type_t *type = dagnode->vtype;
+
+	operands[0] = make_operand (dag, block, dagnode, 0);
+	if (dagnode->children[1]) {
+		operands[1] = make_operand (dag, block, dagnode, 1);
+	}
+	if ((var_iter = set_first (dagnode->identifiers))) {
+		var = dag->labels[var_iter->element];
+		operands[2] = var->op;
+		var_iter = set_next (var_iter);
+		dst = operands[2];
+		st = build_statement ("call", operands, var->expr);
+		sblock_add_statement (block, st);
+		generate_assignments (dag, block, operands[2], var_iter, type);
+	}
+	if (!var) {
+		if (dagnode->children[2]) {
+			// void call or return value ignored, still have to call
+			operands[2] = make_operand (dag, block, dagnode, 2);
+		} else {
+			operands[2] = temp_operand (type, dagnode->label->expr);
+			dst = operands[2];
+		}
+		st = build_statement ("call", operands, dagnode->label->expr);
+		sblock_add_statement (block, st);
+	}
+	return dst;
+}
+
 static void
 dag_gencode (dag_t *dag, sblock_t *block, dagnode_t *dagnode)
 {
@@ -1412,10 +1423,9 @@ dag_gencode (dag_t *dag, sblock_t *block, dagnode_t *dagnode)
 			operands[0] = make_operand (dag, block, dagnode, 0);
 			if (dagnode->children[1])
 				operands[1] = make_operand (dag, block, dagnode, 1);
-			type = get_type (dagnode->label->expr);
+			type = dagnode->vtype;
 			if (!(var_iter = set_first (dagnode->identifiers))) {
-				operands[2] = temp_operand (get_type (dagnode->label->expr),
-											dagnode->label->expr);
+				operands[2] = temp_operand (type, dagnode->label->expr);
 			} else {
 				daglabel_t *var = dag->labels[var_iter->element];
 
diff --git a/tools/qfcc/source/dot_dag.c b/tools/qfcc/source/dot_dag.c
index 1671986e2..a375fd643 100644
--- a/tools/qfcc/source/dot_dag.c
+++ b/tools/qfcc/source/dot_dag.c
@@ -56,11 +56,11 @@ print_node_def (dstring_t *dstr, dag_t *dag, dagnode_t *node)
 	set_iter_t *id_iter;
 	daglabel_t *id;
 
-	dasprintf (dstr, "  \"dagnode_%p\" [%slabel=\"%s%s (%d)", node,
+	dasprintf (dstr, "  \"dagnode_%p\" [%slabel=\"%s%s (%d:%d)", node,
 			   node->type != st_none ? "" : "shape=box,",
 			   daglabel_string (node->label),
 			   node->killed ? " k" : "",
-			   node->topo);
+			   node->number, node->topo);
 	for (id_iter = set_first (node->identifiers); id_iter;
 		 id_iter = set_next (id_iter)) {
 		id = dag->labels[id_iter->element];
diff --git a/tools/qfcc/source/flow.c b/tools/qfcc/source/flow.c
index 683d5ec6d..835700dd8 100644
--- a/tools/qfcc/source/flow.c
+++ b/tools/qfcc/source/flow.c
@@ -1765,7 +1765,10 @@ flow_analyze_statement (statement_t *s, set_t *use, set_t *def, set_t *kill,
 			if (operands) {
 				operands[1] = s->opa;
 				operands[2] = s->opb;
-				operands[3] = s->opc;
+				if (calln >= 0 || (s->opc && s->opc->op_type == op_value)) {
+					operands[3] = s->opc;
+				}
+
 			}
 			break;
 		case st_flow:
diff --git a/tools/qfcc/source/statements.c b/tools/qfcc/source/statements.c
index a628e8729..b15ccc3e2 100644
--- a/tools/qfcc/source/statements.c
+++ b/tools/qfcc/source/statements.c
@@ -1251,8 +1251,7 @@ expr_call (sblock_t *sblock, const expr_t *call, operand_t **op)
 	s->use = use;
 	s->kill = kill;
 	sblock_add_statement (sblock, s);
-	sblock->next = new_sblock ();
-	return sblock->next;
+	return sblock;
 }
 
 static sblock_t *