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.

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

wants to merge 2 commits into from

Conversation

arnaud-lb
Copy link
Contributor

This implements e-SSA PI constraint nodes, without range propagation.

This uses the folding engine to replace comparison ops into constants when possible, and in turn the SCCP engine can elide IF/SWITCH/GUARD. I only implemented folding for EQ right now, but I will add other ops. I also did not add support for symbolic ranges.

PI node signature has the form PI(def, RANGE(min, max)) (op2 is a RANGE op). It should be possible to change this to PI(def, min, max), but the folding engine supports only two operands currently.

PI nodes must be added manually, like PHI nodes. They are replaced by COPY ops after SCCP so that other passes do not have to be aware of them.

Comment on lines +1410 to +1414
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;
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.

Copy link
Owner

@dstogov dstogov left a comment

Choose a reason for hiding this comment

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

Look. I'm not ready for this.
I don't have a complete picture in my head yet (how e-SSA and ranges should be implemented and handled) .
This PR is a start of a big research work and nobody can estimate the profit and cost of the future solution.
I'm appreciate your initial attempt and would be glad if you could try to continue this research.

As I sorter term sub-task, I would recommend to think about the constant GUARD elimination.

Comment on lines +304 to +306
/* PI range */ \
_(RANGE, d2, def, def, ___) /* [min, max] */ \
\
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?

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.

Comment on lines +1499 to +1500
IR_FOLD(EQ(PI, C_DOUBLE))
{
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.

Comment on lines +1090 to +1124
} 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);
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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants