From: Alan T. DeKok Date: Thu, 25 Aug 2022 14:30:04 +0000 (-0400) Subject: move peephole optimization to xlat_purify_op() X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=a7cbb982399003914ef3f1e3d23188bd7c1b8e40;p=thirdparty%2Ffreeradius-server.git move peephole optimization to xlat_purify_op() --- diff --git a/src/lib/unlang/xlat.h b/src/lib/unlang/xlat.h index a2e91faa763..44052de623e 100644 --- a/src/lib/unlang/xlat.h +++ b/src/lib/unlang/xlat.h @@ -417,6 +417,8 @@ void xlat_instances_free(void); typedef struct unlang_interpret_s unlang_interpret_t; int xlat_purify(xlat_exp_head_t *head, unlang_interpret_t *intp); +int xlat_purify_op(TALLOC_CTX *ctx, xlat_exp_t **out, xlat_exp_t *lhs, fr_token_t op, xlat_exp_t *rhs); + /* * xlat.c */ diff --git a/src/lib/unlang/xlat_expr.c b/src/lib/unlang/xlat_expr.c index 331a2aafb19..e8e99ddad29 100644 --- a/src/lib/unlang/xlat_expr.c +++ b/src/lib/unlang/xlat_expr.c @@ -2461,107 +2461,6 @@ static bool valid_type(xlat_exp_t *node) } -static fr_value_box_t *xlat_value_box(xlat_exp_t *node) -{ - if (!node) return NULL; - - if (node->type == XLAT_BOX) { - return &node->data; - - } else if ((node->type == XLAT_TMPL) && tmpl_is_data(node->vpt)) { - return tmpl_value(node->vpt); - } - - return NULL; -} - - -static bool is_truthy(xlat_exp_t *node, bool *out) -{ - fr_value_box_t const *box; - - box = xlat_value_box(node); - if (!box) { - *out = false; - return false; - } - - *out = fr_value_box_is_truthy(box); - return true; -} - -/* - * Do some optimizations. - * - * @todo - check for tail of LHS - * - * if ((lhs->type == XLAT_FUNC) && (lhs->call.func->token == op)) - * && tail is truthy, then remove tail, replace it with RHS - * and return LHS. - * - * lhs->call.args->flags.can_purify |= rhs->flags.can_purify | rhs->flags.pure; - * lhs->flags.can_purify = lhs->call.args->flags.can_purify; - */ -static xlat_exp_t *logical_peephole_optimize(xlat_exp_t *lhs, fr_token_t op, xlat_exp_t *rhs) -{ - bool value; - - if (!is_truthy(lhs, &value)) return NULL; - - /* - * 1 && FOO --> FOO - * 0 && FOO --> 0 - * FOO && BAR --> FOO && BAR - */ - - /* - * 1 || FOO --> 1 - * 0 || FOO --> FOO - * FOO || BAR --> FOO || BAR - */ - if (value == (op != T_LAND)) { - talloc_free(rhs); - return lhs; - } - - talloc_free(lhs); - return rhs; -} - - -/* - * Do some optimizations - */ -static int binary_peephole_optimize(TALLOC_CTX *ctx, xlat_exp_t **out, xlat_exp_t *lhs, fr_token_t op, xlat_exp_t *rhs) -{ - fr_value_box_t *lhs_box, *rhs_box; - fr_value_box_t box; - xlat_exp_t *node; - char *name; - - lhs_box = xlat_value_box(lhs); - if (!lhs_box) return 0; - - rhs_box = xlat_value_box(rhs); - if (!rhs_box) return 0; - - if (fr_value_calc_binary_op(lhs, &box, FR_TYPE_NULL, lhs_box, op, rhs_box) < 0) return -1; - - MEM(node = xlat_exp_alloc_null(ctx)); - xlat_exp_set_type(node, XLAT_BOX); - - if (box.type == FR_TYPE_BOOL) box.enumv = attr_expr_bool_enum; - - (void) fr_value_box_aprint(node, &name, &box, NULL); - - xlat_exp_set_name_buffer_shallow(node, name); - fr_value_box_copy(node, &node->data, &box); - - *out = node; - - return 1; -} - /** Tokenize a mathematical operation. * * (EXPR) @@ -2734,8 +2633,8 @@ redo: fr_assert(func != NULL); /* - * If it's a logical operator, check for rcodes, and also - * merge sequences of the same operator together. + * If it's a logical operator, check for rcodes, and then + * try to purify the results. */ if (logical_ops[op]) { if (reparse_rcode(head, &rhs, true) < 0) { @@ -2753,23 +2652,11 @@ redo: } if (reparse_rcode(head, &lhs, true) < 0) goto fail_lhs; - - /* - * Peephole optimizer. - * - * FOO || 0 --> FOO - * FOO && 1 --> 1 - */ - node = logical_peephole_optimize(lhs, op, rhs); - if (node) { - replace: - lhs = node; - goto redo; - } + goto purify; } /* - * Remove invalid comparisons. + * Complain on comparisons between invalid data types. * * @todo - allow * @@ -2790,10 +2677,14 @@ redo: if (cond) { int rcode; - rcode = binary_peephole_optimize(head, &node, lhs, op, rhs); + purify: + rcode = xlat_purify_op(head, &node, lhs, op, rhs); if (rcode < 0) goto fail_lhs; - if (rcode) goto replace; + if (rcode) { + lhs = node; + goto redo; + } } } diff --git a/src/lib/unlang/xlat_purify.c b/src/lib/unlang/xlat_purify.c index 077802a328a..edf5e69cd15 100644 --- a/src/lib/unlang/xlat_purify.c +++ b/src/lib/unlang/xlat_purify.c @@ -28,6 +28,7 @@ RCSID("$Id$") #include #include +#include static void xlat_value_list_to_xlat(xlat_exp_head_t *head, fr_value_box_list_t *list) { @@ -211,3 +212,125 @@ int xlat_purify(xlat_exp_head_t *head, unlang_interpret_t *intp) return rcode; } + +static fr_value_box_t *xlat_value_box(xlat_exp_t *node) +{ +#ifdef STATIC_ANALYZER + if (!node) return NULL; +#endif + + if (node->type == XLAT_BOX) { + return &node->data; + + } else if ((node->type == XLAT_TMPL) && tmpl_is_data(node->vpt)) { + return tmpl_value(node->vpt); + } + + return NULL; +} + + +static bool is_truthy(xlat_exp_t *node, bool *out) +{ + fr_value_box_t const *box; + + box = xlat_value_box(node); + if (!box) { + *out = false; + return false; + } + + *out = fr_value_box_is_truthy(box); + return true; +} + +/* + * Do some optimizations. + * + */ +static xlat_exp_t *logical_peephole_optimize(xlat_exp_t *lhs, fr_token_t op, xlat_exp_t *rhs) +{ + bool value; + + /* + * @todo - check for tail of LHS + * && tail is truthy, then remove tail, and call ourselves recursively + * if there's a new node, it becomes the new tail. Otherwise + * we append the rhs to the lhs args. + * + * lhs->call.args->flags.can_purify |= rhs->flags.can_purify | rhs->flags.pure; + * lhs->flags.can_purify = lhs->call.args->flags.can_purify; + */ + if (!is_truthy(lhs, &value)) return NULL; + + /* + * 1 && FOO --> FOO + * 0 && FOO --> 0 + * FOO && BAR --> FOO && BAR + */ + + /* + * 1 || FOO --> 1 + * 0 || FOO --> FOO + * FOO || BAR --> FOO || BAR + */ + if (value == (op != T_LAND)) { + talloc_free(rhs); + return lhs; + } + + talloc_free(lhs); + return rhs; +} + + +/* + * Do some optimizations + * + * @todo check types, if one side is uint8, and the other side is uint32, there are some situations where + * the comparison will always fail. And should therefore be invalid? + */ +static int binary_peephole_optimize(TALLOC_CTX *ctx, xlat_exp_t **out, xlat_exp_t *lhs, fr_token_t op, xlat_exp_t *rhs) +{ + fr_value_box_t *lhs_box, *rhs_box; + fr_value_box_t box; + xlat_exp_t *node; + char *name; + + lhs_box = xlat_value_box(lhs); + if (!lhs_box) return 0; + + rhs_box = xlat_value_box(rhs); + if (!rhs_box) return 0; + + if (fr_value_calc_binary_op(lhs, &box, FR_TYPE_NULL, lhs_box, op, rhs_box) < 0) return -1; + + MEM(node = xlat_exp_alloc_null(ctx)); + xlat_exp_set_type(node, XLAT_BOX); + + if (box.type == FR_TYPE_BOOL) box.enumv = attr_expr_bool_enum; + + (void) fr_value_box_aprint(node, &name, &box, NULL); + + xlat_exp_set_name_buffer_shallow(node, name); + fr_value_box_copy(node, &node->data, &box); + + *out = node; + + return 1; +} + +int xlat_purify_op(TALLOC_CTX *ctx, xlat_exp_t **out, xlat_exp_t *lhs, fr_token_t op, xlat_exp_t *rhs) +{ + if ((op == T_LAND) || (op == T_LOR)) { + xlat_exp_t *node; + + node = logical_peephole_optimize(lhs, op, rhs); + if (!node) return 0; + + *out = node; + return 1; + } + + return binary_peephole_optimize(ctx, out, lhs, op, rhs); +}