Improved SCDF<->SCCP interface

dstogov · dstogov · commit db0cd64dfa7a · 2017-07-06T03:04:27.000+03:00
- "get_feasible_successors" callback is changed into "mark_feasible_successors" and should mark necessary edges through scdf_mark_edge_feasible()
 - SCDF takes care about OP_DATA instruction
 - SCDF code is re-arranged to avoid repeatable checks
diff --git a/ext/opcache/Optimizer/sccp.c b/ext/opcache/Optimizer/sccp.c
@@ -601,15 +601,10 @@ static inline int ct_eval_func_call(
 
 #define SKIP_IF_TOP(op) if (IS_TOP(op)) break;
 
-static void sccp_visit_instr(scdf_ctx *scdf, void *void_ctx, zend_op *opline, zend_ssa_op *ssa_op) {
-	sccp_ctx *ctx = (sccp_ctx *) void_ctx;
+static void sccp_visit_instr(scdf_ctx *scdf, zend_op *opline, zend_ssa_op *ssa_op) {
+	sccp_ctx *ctx = (sccp_ctx *) scdf->ctx;
 	zval *op1, *op2, zv; /* zv is a temporary to hold result values */
 
-	if (opline->opcode == ZEND_OP_DATA) {
-		opline--;
-		ssa_op--;
-	}
-
 	op1 = get_op1_value(ctx, opline, ssa_op);
 	op2 = get_op2_value(ctx, opline, ssa_op);
 
@@ -1032,11 +1027,13 @@ static void sccp_visit_instr(scdf_ctx *scdf, void *void_ctx, zend_op *opline, ze
 }
 
 /* Returns whether there is a successor */
-static zend_bool sccp_get_feasible_successors(
-		scdf_ctx *scdf, void *void_ctx, zend_basic_block *block,
-		zend_op *opline, zend_ssa_op *ssa_op, zend_bool *suc) {
-	sccp_ctx *ctx = (sccp_ctx *) void_ctx;
+static void sccp_mark_feasible_successors(
+		scdf_ctx *scdf,
+		int block_num, zend_basic_block *block,
+		zend_op *opline, zend_ssa_op *ssa_op) {
+	sccp_ctx *ctx = (sccp_ctx *) scdf->ctx;
 	zval *op1;
+	int s;
 
 	/* We can't determine the branch target at compile-time for these */
 	switch (opline->opcode) {
@@ -1046,51 +1043,56 @@ static zend_bool sccp_get_feasible_successors(
 		case ZEND_DECLARE_ANON_INHERITED_CLASS:
 		case ZEND_FE_FETCH_R:
 		case ZEND_FE_FETCH_RW:
-			suc[0] = 1;
-			suc[1] = 1;
-			return 1;
+			scdf_mark_edge_feasible(scdf, block_num, block->successors[0]);
+			scdf_mark_edge_feasible(scdf, block_num, block->successors[1]);
+			return;
 	}
 
 	op1 = get_op1_value(ctx, opline, ssa_op);
 
-	/* Branch target not yet known */
-	if (IS_TOP(op1)) {
-		return 0;
+	/* Branch target can be either one */
+	if (!op1 || IS_BOT(op1)) {
+		for (s = 0; s < block->successors_count; s++) {
+			scdf_mark_edge_feasible(scdf, block_num, block->successors[s]);
+		}
+		return;
 	}
 
-	/* Branch target can be either one */
-	if (IS_BOT(op1)) {
-		suc[0] = 1;
-		suc[1] = 1;
-		return 1;
+	/* Branch target not yet known */
+	if (IS_TOP(op1)) {
+		return;
 	}
 
 	switch (opline->opcode) {
 		case ZEND_JMPZ:
 		case ZEND_JMPZNZ:
 		case ZEND_JMPZ_EX:
-			suc[zend_is_true(op1)] = 1;
+			s = zend_is_true(op1);
 			break;
 		case ZEND_JMPNZ:
 		case ZEND_JMPNZ_EX:
 		case ZEND_JMP_SET:
-			suc[!zend_is_true(op1)] = 1;
+			s = !zend_is_true(op1);
 			break;
 		case ZEND_COALESCE:
-			suc[Z_TYPE_P(op1) == IS_NULL] = 1;
+			s = (Z_TYPE_P(op1) == IS_NULL);
 			break;
 		case ZEND_FE_RESET_R:
 		case ZEND_FE_RESET_RW:
-			if (Z_TYPE_P(op1) == IS_ARRAY) {
-				suc[zend_hash_num_elements(Z_ARR_P(op1)) != 0] = 1;
-			} else {
-				suc[0] = 1;
-				suc[1] = 1;
+			if (Z_TYPE_P(op1) != IS_ARRAY) {
+				scdf_mark_edge_feasible(scdf, block_num, block->successors[0]);
+				scdf_mark_edge_feasible(scdf, block_num, block->successors[1]);
+				return;
 			}
+			s = zend_hash_num_elements(Z_ARR_P(op1)) != 0;
 			break;
-		EMPTY_SWITCH_DEFAULT_CASE()
+		default:
+			for (s = 0; s < block->successors_count; s++) {
+				scdf_mark_edge_feasible(scdf, block_num, block->successors[s]);
+			}
+			return;
 	}
-	return 1;
+	scdf_mark_edge_feasible(scdf, block_num, block->successors[s]);
 }
 
 static void join_phi_values(zval *a, zval *b) {
@@ -1108,8 +1110,8 @@ static void join_phi_values(zval *a, zval *b) {
 	}
 }
 
-static void sccp_visit_phi(scdf_ctx *scdf, void *void_ctx, zend_ssa_phi *phi) {
-	sccp_ctx *ctx = (sccp_ctx *) void_ctx;
+static void sccp_visit_phi(scdf_ctx *scdf, zend_ssa_phi *phi) {
+	sccp_ctx *ctx = (sccp_ctx *) scdf->ctx;
 	zend_ssa *ssa = ctx->ssa;
 	ZEND_ASSERT(phi->ssa_var >= 0);
 	if (!IS_BOT(&ctx->values[phi->ssa_var])) {
@@ -1372,7 +1374,7 @@ void sccp_optimize_op_array(zend_op_array *op_array, zend_ssa *ssa, zend_call_in
 
 	scdf.handlers.visit_instr = sccp_visit_instr;
 	scdf.handlers.visit_phi = sccp_visit_phi;
-	scdf.handlers.get_feasible_successors = sccp_get_feasible_successors;
+	scdf.handlers.mark_feasible_successors = sccp_mark_feasible_successors;
 
 	scdf_init(&scdf, op_array, ssa, &ctx);
 	scdf_solve(&scdf, "SCCP");
diff --git a/ext/opcache/Optimizer/scdf.c b/ext/opcache/Optimizer/scdf.c
@@ -52,7 +52,7 @@
 #define DEBUG_PRINT(...)
 #endif
 
-static void mark_edge_feasible(scdf_ctx *ctx, int from, int to) {
+void scdf_mark_edge_feasible(scdf_ctx *ctx, int from, int to) {
 	uint32_t edge = scdf_edge(&ctx->ssa->cfg, from, to);
 
 	if (zend_bitset_in(ctx->feasible_edges, edge)) {
@@ -75,50 +75,7 @@ static void mark_edge_feasible(scdf_ctx *ctx, int from, int to) {
 		zend_ssa_phi *phi;
 		for (phi = ssa_block->phis; phi; phi = phi->next) {
 			zend_bitset_excl(ctx->phi_var_worklist, phi->ssa_var);
-			ctx->handlers.visit_phi(ctx, ctx->ctx, phi);
-		}
-	}
-}
-
-/* Returns whether there is a successor */
-static inline zend_bool get_feasible_successors(
-		scdf_ctx *ctx, zend_basic_block *block,
-		zend_op *opline, zend_ssa_op *ssa_op, zend_bool *suc) {
-	/* Terminal block without successors */
-	if (block->successors_count == 0) {
-		return 0;
-	}
-
-	/* Unconditional jump */
-	if (block->successors_count == 1) {
-		suc[0] = 1;
-		return 1;
-	}
-
-	return ctx->handlers.get_feasible_successors(ctx, ctx->ctx, block, opline, ssa_op, suc);
-}
-
-static void handle_instr(scdf_ctx *ctx, int block_num, zend_op *opline, zend_ssa_op *ssa_op) {
-	zend_basic_block *block = &ctx->ssa->cfg.blocks[block_num];
-	ctx->handlers.visit_instr(ctx, ctx->ctx, opline, ssa_op);
-
-	if (opline - ctx->op_array->opcodes == block->start + block->len - 1) {
-		if (opline->opcode == ZEND_SWITCH_LONG || opline->opcode == ZEND_SWITCH_STRING) {
-			// TODO For now consider all edges feasible
-			int s;
-			for (s = 0; s < block->successors_count; s++) {
-				mark_edge_feasible(ctx, block_num, block->successors[s]);
-			}
-		} else {
-			zend_bool suc[2] = {0};
-			if (get_feasible_successors(ctx, block, opline, ssa_op, suc)) {
-				if (suc[0]) {
-					mark_edge_feasible(ctx, block_num, block->successors[0]);
-				}
-				if (suc[1]) {
-					mark_edge_feasible(ctx, block_num, block->successors[1]);
-				}
-			}
+			ctx->handlers.visit_phi(ctx, phi);
 		}
 	}
 }
@@ -168,14 +125,28 @@ void scdf_solve(scdf_ctx *ctx, const char *name) {
 			zend_ssa_phi *phi = ssa->vars[i].definition_phi;
 			ZEND_ASSERT(phi);
 			if (zend_bitset_in(ctx->executable_blocks, phi->block)) {
-				ctx->handlers.visit_phi(ctx, ctx->ctx, phi);
+				ctx->handlers.visit_phi(ctx, phi);
 			}
 		}
 
 		while ((i = zend_bitset_pop_first(ctx->instr_worklist, ctx->instr_worklist_len)) >= 0) {
 			int block_num = ssa->cfg.map[i];
 			if (zend_bitset_in(ctx->executable_blocks, block_num)) {
-				handle_instr(ctx, block_num, &ctx->op_array->opcodes[i], &ssa->ops[i]);
+				zend_basic_block *block = &ssa->cfg.blocks[block_num];
+				zend_op *opline = &ctx->op_array->opcodes[i];
+				zend_ssa_op *ssa_op = &ssa->ops[i];
+				if (opline->opcode == ZEND_OP_DATA) {
+					opline--;
+					ssa_op--;
+				}
+				ctx->handlers.visit_instr(ctx, opline, ssa_op);
+				if (i == block->start + block->len - 1) {
+					if (block->successors_count == 1) {
+						scdf_mark_edge_feasible(ctx, block_num, block->successors[0]);
+					} else if (block->successors_count > 1) {
+						ctx->handlers.mark_feasible_successors(ctx, block_num, block, opline, ssa_op);
+					}
+				}
 			}
 		}
 
@@ -191,21 +162,32 @@ void scdf_solve(scdf_ctx *ctx, const char *name) {
 				zend_ssa_phi *phi;
 				for (phi = ssa_block->phis; phi; phi = phi->next) {
 					zend_bitset_excl(ctx->phi_var_worklist, phi->ssa_var);
-					ctx->handlers.visit_phi(ctx, ctx->ctx, phi);
+					ctx->handlers.visit_phi(ctx, phi);
 				}
 			}
 
-			{
+			if (block->len == 0) {
+				/* Zero length blocks don't have a last instruction that would normally do this */
+				scdf_mark_edge_feasible(ctx, i, block->successors[0]);
+			} else {
+				zend_op *opline;
 				int j, end = block->start + block->len;
 				for (j = block->start; j < end; j++) {
+					opline = &ctx->op_array->opcodes[j];
 					zend_bitset_excl(ctx->instr_worklist, j);
-					handle_instr(ctx, i, &ctx->op_array->opcodes[j], &ssa->ops[j]);
+					if (opline->opcode != ZEND_OP_DATA) {
+						ctx->handlers.visit_instr(ctx, opline, &ssa->ops[j]);
+					}
+				}
+				if (block->successors_count == 1) {
+					scdf_mark_edge_feasible(ctx, i, block->successors[0]);
+				} else if (block->successors_count > 1) {
+					if (opline->opcode == ZEND_OP_DATA) {
+						opline--;
+						j--;
+					}
+					ctx->handlers.mark_feasible_successors(ctx, i, block, opline, &ssa->ops[j-1]);
 				}
-			}
-
-			if (block->len == 0) {
-				/* Zero length blocks don't have a last instruction that would normally do this */
-				mark_edge_feasible(ctx, i, block->successors[0]);
 			}
 		}
 	}
diff --git a/ext/opcache/Optimizer/scdf.h b/ext/opcache/Optimizer/scdf.h
@@ -38,12 +38,12 @@ typedef struct _scdf_ctx {
 
 	struct {
 		void (*visit_instr)(
-			struct _scdf_ctx *scdf, void *ctx, zend_op *opline, zend_ssa_op *ssa_op);
+			struct _scdf_ctx *scdf, zend_op *opline, zend_ssa_op *ssa_op);
 		void (*visit_phi)(
-			struct _scdf_ctx *scdf, void *ctx, zend_ssa_phi *phi);
-		zend_bool (*get_feasible_successors)(
-			struct _scdf_ctx *scdf, void *ctx, zend_basic_block *block,
-			zend_op *opline, zend_ssa_op *ssa_op, zend_bool *suc);
+			struct _scdf_ctx *scdf, zend_ssa_phi *phi);
+		void (*mark_feasible_successors)(
+			struct _scdf_ctx *scdf, int block_num, zend_basic_block *block,
+			zend_op *opline, zend_ssa_op *ssa_op);
 	} handlers;
 } scdf_ctx;
 
@@ -96,4 +96,6 @@ static inline zend_bool scdf_is_edge_feasible(scdf_ctx *scdf, int from, int to)
 	return zend_bitset_in(scdf->feasible_edges, edge);
 }
 
+void scdf_mark_edge_feasible(scdf_ctx *ctx, int from, int to);
+
 #endif
