Re: [PATCH v8 3/8] perf expr: Modify code layout

From: Jiri Olsa <jolsa@redhat.com>
To: Ian Rogers <irogers@google.com>
Cc: Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@redhat.com>,
	Arnaldo Carvalho de Melo <acme@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Namhyung Kim <namhyung@kernel.org>,
	linux-kernel@vger.kernel.org, Andi Kleen <ak@linux.intel.com>,
	Jin Yao <yao.jin@linux.intel.com>,
	John Garry <john.garry@huawei.com>, Paul Clarke <pc@us.ibm.com>,
	kajoljain <kjain@linux.ibm.com>,
	linux-perf-users@vger.kernel.org,
	Stephane Eranian <eranian@google.com>,
	Sandeep Dasgupta <sdasgup@google.com>
Subject: Re: [PATCH v8 3/8] perf expr: Modify code layout
Date: Wed, 22 Sep 2021 22:58:47 +0200	[thread overview]
Message-ID: <YUuZBwiW/0M7yosQ@krava> (raw)
In-Reply-To: <20210918063513.2356923-4-irogers@google.com>

On Fri, Sep 17, 2021 at 11:35:08PM -0700, Ian Rogers wrote:
> No functional change. Alter whitespace. Use helper macros that will be
> made more complex in a later change.

please split into more patches, with the spacing changes
it's not easy to check there's no functional change

thanks,
jirka

> 
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
>  tools/perf/util/expr.y | 149 +++++++++++++++++++++++------------------
>  1 file changed, 85 insertions(+), 64 deletions(-)
> 
> diff --git a/tools/perf/util/expr.y b/tools/perf/util/expr.y
> index 41c9cd4efadd..51f4f0aa1955 100644
> --- a/tools/perf/util/expr.y
> +++ b/tools/perf/util/expr.y
> @@ -2,23 +2,10 @@
>  %{
>  #define YYDEBUG 1
>  #include <math.h>
> -#include <stdio.h>
> -#include "util.h"
>  #include "util/debug.h"
> -#include <stdlib.h> // strtod()
> +#include "smt.h"
>  #define IN_EXPR_Y 1
>  #include "expr.h"
> -#include "smt.h"
> -#include <string.h>
> -
> -static double d_ratio(double val0, double val1)
> -{
> -	if (val1 == 0) {
> -		return 0;
> -	}
> -	return  val0 / val1;
> -}
> -
>  %}
>  
>  %define api.pure full
> @@ -33,11 +20,7 @@ static double d_ratio(double val0, double val1)
>  	char	*str;
>  }
>  
> -%token EXPR_PARSE EXPR_OTHER EXPR_ERROR
> -%token <num> NUMBER
> -%token <str> ID
> -%destructor { free ($$); } <str>
> -%token MIN MAX IF ELSE SMT_ON D_RATIO
> +%token ID NUMBER MIN MAX IF ELSE SMT_ON D_RATIO EXPR_ERROR EXPR_PARSE EXPR_OTHER
>  %left MIN MAX IF
>  %left '|'
>  %left '^'
> @@ -46,6 +29,9 @@ static double d_ratio(double val0, double val1)
>  %left '-' '+'
>  %left '*' '/' '%'
>  %left NEG NOT
> +%type <num> NUMBER
> +%type <str> ID
> +%destructor { free ($$); } <str>
>  %type <num> expr if_expr
>  
>  %{
> @@ -57,6 +43,12 @@ static void expr_error(double *final_val __maybe_unused,
>  	pr_debug("%s\n", s);
>  }
>  
> +#define BINARY_LONG_OP(RESULT, OP, LHS, RHS)				\
> +	RESULT = (long)LHS OP (long)RHS;
> +
> +#define BINARY_OP(RESULT, OP, LHS, RHS)					\
> +	RESULT = LHS OP RHS;
> +
>  %}
>  %%
>  
> @@ -78,49 +70,78 @@ MIN | MAX | IF | ELSE | SMT_ON | NUMBER | '|' | '^' | '&' | '-' | '+' | '*' | '/
>  '<' | '>' | D_RATIO
>  
>  all_expr: if_expr			{ *final_val = $1; }
> -	;
> -
> -if_expr:
> -	expr IF expr ELSE expr { $$ = $3 ? $1 : $5; }
> -	| expr
> -	;
> -
> -expr:	  NUMBER
> -	| ID			{
> -					struct expr_id_data *data;
> -
> -					$$ = NAN;
> -					if (expr__resolve_id(ctx, $1, &data) == 0)
> -						$$ = expr_id_data__value(data);
> -
> -					free($1);
> -				}
> -	| expr '|' expr		{ $$ = (long)$1 | (long)$3; }
> -	| expr '&' expr		{ $$ = (long)$1 & (long)$3; }
> -	| expr '^' expr		{ $$ = (long)$1 ^ (long)$3; }
> -	| expr '<' expr		{ $$ = $1 < $3; }
> -	| expr '>' expr		{ $$ = $1 > $3; }
> -	| expr '+' expr		{ $$ = $1 + $3; }
> -	| expr '-' expr		{ $$ = $1 - $3; }
> -	| expr '*' expr		{ $$ = $1 * $3; }
> -	| expr '/' expr		{ if ($3 == 0) {
> -					pr_debug("division by zero\n");
> -					YYABORT;
> -				  }
> -				  $$ = $1 / $3;
> -	                        }
> -	| expr '%' expr		{ if ((long)$3 == 0) {
> -					pr_debug("division by zero\n");
> -					YYABORT;
> -				  }
> -				  $$ = (long)$1 % (long)$3;
> -	                        }
> -	| '-' expr %prec NEG	{ $$ = -$2; }
> -	| '(' if_expr ')'	{ $$ = $2; }
> -	| MIN '(' expr ',' expr ')' { $$ = $3 < $5 ? $3 : $5; }
> -	| MAX '(' expr ',' expr ')' { $$ = $3 > $5 ? $3 : $5; }
> -	| SMT_ON		 { $$ = smt_on() > 0; }
> -	| D_RATIO '(' expr ',' expr ')' { $$ = d_ratio($3,$5); }
> -	;
>  
> -%%
> +if_expr: expr IF expr ELSE expr
> +{
> +	$$ = $3 ? $1 : $5;
> +}
> +| expr
> +;
> +
> +expr: NUMBER
> +{
> +	$$ = $1;
> +}
> +| ID
> +{
> +	struct expr_id_data *data;
> +
> +	$$ = NAN;
> +	if (expr__resolve_id(ctx, $1, &data) == 0)
> +		$$ = expr_id_data__value(data);
> +
> +	free($1);
> +}
> +| expr '|' expr { BINARY_LONG_OP($$, |, $1, $3); }
> +| expr '&' expr { BINARY_LONG_OP($$, &, $1, $3); }
> +| expr '^' expr { BINARY_LONG_OP($$, ^, $1, $3); }
> +| expr '<' expr { BINARY_OP($$, <, $1, $3); }
> +| expr '>' expr { BINARY_OP($$, >, $1, $3); }
> +| expr '+' expr { BINARY_OP($$, +, $1, $3); }
> +| expr '-' expr { BINARY_OP($$, -, $1, $3); }
> +| expr '*' expr { BINARY_OP($$, *, $1, $3); }
> +| expr '/' expr
> +{
> +	if ($3 == 0) {
> +		pr_debug("division by zero\n");
> +		YYABORT;
> +	}
> +	$$ = $1 / $3;
> +}
> +| expr '%' expr
> +{
> +	if ((long)$3 == 0) {
> +		pr_debug("division by zero\n");
> +		YYABORT;
> +	}
> +	$$ = (long)$1 % (long)$3;
> +}
> +| D_RATIO '(' expr ',' expr ')'
> +{
> +	if ($5 == 0) {
> +		$$ = 0;
> +	} else {
> +		$$ = $3 / $5;
> +	}
> +}
> +| '-' expr %prec NEG
> +{
> +	$$ = -$2;
> +}
> +| '(' if_expr ')'
> +{
> +	$$ = $2;
> +}
> +| MIN '(' expr ',' expr ')'
> +{
> +	$$ = $3 < $5 ? $3 : $5;
> +}
> +| MAX '(' expr ',' expr ')'
> +{
> +	$$ = $3 > $5 ? $3 : $5;
> +}
> +| SMT_ON
> +{
> +	$$ = smt_on() > 0 ? 1.0 : 0.0;
> +}
> +;
> -- 
> 2.33.0.464.g1972c5931b-goog
>