Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement e-SSA PI nodes #81

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Implement e-SSA PI nodes #81

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
73313a9
Implement e-SSA PI nodes
arnaud-lb Mar 26, 2024
e46d2bb
fixup
arnaud-lb Mar 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions ir.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1399,6 +1399,29 @@ bool ir_list_contains(const ir_list *l, ir_ref val)
return 0;
}

bool ir_remove_pis(ir_ctx *ctx)
{
ir_ref n, i;
ir_insn *insn;

for (i = IR_UNUSED + 1, insn = ctx->ir_base + i; i < ctx->insns_count;) {
n = insn->inputs_count;

if (insn->op == IR_PI) {
ir_use_list_remove_all(ctx, insn->op2, i);
insn->op2 = IR_UNUSED;
insn->op = IR_COPY;
insn->inputs_count = 1;
Comment on lines +1410 to +1414
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From the first look, I don't see a big reason in converting PI to COPY.

COPY is not going to be removed, and have to be handled by all the following passes. Actually, we may handle PI in the same way as COPY and keep the range info. On the other hand this will may additional overhead for all the following passes.

It's also possible to remove PI completely (turning it into NOP and re-connecting) input/def/use chains. T

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This may be done at the second phase of SCCP, that removes constant calculations and redundant copies.

I don't know what approach is better yet.

}

n = ir_insn_inputs_to_len(n);
i += n;
insn += n;
}

return true;
}

static uint32_t ir_hashtab_hash_size(uint32_t size)
{
size -= 1;
Expand Down
12 changes: 9 additions & 3 deletions ir.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -296,11 +296,14 @@ typedef enum _ir_type {
\
/* data-flow and miscellaneous ops */ \
_(PHI, pN, reg, def, def) /* SSA Phi function */ \
_(PI, d2, def, def, ___) /* e-SSA Pi constraint */ \
_(COPY, d1X1, def, opt, ___) /* COPY (last foldable op) */ \
_(PI, p2, reg, def, ___) /* e-SSA Pi constraint ??? */ \
_(FRAME_ADDR, d0, ___, ___, ___) /* function frame address */ \
/* (USE, RENAME) */ \
\
/* PI range */ \
_(RANGE, d2, def, def, ___) /* [min, max] */ \
\
Comment on lines +304 to +306
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer to make RANGE a "constant" (similar to FUNC/SYM/STR).
May be instead of RNNGE we could use PI.op2 and P.op3 as min and max?

/* data ops */ \
_(PARAM, p1X2, reg, str, num) /* incoming parameter proj. */ \
_(VAR, p1X1, reg, str, ___) /* local variable */ \
Expand Down Expand Up @@ -749,6 +752,7 @@ void ir_build_prev_refs(ir_ctx *ctx);

/* SCCP - Sparse Conditional Constant Propagation (implementation in ir_sccp.c) */
int ir_sccp(ir_ctx *ctx);
bool ir_remove_pis(ir_ctx *ctx);

/* GCM - Global Code Motion and scheduling (implementation in ir_gcm.c) */
int ir_gcm(ir_ctx *ctx);
Expand Down Expand Up @@ -908,7 +912,8 @@ IR_ALWAYS_INLINE void *ir_jit_compile(ir_ctx *ctx, int opt_level, size_t *size)

ir_build_def_use_lists(ctx);

if (!ir_build_cfg(ctx)
if (!ir_remove_pis(ctx)
|| !ir_build_cfg(ctx)
|| !ir_match(ctx)
|| !ir_assign_virtual_registers(ctx)
|| !ir_compute_dessa_moves(ctx)) {
Expand All @@ -930,7 +935,8 @@ IR_ALWAYS_INLINE void *ir_jit_compile(ir_ctx *ctx, int opt_level, size_t *size)
return NULL;
}

if (!ir_build_cfg(ctx)
if (!ir_remove_pis(ctx)
|| !ir_build_cfg(ctx)
|| !ir_build_dominators_tree(ctx)
|| !ir_find_loops(ctx)
|| !ir_gcm(ctx)
Expand Down
6 changes: 6 additions & 0 deletions ir_builder.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -471,6 +471,12 @@ extern "C" {
#define ir_PHI_N(type, _n, _inputs) _ir_PHI_N(_ir_CTX, type, (_n), (_inputs))
#define ir_PHI_SET_OP(_ref, _pos, _src) _ir_PHI_SET_OP(_ir_CTX, (_ref), (_pos), (_src))

// TODO: typed builders
#define ir_PI(_type, _ref, _range) ir_BINARY_OP(IR_PI, (_type), (_ref), (_range))

// TODO: typed builders
#define ir_RANGE(_type, _min, _max) ir_BINARY_OP(IR_PI, (_type), (_min), (_max))

#define ir_COPY(_type, _op1) ir_UNARY_OP(IR_COPY, (_type), (_op1))
#define ir_COPY_B(_op1) ir_UNARY_OP_B(IR_COPY, (_op1))
#define ir_COPY_U8(_op1) ir_UNARY_OP_U8(IR_COPY, (_op1))
Expand Down
7 changes: 7 additions & 0 deletions ir_check.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,13 @@ bool ir_check(const ir_ctx *ctx)
i, j, use, use_insn->type, insn->type);
ok = 0;
}
if (insn->op == IR_PI) {
if (j == 2 && use_insn->op != IR_RANGE) {
fprintf(stderr, "ir_base[%d].ops[%d] (%d) op must be RANGE (%d)\n",
i, j, use_insn->op, IR_RANGE);
ok = 0;
}
}
break;
}
}
Expand Down
2 changes: 1 addition & 1 deletion ir_emit_c.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -407,7 +407,7 @@ static void ir_emit_bitcast(ir_ctx *ctx, FILE *f, int def, ir_insn *insn)
fprintf(f, " = _u.d;}\n");
} else {
IR_ASSERT(insn->type == IR_FLOAT);
fprintf(f, "\t{union {float f; uint32_t bits;} _u; _u.buts = ");
fprintf(f, "\t{union {float f; uint32_t bits;} _u; _u.bits = ");
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've merged this typo fix.

ir_emit_ref(ctx, f, insn->op1);
fprintf(f, "; ");
ir_emit_ref(ctx, f, def);
Expand Down
114 changes: 114 additions & 0 deletions ir_fold.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1352,6 +1352,23 @@ IR_FOLD(FP2FP(C_DOUBLE))
}
}

IR_FOLD(PI(C_BOOL, _))
IR_FOLD(PI(C_U8, _))
IR_FOLD(PI(C_U16, _))
IR_FOLD(PI(C_U32, _))
IR_FOLD(PI(C_U64, _))
IR_FOLD(PI(C_ADDR, _))
IR_FOLD(PI(C_CHAR, _))
IR_FOLD(PI(C_I8, _))
IR_FOLD(PI(C_I16, _))
IR_FOLD(PI(C_I32, _))
IR_FOLD(PI(C_I64, _))
IR_FOLD(PI(C_DOUBLE, _))
IR_FOLD(PI(C_FLOAT, _))
{
IR_FOLD_COPY(op1);
}

// TODO: constant functions (e.g. sin, cos)

/* Copy Propagation */
Expand Down Expand Up @@ -1426,6 +1443,103 @@ IR_FOLD(BSWAP(BSWAP))
IR_FOLD_COPY(op1_insn->op1);
}

IR_FOLD(EQ(PI, C_BOOL))
IR_FOLD(EQ(PI, C_U8))
IR_FOLD(EQ(PI, C_U16))
IR_FOLD(EQ(PI, C_U32))
IR_FOLD(EQ(PI, C_U64))
IR_FOLD(EQ(PI, C_ADDR))
{
ir_insn *range_insn = &ctx->ir_base[op1_insn->op2];
IR_ASSERT(range_insn->op == IR_RANGE);
if (IR_IS_CONST_REF(range_insn->op1)) {
if (op2_insn->val.u64 < ctx->ir_base[range_insn->op1].val.u64) {
IR_FOLD_COPY(IR_FALSE);
}
}
if (IR_IS_CONST_REF(range_insn->op2)) {
if (op2_insn->val.u64 > ctx->ir_base[range_insn->op2].val.u64) {
IR_FOLD_COPY(IR_FALSE);
}
}
if (IR_IS_CONST_REF(range_insn->op1) && IR_IS_CONST_REF(range_insn->op2)) {
if (ctx->ir_base[range_insn->op1].val.u64 == ctx->ir_base[range_insn->op2].val.u64) {
IR_FOLD_COPY(IR_TRUE);
}
}
IR_FOLD_NEXT;
}

IR_FOLD(EQ(PI, C_CHAR))
IR_FOLD(EQ(PI, C_I8))
IR_FOLD(EQ(PI, C_I16))
IR_FOLD(EQ(PI, C_I32))
IR_FOLD(EQ(PI, C_I64))
{
ir_insn *range_insn = &ctx->ir_base[op1_insn->op2];
IR_ASSERT(range_insn->op == IR_RANGE);
if (IR_IS_CONST_REF(range_insn->op1)) {
if (op2_insn->val.i64 < ctx->ir_base[range_insn->op1].val.i64) {
IR_FOLD_COPY(IR_FALSE);
}
}
if (IR_IS_CONST_REF(range_insn->op2)) {
if (op2_insn->val.i64 > ctx->ir_base[range_insn->op2].val.i64) {
IR_FOLD_COPY(IR_FALSE);
}
}
if (IR_IS_CONST_REF(range_insn->op1) && IR_IS_CONST_REF(range_insn->op2)) {
if (ctx->ir_base[range_insn->op1].val.i64 == ctx->ir_base[range_insn->op2].val.i64) {
IR_FOLD_COPY(IR_TRUE);
}
}
IR_FOLD_NEXT;
}

IR_FOLD(EQ(PI, C_DOUBLE))
{
Comment on lines +1499 to +1500
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Floating point numbers might need additional checks for NAN (and may be for negative zero and INF).
I thought, I won't implement range support for them.

ir_insn *range_insn = &ctx->ir_base[op1_insn->op2];
IR_ASSERT(range_insn->op == IR_RANGE);
if (IR_IS_CONST_REF(range_insn->op1)) {
if (op2_insn->val.d < ctx->ir_base[range_insn->op1].val.d) {
IR_FOLD_COPY(IR_FALSE);
}
}
if (IR_IS_CONST_REF(range_insn->op2)) {
if (op2_insn->val.d > ctx->ir_base[range_insn->op2].val.d) {
IR_FOLD_COPY(IR_FALSE);
}
}
if (IR_IS_CONST_REF(range_insn->op1) && IR_IS_CONST_REF(range_insn->op2)) {
if (ctx->ir_base[range_insn->op1].val.d == ctx->ir_base[range_insn->op2].val.d) {
IR_FOLD_COPY(IR_TRUE);
}
}
IR_FOLD_NEXT;
}

IR_FOLD(EQ(PI, C_FLOAT))
{
ir_insn *range_insn = &ctx->ir_base[op1_insn->op2];
IR_ASSERT(range_insn->op == IR_RANGE);
if (IR_IS_CONST_REF(range_insn->op1)) {
if (op2_insn->val.f < ctx->ir_base[range_insn->op1].val.f) {
IR_FOLD_COPY(IR_FALSE);
}
}
if (IR_IS_CONST_REF(range_insn->op2)) {
if (op2_insn->val.f > ctx->ir_base[range_insn->op2].val.f) {
IR_FOLD_COPY(IR_FALSE);
}
}
if (IR_IS_CONST_REF(range_insn->op1) && IR_IS_CONST_REF(range_insn->op2)) {
if (ctx->ir_base[range_insn->op1].val.f == ctx->ir_base[range_insn->op2].val.f) {
IR_FOLD_COPY(IR_TRUE);
}
}
IR_FOLD_NEXT;
}

IR_FOLD(EQ(_, C_BOOL))
{
if (op2 == IR_TRUE) {
Expand Down
2 changes: 2 additions & 0 deletions ir_main.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,8 @@ int ir_compile_func(ir_ctx *ctx, int opt_level, uint32_t save_flags, uint32_t du
#endif
}

ir_remove_pis(ctx);

if (opt_level > 0 || (ctx->flags & (IR_GEN_NATIVE|IR_GEN_CODE))) {
ir_build_cfg(ctx);
if ((dump & (IR_DUMP_AFTER_CFG|IR_DUMP_AFTER_ALL))
Expand Down
1 change: 1 addition & 0 deletions ir_private.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -525,6 +525,7 @@ IR_ALWAYS_INLINE int ir_bitqueue_pop(ir_bitqueue *q)

IR_ALWAYS_INLINE void ir_bitqueue_add(ir_bitqueue *q, uint32_t n)
{
IR_ASSERT(n > 0);
uint32_t i = n / IR_BITSET_BITS;
q->set[i] |= IR_BITSET_ONE << (n % IR_BITSET_BITS);
if (i < q->pos) {
Expand Down
56 changes: 56 additions & 0 deletions ir_sccp.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -550,6 +550,24 @@ static void ir_sccp_remove_if(ir_ctx *ctx, ir_insn *_values, ir_ref ref, ir_ref
}
}

static void ir_sccp_remove_guard(ir_ctx *ctx, ir_ref ref, ir_ref dst)
{
ir_ref use;
ir_insn *insn, *use_insn;
ir_use_list *use_list = &ctx->use_lists[ref];

insn = &ctx->ir_base[ref];
IR_ASSERT(use_list->count == 1);
use = ctx->use_edges[use_list->refs];
if (use == dst) {
use_insn = &ctx->ir_base[use];
use_insn->op1 = insn->op1;
ir_use_list_replace_one(ctx, insn->op1, ref, use);
/* remove GUARD/GUARD_NOT instruction */
ir_sccp_make_nop(ctx, ref);
}
}

static void ir_sccp_remove_unfeasible_merge_inputs(ir_ctx *ctx, ir_insn *_values, ir_ref ref, ir_ref unfeasible_inputs)
{
ir_ref i, j, n, k, *p, use;
Expand Down Expand Up @@ -1069,6 +1087,41 @@ int ir_sccp(ir_ctx *ctx)
continue;
}
IR_MAKE_BOTTOM(i);
} else if (insn->op == IR_GUARD || insn->op == IR_GUARD_NOT) {
if (IR_IS_TOP(insn->op2)) {
/* do backward propagaton only once */
if (!_values[insn->op2].op1) {
_values[insn->op2].op1 = 1;
ir_bitqueue_add(&worklist, insn->op2);
}
continue;
}
if (!IR_IS_BOTTOM(insn->op2)
#if IR_COMBO_COPY_PROPAGATION
&& (IR_IS_CONST_REF(insn->op2) || _values[insn->op2].op != IR_COPY)
#endif
) {
bool b = ir_sccp_is_true(ctx, _values, insn->op2);
use_list = &ctx->use_lists[i];
IR_ASSERT(use_list->count == 1);
p = &ctx->use_edges[use_list->refs];
use = *p;
use_insn = &ctx->ir_base[use];
if ((insn->op == IR_GUARD) != b) {
use = IR_UNUSED;
}
if (_values[i].optx == IR_TOP) {
_values[i].optx = IR_GUARD;
_values[i].op1 = use;
} else if (_values[i].optx != IR_GUARD || _values[i].op1 != use) {
IR_MAKE_BOTTOM(i);
}
if (use > 0 && !IR_IS_BOTTOM(use)) {
ir_bitqueue_add(&worklist, use);
}
continue;
}
IR_MAKE_BOTTOM(i);
Comment on lines +1090 to +1124
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It may be better not to add a special handling for GUARDS at this stage, but check for them during constant substitution (on the second SCCP stage). e.g. when we attempt to make GUARD(_, const, _), we may decide to eliminate it. The elimination itself may be done on the third SCCP stage (similar to dead loads).

Actually, this may be profitable even without ranges and PI. Can you try this in a separate PR?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the other hand, handling GUARDs at first stage allows not only eliminate the "false" GUARDs, but also allows eliminate all the code after the "true" GUARDs. I didn't get if your code does this or not.

} else if (insn->op == IR_SWITCH) {
if (IR_IS_TOP(insn->op2)) {
/* do backward propagaton only once */
Expand Down Expand Up @@ -1232,6 +1285,9 @@ int ir_sccp(ir_ctx *ctx)
} else if (value->op == IR_IF) {
/* remove one way IF/SWITCH */
ir_sccp_remove_if(ctx, _values, i, value->op1);
} else if (value->op == IR_GUARD) {
/* remove one way GUARD/GUARD_NOT */
ir_sccp_remove_guard(ctx, i, value->op1);
} else if (value->op == IR_MERGE) {
/* schedule merge to remove unfeasible MERGE inputs */
ir_bitqueue_add(&worklist, i);
Expand Down
28 changes: 28 additions & 0 deletions tests/dead_guard_001.irt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
--TEST--
001: Dead guard
--CODE--
{
int32_t d_1 = 0;
uintptr_t d_2 = 0;
uintptr_t d_3 = 1;
l_1 = START(l_12);
int32_t d_4 = PARAM(l_1, "x", 0);
bool d_5 = EQ(d_4, d_2);
l_4 = GUARD(l_1, d_4, d_1);
int32_t d_6 = RANGE(d_2, d_2);
int32_t d_7 = PI(d_4, d_6);
bool d_8 = EQ(d_7, d_2);
l_5 = GUARD(l_4, d_8, d_1);
l_12 = RETURN(l_5);
}
--EXPECT--
{
uintptr_t c_1 = 0;
bool c_2 = 0;
bool c_3 = 1;
int32_t c_4 = 0;
l_1 = START(l_4);
int32_t d_2 = PARAM(l_1, "x", 0);
l_3 = GUARD(l_1, d_2, c_4);
l_4 = RETURN(l_3, null);
}
21 changes: 21 additions & 0 deletions tests/folding/fold_014.irt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
--TEST--
014: FOLD: PI(A, CONST) => CONST
--CODE--
{
l_1 = START(l_6);
int32_t d_1 = 2;
int32_t d_2 = 1;
int32_t d_3 = ADD(d_2, d_2);
int32_t d_4 = RANGE(d_2, d_1);
int32_t d_5 = PI(d_3, d_4);
l_6 = RETURN(l_1, d_5);
}
--EXPECT--
{
uintptr_t c_1 = 0;
bool c_2 = 0;
bool c_3 = 1;
int32_t c_4 = 2;
l_1 = START(l_2);
l_2 = RETURN(l_1, c_4);
}
Loading
Loading