On Tue, 19 Oct 2021 18:31:40 -0700
Kalesh Singh <kaleshsingh@xxxxxxxxxx> wrote:
> @@ -2391,60 +2460,61 @@ static int check_expr_operands(struct trace_array *tr,
> static struct hist_field *parse_expr(struct hist_trigger_data *hist_data,
> struct trace_event_file *file,
> char *str, unsigned long flags,
> - char *var_name, unsigned int level)
> + char *var_name, unsigned int *n_subexprs)
> {
> struct hist_field *operand1 = NULL, *operand2 = NULL, *expr = NULL;
> unsigned long operand_flags;
> int field_op, ret = -EINVAL;
> char *sep, *operand1_str;
>
> - if (level > 3) {
> + if (*n_subexprs > 3) {
Why limit the sub expressions, and not just keep the limit of the level of
recursion. We allow 3 levels of recursion, but we could have more than 3
sub expressions.
If we have: a * b + c / d - e * f / h
It would break down into:
-
+ /
* / * h
a b c d e f
Which I believe is 6 "sub expressions", but never goes more than three deep
in recursion:
"a * b + c / d - e * f / h"
Step 1:
op = "-"
operand1 = "a * b + c / d"
operand2 = "e * f / h"
Process operand1: (recursion level 1)
op = "+"
operand1a = "a * b"
operand2a = "c / d"
Process operand1a: (recursion level 2)
op = "*"
operand1b = "a"
operand2b = "b"
return;
Process operand1b: (recursion level 2)
op = "/"
operand1b = "c"
operand2b = "d"
return;
return;
Process operand2: (recursion level 1)
op = "/"
operand1c = "e * f"
operand2c = "h"
Process operand1c: (recursion level 2)
op = "*"
operand1c = "e"
operand2c = "f"
return;
return;
> +
> + /* LHS of string is an expression e.g. a+b in a+b+c */
> + operand1 = parse_expr(hist_data, file, operand1_str, operand_flags, NULL, n_subexprs);
> if (IS_ERR(operand1)) {
> ret = PTR_ERR(operand1);
> operand1 = NULL;
I wonder if we should look for optimizations, in case of operand1 and
operand2 are both constants?
Just perform the function, and convert it into a constant as well.
-- Steve
