* [PATCH 1/5] perf expr: Add expr.c object
2020-02-24 8:29 [PATCHv2 0/5] perf expr: Add flex scanner Jiri Olsa
@ 2020-02-24 8:29 ` Jiri Olsa
2020-02-24 8:29 ` [PATCH 2/5] perf expr: Move expr lexer to flex Jiri Olsa
` (4 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Jiri Olsa @ 2020-02-24 8:29 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: lkml, Ingo Molnar, Namhyung Kim, Alexander Shishkin,
Peter Zijlstra, Michael Petlan, Ravi Bangoria, Andi Kleen,
Kajol Jain, John Garry
Add generic expr code into new expr.c object.
The expr.c object will be mainly used in following
change that will get rid of the manual flex code,
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
tools/perf/util/Build | 1 +
tools/perf/util/expr.c | 19 +++++++++++++++++++
tools/perf/util/expr.y | 16 ----------------
3 files changed, 20 insertions(+), 16 deletions(-)
create mode 100644 tools/perf/util/expr.c
diff --git a/tools/perf/util/Build b/tools/perf/util/Build
index 07da6c790b63..6fdf073093a6 100644
--- a/tools/perf/util/Build
+++ b/tools/perf/util/Build
@@ -122,6 +122,7 @@ perf-y += vsprintf.o
perf-y += units.o
perf-y += time-utils.o
perf-y += expr-bison.o
+perf-y += expr.o
perf-y += branch.o
perf-y += mem2node.o
diff --git a/tools/perf/util/expr.c b/tools/perf/util/expr.c
new file mode 100644
index 000000000000..816b23b2068a
--- /dev/null
+++ b/tools/perf/util/expr.c
@@ -0,0 +1,19 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <assert.h>
+#include "expr.h"
+
+/* Caller must make sure id is allocated */
+void expr__add_id(struct parse_ctx *ctx, const char *name, double val)
+{
+ int idx;
+
+ assert(ctx->num_ids < MAX_PARSE_ID);
+ idx = ctx->num_ids++;
+ ctx->ids[idx].name = name;
+ ctx->ids[idx].val = val;
+}
+
+void expr__ctx_init(struct parse_ctx *ctx)
+{
+ ctx->num_ids = 0;
+}
diff --git a/tools/perf/util/expr.y b/tools/perf/util/expr.y
index 7d226241f1d7..7cea8b7c3a5c 100644
--- a/tools/perf/util/expr.y
+++ b/tools/perf/util/expr.y
@@ -6,7 +6,6 @@
#define IN_EXPR_Y 1
#include "expr.h"
#include "smt.h"
-#include <assert.h>
#include <string.h>
#define MAXIDLEN 256
@@ -169,21 +168,6 @@ static int expr__lex(YYSTYPE *res, const char **pp)
return tok;
}
-/* Caller must make sure id is allocated */
-void expr__add_id(struct parse_ctx *ctx, const char *name, double val)
-{
- int idx;
- assert(ctx->num_ids < MAX_PARSE_ID);
- idx = ctx->num_ids++;
- ctx->ids[idx].name = name;
- ctx->ids[idx].val = val;
-}
-
-void expr__ctx_init(struct parse_ctx *ctx)
-{
- ctx->num_ids = 0;
-}
-
static bool already_seen(const char *val, const char *one, const char **other,
int num_other)
{
--
2.24.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 2/5] perf expr: Move expr lexer to flex
2020-02-24 8:29 [PATCHv2 0/5] perf expr: Add flex scanner Jiri Olsa
2020-02-24 8:29 ` [PATCH 1/5] perf expr: Add expr.c object Jiri Olsa
@ 2020-02-24 8:29 ` Jiri Olsa
2020-02-24 8:29 ` [PATCH 3/5] perf expr: Increase EXPR_MAX_OTHER Jiri Olsa
` (3 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Jiri Olsa @ 2020-02-24 8:29 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: lkml, Ingo Molnar, Namhyung Kim, Alexander Shishkin,
Peter Zijlstra, Michael Petlan, Ravi Bangoria, Andi Kleen,
Kajol Jain, John Garry
Adding expr flex code instead of the manual parser
code. So it's easily extensible in upcoming changes.
The new flex code is in flex.l object and gets compiled
like all the other flexers we use. It's defined as flex
reentrant parser.
It's used by both expr__parse and expr__find_other
interfaces by separating the starting point.
There's no intended change of functionality ;-) the
test expr is passing.
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
tools/perf/util/Build | 10 ++-
tools/perf/util/expr.c | 93 ++++++++++++++++++++
tools/perf/util/expr.h | 2 -
tools/perf/util/expr.l | 87 +++++++++++++++++++
tools/perf/util/expr.y | 192 ++++++++++++-----------------------------
5 files changed, 243 insertions(+), 141 deletions(-)
create mode 100644 tools/perf/util/expr.l
diff --git a/tools/perf/util/Build b/tools/perf/util/Build
index 6fdf073093a6..c0cf8dff694e 100644
--- a/tools/perf/util/Build
+++ b/tools/perf/util/Build
@@ -121,6 +121,7 @@ perf-y += mem-events.o
perf-y += vsprintf.o
perf-y += units.o
perf-y += time-utils.o
+perf-y += expr-flex.o
perf-y += expr-bison.o
perf-y += expr.o
perf-y += branch.o
@@ -190,9 +191,13 @@ $(OUTPUT)util/parse-events-bison.c: util/parse-events.y
$(call rule_mkdir)
$(Q)$(call echo-cmd,bison)$(BISON) -v util/parse-events.y -d $(PARSER_DEBUG_BISON) -o $@ -p parse_events_
+$(OUTPUT)util/expr-flex.c: util/expr.l $(OUTPUT)util/expr-bison.c
+ $(call rule_mkdir)
+ $(Q)$(call echo-cmd,flex)$(FLEX) -o $@ --header-file=$(OUTPUT)util/expr-flex.h $(PARSER_DEBUG_FLEX) util/expr.l
+
$(OUTPUT)util/expr-bison.c: util/expr.y
$(call rule_mkdir)
- $(Q)$(call echo-cmd,bison)$(BISON) -v util/expr.y -d $(PARSER_DEBUG_BISON) -o $@ -p expr__
+ $(Q)$(call echo-cmd,bison)$(BISON) -v util/expr.y -d $(PARSER_DEBUG_BISON) -o $@ -p expr_
$(OUTPUT)util/pmu-flex.c: util/pmu.l $(OUTPUT)util/pmu-bison.c
$(call rule_mkdir)
@@ -204,12 +209,14 @@ $(OUTPUT)util/pmu-bison.c: util/pmu.y
CFLAGS_parse-events-flex.o += -w
CFLAGS_pmu-flex.o += -w
+CFLAGS_expr-flex.o += -w
CFLAGS_parse-events-bison.o += -DYYENABLE_NLS=0 -w
CFLAGS_pmu-bison.o += -DYYENABLE_NLS=0 -DYYLTYPE_IS_TRIVIAL=0 -w
CFLAGS_expr-bison.o += -DYYENABLE_NLS=0 -DYYLTYPE_IS_TRIVIAL=0 -w
$(OUTPUT)util/parse-events.o: $(OUTPUT)util/parse-events-flex.c $(OUTPUT)util/parse-events-bison.c
$(OUTPUT)util/pmu.o: $(OUTPUT)util/pmu-flex.c $(OUTPUT)util/pmu-bison.c
+$(OUTPUT)util/expr.o: $(OUTPUT)util/expr-flex.c $(OUTPUT)util/expr-bison.c
CFLAGS_bitmap.o += -Wno-unused-parameter -DETC_PERFCONFIG="BUILD_STR($(ETC_PERFCONFIG_SQ))"
CFLAGS_find_bit.o += -Wno-unused-parameter -DETC_PERFCONFIG="BUILD_STR($(ETC_PERFCONFIG_SQ))"
@@ -217,6 +224,7 @@ CFLAGS_rbtree.o += -Wno-unused-parameter -DETC_PERFCONFIG="BUILD_STR($(ET
CFLAGS_libstring.o += -Wno-unused-parameter -DETC_PERFCONFIG="BUILD_STR($(ETC_PERFCONFIG_SQ))"
CFLAGS_hweight.o += -Wno-unused-parameter -DETC_PERFCONFIG="BUILD_STR($(ETC_PERFCONFIG_SQ))"
CFLAGS_parse-events.o += -Wno-redundant-decls
+CFLAGS_expr.o += -Wno-redundant-decls
CFLAGS_header.o += -include $(OUTPUT)PERF-VERSION-FILE
$(OUTPUT)util/kallsyms.o: ../lib/symbol/kallsyms.c FORCE
diff --git a/tools/perf/util/expr.c b/tools/perf/util/expr.c
index 816b23b2068a..b39fd39f10ec 100644
--- a/tools/perf/util/expr.c
+++ b/tools/perf/util/expr.c
@@ -1,6 +1,14 @@
// SPDX-License-Identifier: GPL-2.0
+#include <stdbool.h>
#include <assert.h>
#include "expr.h"
+#include "expr-bison.h"
+#define YY_EXTRA_TYPE int
+#include "expr-flex.h"
+
+#ifdef PARSER_DEBUG
+extern int expr_debug;
+#endif
/* Caller must make sure id is allocated */
void expr__add_id(struct parse_ctx *ctx, const char *name, double val)
@@ -17,3 +25,88 @@ void expr__ctx_init(struct parse_ctx *ctx)
{
ctx->num_ids = 0;
}
+
+static int
+__expr__parse(double *val, struct parse_ctx *ctx, const char *expr,
+ int start)
+{
+ YY_BUFFER_STATE buffer;
+ void *scanner;
+ int ret;
+
+ ret = expr_lex_init_extra(start, &scanner);
+ if (ret)
+ return ret;
+
+ buffer = expr__scan_string(expr, scanner);
+
+#ifdef PARSER_DEBUG
+ expr_debug = 1;
+#endif
+
+ ret = expr_parse(val, ctx, scanner);
+
+ expr__flush_buffer(buffer, scanner);
+ expr__delete_buffer(buffer, scanner);
+ expr_lex_destroy(scanner);
+ return ret;
+}
+
+int expr__parse(double *final_val, struct parse_ctx *ctx, const char **pp)
+{
+ return __expr__parse(final_val, ctx, *pp, EXPR_PARSE);
+}
+
+static bool
+already_seen(const char *val, const char *one, const char **other,
+ int num_other)
+{
+ int i;
+
+ if (one && !strcasecmp(one, val))
+ return true;
+ for (i = 0; i < num_other; i++)
+ if (!strcasecmp(other[i], val))
+ return true;
+ return false;
+}
+
+int expr__find_other(const char *p, const char *one, const char ***other,
+ int *num_other)
+{
+ int err, i = 0, j = 0;
+ struct parse_ctx ctx;
+
+ expr__ctx_init(&ctx);
+ err = __expr__parse(NULL, &ctx, p, EXPR_OTHER);
+ if (err)
+ return -1;
+
+ *other = malloc((ctx.num_ids + 1) * sizeof(char *));
+ if (!*other)
+ return -ENOMEM;
+
+ for (i = 0, j = 0; i < ctx.num_ids; i++) {
+ const char *str = ctx.ids[i].name;
+
+ if (already_seen(str, one, *other, j))
+ continue;
+
+ str = strdup(str);
+ if (!str)
+ goto out;
+ (*other)[j++] = str;
+ }
+ (*other)[j] = NULL;
+
+out:
+ if (i != ctx.num_ids) {
+ while (--j)
+ free((char *) (*other)[i]);
+ free(*other);
+ err = -1;
+ }
+
+ *num_other = j;
+ return err;
+}
diff --git a/tools/perf/util/expr.h b/tools/perf/util/expr.h
index 046160831f90..9332796e6649 100644
--- a/tools/perf/util/expr.h
+++ b/tools/perf/util/expr.h
@@ -17,9 +17,7 @@ struct parse_ctx {
void expr__ctx_init(struct parse_ctx *ctx);
void expr__add_id(struct parse_ctx *ctx, const char *id, double val);
-#ifndef IN_EXPR_Y
int expr__parse(double *final_val, struct parse_ctx *ctx, const char **pp);
-#endif
int expr__find_other(const char *p, const char *one, const char ***other,
int *num_other);
diff --git a/tools/perf/util/expr.l b/tools/perf/util/expr.l
new file mode 100644
index 000000000000..ebaa80736536
--- /dev/null
+++ b/tools/perf/util/expr.l
@@ -0,0 +1,87 @@
+%option prefix="expr_"
+%option reentrant
+%option bison-bridge
+
+%{
+#include <linux/compiler.h>
+#include "expr.h"
+#include "expr-bison.h"
+
+char *expr_get_text(yyscan_t yyscanner);
+YYSTYPE *expr_get_lval(yyscan_t yyscanner);
+
+static int __value(YYSTYPE *yylval, char *str, int base, int token)
+{
+ u64 num;
+
+ errno = 0;
+ num = strtoull(str, NULL, base);
+ if (errno)
+ return EXPR_ERROR;
+
+ yylval->num = num;
+ return token;
+}
+
+static int value(yyscan_t scanner, int base)
+{
+ YYSTYPE *yylval = expr_get_lval(scanner);
+ char *text = expr_get_text(scanner);
+
+ return __value(yylval, text, base, NUMBER);
+}
+
+static int str(yyscan_t scanner, int token)
+{
+ YYSTYPE *yylval = expr_get_lval(scanner);
+ char *text = expr_get_text(scanner);
+
+ yylval->str = strdup(text);
+ return token;
+}
+%}
+
+number [0-9]+
+
+sch [-,=]
+spec \\{sch}
+sym [0-9a-zA-Z_\.:@]+
+symbol {spec}*{sym}*{spec}*{sym}*
+
+%%
+ {
+ int start_token;
+
+ start_token = parse_events_get_extra(yyscanner);
+
+ if (start_token) {
+ parse_events_set_extra(NULL, yyscanner);
+ return start_token;
+ }
+ }
+
+max { return MAX; }
+min { return MIN; }
+if { return IF; }
+else { return ELSE; }
+#smt_on { return SMT_ON; }
+{number} { return value(yyscanner, 10); }
+{symbol} { return str(yyscanner, ID); }
+"|" { return '|'; }
+"^" { return '^'; }
+"&" { return '&'; }
+"-" { return '-'; }
+"+" { return '+'; }
+"*" { return '*'; }
+"/" { return '/'; }
+"%" { return '%'; }
+"(" { return '('; }
+")" { return ')'; }
+"," { return ','; }
+. { }
+%%
+
+int expr_wrap(void *scanner __maybe_unused)
+{
+ return 1;
+}
diff --git a/tools/perf/util/expr.y b/tools/perf/util/expr.y
index 7cea8b7c3a5c..9a87024bce5b 100644
--- a/tools/perf/util/expr.y
+++ b/tools/perf/util/expr.y
@@ -1,5 +1,7 @@
/* Simple expression parser */
%{
+#define YYDEBUG 1
+#include <stdio.h>
#include "util.h"
#include "util/debug.h"
#include <stdlib.h> // strtod()
@@ -8,23 +10,23 @@
#include "smt.h"
#include <string.h>
-#define MAXIDLEN 256
%}
%define api.pure full
%parse-param { double *final_val }
%parse-param { struct parse_ctx *ctx }
-%parse-param { const char **pp }
-%lex-param { const char **pp }
+%parse-param {void *scanner}
+%lex-param {void* scanner}
%union {
- double num;
- char id[MAXIDLEN+1];
+ double num;
+ char *str;
}
+%token EXPR_PARSE EXPR_OTHER EXPR_ERROR
%token <num> NUMBER
-%token <id> ID
+%token <str> ID
%token MIN MAX IF ELSE SMT_ON
%left MIN MAX IF
%left '|'
@@ -36,11 +38,9 @@
%type <num> expr if_expr
%{
-static int expr__lex(YYSTYPE *res, const char **pp);
-
-static void expr__error(double *final_val __maybe_unused,
+static void expr_error(double *final_val __maybe_unused,
struct parse_ctx *ctx __maybe_unused,
- const char **pp __maybe_unused,
+ void *scanner,
const char *s)
{
pr_debug("%s\n", s);
@@ -59,9 +59,53 @@ static int lookup_id(struct parse_ctx *ctx, char *id, double *val)
return -1;
}
+/*
+ * Allow @ instead of / to be able to specify pmu/event/ without
+ * conflicts with normal division.
+ */
+static char *normalize(char *str)
+{
+ char *ret = str;
+ char *dst = str;
+
+ while (*str) {
+ if (*str == '@')
+ *dst++ = '/';
+ else if (*str == '\\')
+ *dst++ = *++str;
+ else
+ *dst++ = *str;
+ str++;
+ }
+
+ *dst = 0x0;
+ return ret;
+}
+
%}
%%
+start:
+EXPR_PARSE all_expr
+|
+EXPR_OTHER all_other
+
+all_other: all_other other
+|
+
+other: ID
+{
+ if (ctx->num_ids + 1 >= EXPR_MAX_OTHER) {
+ pr_err("failed: way too many variables");
+ YYABORT;
+ }
+
+ ctx->ids[ctx->num_ids++].name = normalize($1);
+}
+|
+MIN | MAX | IF | ELSE | SMT_ON | NUMBER | '|' | '^' | '&' | '-' | '+' | '*' | '/' | '%' | '(' | ')'
+
+
all_expr: if_expr { *final_val = $1; }
;
@@ -92,131 +136,3 @@ expr: NUMBER
;
%%
-
-static int expr__symbol(YYSTYPE *res, const char *p, const char **pp)
-{
- char *dst = res->id;
- const char *s = p;
-
- if (*p == '#')
- *dst++ = *p++;
-
- while (isalnum(*p) || *p == '_' || *p == '.' || *p == ':' || *p == '@' || *p == '\\') {
- if (p - s >= MAXIDLEN)
- return -1;
- /*
- * Allow @ instead of / to be able to specify pmu/event/ without
- * conflicts with normal division.
- */
- if (*p == '@')
- *dst++ = '/';
- else if (*p == '\\')
- *dst++ = *++p;
- else
- *dst++ = *p;
- p++;
- }
- *dst = 0;
- *pp = p;
- dst = res->id;
- switch (dst[0]) {
- case 'm':
- if (!strcmp(dst, "min"))
- return MIN;
- if (!strcmp(dst, "max"))
- return MAX;
- break;
- case 'i':
- if (!strcmp(dst, "if"))
- return IF;
- break;
- case 'e':
- if (!strcmp(dst, "else"))
- return ELSE;
- break;
- case '#':
- if (!strcasecmp(dst, "#smt_on"))
- return SMT_ON;
- break;
- }
- return ID;
-}
-
-static int expr__lex(YYSTYPE *res, const char **pp)
-{
- int tok;
- const char *s;
- const char *p = *pp;
-
- while (isspace(*p))
- p++;
- s = p;
- switch (*p++) {
- case '#':
- case 'a' ... 'z':
- case 'A' ... 'Z':
- return expr__symbol(res, p - 1, pp);
- case '0' ... '9': case '.':
- res->num = strtod(s, (char **)&p);
- tok = NUMBER;
- break;
- default:
- tok = *s;
- break;
- }
- *pp = p;
- return tok;
-}
-
-static bool already_seen(const char *val, const char *one, const char **other,
- int num_other)
-{
- int i;
-
- if (one && !strcasecmp(one, val))
- return true;
- for (i = 0; i < num_other; i++)
- if (!strcasecmp(other[i], val))
- return true;
- return false;
-}
-
-int expr__find_other(const char *p, const char *one, const char ***other,
- int *num_otherp)
-{
- const char *orig = p;
- int err = -1;
- int num_other;
-
- *other = malloc((EXPR_MAX_OTHER + 1) * sizeof(char *));
- if (!*other)
- return -1;
-
- num_other = 0;
- for (;;) {
- YYSTYPE val;
- int tok = expr__lex(&val, &p);
- if (tok == 0) {
- err = 0;
- break;
- }
- if (tok == ID && !already_seen(val.id, one, *other, num_other)) {
- if (num_other >= EXPR_MAX_OTHER - 1) {
- pr_debug("Too many extra events in %s\n", orig);
- break;
- }
- (*other)[num_other] = strdup(val.id);
- if (!(*other)[num_other])
- return -1;
- num_other++;
- }
- }
- (*other)[num_other] = NULL;
- *num_otherp = num_other;
- if (err) {
- *num_otherp = 0;
- free(*other);
- *other = NULL;
- }
- return err;
-}
--
2.24.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 3/5] perf expr: Increase EXPR_MAX_OTHER
2020-02-24 8:29 [PATCHv2 0/5] perf expr: Add flex scanner Jiri Olsa
2020-02-24 8:29 ` [PATCH 1/5] perf expr: Add expr.c object Jiri Olsa
2020-02-24 8:29 ` [PATCH 2/5] perf expr: Move expr lexer to flex Jiri Olsa
@ 2020-02-24 8:29 ` Jiri Olsa
2020-02-24 21:03 ` Andi Kleen
2020-02-24 8:29 ` [PATCH 4/5] perf expr: Straighten expr__parse/expr__find_other interface Jiri Olsa
` (2 subsequent siblings)
5 siblings, 1 reply; 10+ messages in thread
From: Jiri Olsa @ 2020-02-24 8:29 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: lkml, Ingo Molnar, Namhyung Kim, Alexander Shishkin,
Peter Zijlstra, Michael Petlan, Ravi Bangoria, Andi Kleen,
Kajol Jain, John Garry
There's no need to be greedy on allowed variables, also
when some of the metrics define more than 15 variables,
like Branch_Misprediction_Cost.
Increasing the maximum to 100.
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
tools/perf/util/expr.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/perf/util/expr.h b/tools/perf/util/expr.h
index 9332796e6649..01e1529bdfd9 100644
--- a/tools/perf/util/expr.h
+++ b/tools/perf/util/expr.h
@@ -2,7 +2,7 @@
#ifndef PARSE_CTX_H
#define PARSE_CTX_H 1
-#define EXPR_MAX_OTHER 15
+#define EXPR_MAX_OTHER 100
#define MAX_PARSE_ID EXPR_MAX_OTHER
struct parse_id {
--
2.24.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 3/5] perf expr: Increase EXPR_MAX_OTHER
2020-02-24 8:29 ` [PATCH 3/5] perf expr: Increase EXPR_MAX_OTHER Jiri Olsa
@ 2020-02-24 21:03 ` Andi Kleen
0 siblings, 0 replies; 10+ messages in thread
From: Andi Kleen @ 2020-02-24 21:03 UTC (permalink / raw)
To: Jiri Olsa
Cc: Arnaldo Carvalho de Melo, lkml, Ingo Molnar, Namhyung Kim,
Alexander Shishkin, Peter Zijlstra, Michael Petlan,
Ravi Bangoria, Kajol Jain, John Garry
On Mon, Feb 24, 2020 at 09:29:16AM +0100, Jiri Olsa wrote:
> There's no need to be greedy on allowed variables, also
> when some of the metrics define more than 15 variables,
> like Branch_Misprediction_Cost.
>
> Increasing the maximum to 100.
FWIW some of the algorithms (e.g. already_seen) have O(n^2) complexity.
If you really want that many probably would need to add some hash tables.
-Andi
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 4/5] perf expr: Straighten expr__parse/expr__find_other interface
2020-02-24 8:29 [PATCHv2 0/5] perf expr: Add flex scanner Jiri Olsa
` (2 preceding siblings ...)
2020-02-24 8:29 ` [PATCH 3/5] perf expr: Increase EXPR_MAX_OTHER Jiri Olsa
@ 2020-02-24 8:29 ` Jiri Olsa
2020-02-24 8:29 ` [PATCH 5/5] perf expr: Make expr__parse return -1 on error Jiri Olsa
2020-02-27 12:10 ` [PATCHv2 0/5] perf expr: Add flex scanner Jiri Olsa
5 siblings, 0 replies; 10+ messages in thread
From: Jiri Olsa @ 2020-02-24 8:29 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: lkml, Ingo Molnar, Namhyung Kim, Alexander Shishkin,
Peter Zijlstra, Michael Petlan, Ravi Bangoria, Andi Kleen,
Kajol Jain, John Garry
Now with flex parser we don't need to update parsed string
pointer, so the interface can just passed the pointer to the
expression instead of pointer to pointer.
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
tools/perf/tests/expr.c | 6 +++---
tools/perf/util/expr.c | 8 ++++----
tools/perf/util/expr.h | 4 ++--
tools/perf/util/stat-shadow.c | 4 +---
4 files changed, 10 insertions(+), 12 deletions(-)
diff --git a/tools/perf/tests/expr.c b/tools/perf/tests/expr.c
index 87843af4c118..755d73c86c68 100644
--- a/tools/perf/tests/expr.c
+++ b/tools/perf/tests/expr.c
@@ -10,7 +10,7 @@ static int test(struct parse_ctx *ctx, const char *e, double val2)
{
double val;
- if (expr__parse(&val, ctx, &e))
+ if (expr__parse(&val, ctx, e))
TEST_ASSERT_VAL("parse test failed", 0);
TEST_ASSERT_VAL("unexpected value", val == val2);
return 0;
@@ -44,11 +44,11 @@ int test__expr(struct test *t __maybe_unused, int subtest __maybe_unused)
return ret;
p = "FOO/0";
- ret = expr__parse(&val, &ctx, &p);
+ ret = expr__parse(&val, &ctx, p);
TEST_ASSERT_VAL("division by zero", ret == 1);
p = "BAR/";
- ret = expr__parse(&val, &ctx, &p);
+ ret = expr__parse(&val, &ctx, p);
TEST_ASSERT_VAL("missing operand", ret == 1);
TEST_ASSERT_VAL("find other",
diff --git a/tools/perf/util/expr.c b/tools/perf/util/expr.c
index b39fd39f10ec..45b25530db5b 100644
--- a/tools/perf/util/expr.c
+++ b/tools/perf/util/expr.c
@@ -52,9 +52,9 @@ __expr__parse(double *val, struct parse_ctx *ctx, const char *expr,
return ret;
}
-int expr__parse(double *final_val, struct parse_ctx *ctx, const char **pp)
+int expr__parse(double *final_val, struct parse_ctx *ctx, const char *expr)
{
- return __expr__parse(final_val, ctx, *pp, EXPR_PARSE);
+ return __expr__parse(final_val, ctx, expr, EXPR_PARSE);
}
static bool
@@ -71,14 +71,14 @@ already_seen(const char *val, const char *one, const char **other,
return false;
}
-int expr__find_other(const char *p, const char *one, const char ***other,
+int expr__find_other(const char *expr, const char *one, const char ***other,
int *num_other)
{
int err, i = 0, j = 0;
struct parse_ctx ctx;
expr__ctx_init(&ctx);
- err = __expr__parse(NULL, &ctx, p, EXPR_OTHER);
+ err = __expr__parse(NULL, &ctx, expr, EXPR_OTHER);
if (err)
return -1;
diff --git a/tools/perf/util/expr.h b/tools/perf/util/expr.h
index 01e1529bdfd9..28a0a2dfbd8c 100644
--- a/tools/perf/util/expr.h
+++ b/tools/perf/util/expr.h
@@ -17,8 +17,8 @@ struct parse_ctx {
void expr__ctx_init(struct parse_ctx *ctx);
void expr__add_id(struct parse_ctx *ctx, const char *id, double val);
-int expr__parse(double *final_val, struct parse_ctx *ctx, const char **pp);
-int expr__find_other(const char *p, const char *one, const char ***other,
+int expr__parse(double *final_val, struct parse_ctx *ctx, const char *expr);
+int expr__find_other(const char *expr, const char *one, const char ***other,
int *num_other);
#endif
diff --git a/tools/perf/util/stat-shadow.c b/tools/perf/util/stat-shadow.c
index 2c41d47f6f83..854d6abf2993 100644
--- a/tools/perf/util/stat-shadow.c
+++ b/tools/perf/util/stat-shadow.c
@@ -779,9 +779,7 @@ static void generic_metric(struct perf_stat_config *config,
}
if (!metric_events[i]) {
- const char *p = metric_expr;
-
- if (expr__parse(&ratio, &pctx, &p) == 0) {
+ if (expr__parse(&ratio, &pctx, metric_expr) == 0) {
char *unit;
char metric_bf[64];
--
2.24.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 5/5] perf expr: Make expr__parse return -1 on error
2020-02-24 8:29 [PATCHv2 0/5] perf expr: Add flex scanner Jiri Olsa
` (3 preceding siblings ...)
2020-02-24 8:29 ` [PATCH 4/5] perf expr: Straighten expr__parse/expr__find_other interface Jiri Olsa
@ 2020-02-24 8:29 ` Jiri Olsa
2020-02-27 12:10 ` [PATCHv2 0/5] perf expr: Add flex scanner Jiri Olsa
5 siblings, 0 replies; 10+ messages in thread
From: Jiri Olsa @ 2020-02-24 8:29 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: lkml, Ingo Molnar, Namhyung Kim, Alexander Shishkin,
Peter Zijlstra, Michael Petlan, Ravi Bangoria, Andi Kleen,
Kajol Jain, John Garry
To match the error value of the expr__find_other function,
so all exported expr functions return the same values:
0 on success, -1 on error.
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
tools/perf/tests/expr.c | 4 ++--
tools/perf/util/expr.c | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/tools/perf/tests/expr.c b/tools/perf/tests/expr.c
index 755d73c86c68..28313e59d6f6 100644
--- a/tools/perf/tests/expr.c
+++ b/tools/perf/tests/expr.c
@@ -45,11 +45,11 @@ int test__expr(struct test *t __maybe_unused, int subtest __maybe_unused)
p = "FOO/0";
ret = expr__parse(&val, &ctx, p);
- TEST_ASSERT_VAL("division by zero", ret == 1);
+ TEST_ASSERT_VAL("division by zero", ret == -1);
p = "BAR/";
ret = expr__parse(&val, &ctx, p);
- TEST_ASSERT_VAL("missing operand", ret == 1);
+ TEST_ASSERT_VAL("missing operand", ret == -1);
TEST_ASSERT_VAL("find other",
expr__find_other("FOO + BAR + BAZ + BOZO", "FOO", &other, &num_other) == 0);
diff --git a/tools/perf/util/expr.c b/tools/perf/util/expr.c
index 45b25530db5b..fd192ddf93c1 100644
--- a/tools/perf/util/expr.c
+++ b/tools/perf/util/expr.c
@@ -54,7 +54,7 @@ __expr__parse(double *val, struct parse_ctx *ctx, const char *expr,
int expr__parse(double *final_val, struct parse_ctx *ctx, const char *expr)
{
- return __expr__parse(final_val, ctx, expr, EXPR_PARSE);
+ return __expr__parse(final_val, ctx, expr, EXPR_PARSE) ? -1 : 0;
}
static bool
--
2.24.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCHv2 0/5] perf expr: Add flex scanner
2020-02-24 8:29 [PATCHv2 0/5] perf expr: Add flex scanner Jiri Olsa
` (4 preceding siblings ...)
2020-02-24 8:29 ` [PATCH 5/5] perf expr: Make expr__parse return -1 on error Jiri Olsa
@ 2020-02-27 12:10 ` Jiri Olsa
2020-02-27 13:46 ` Arnaldo Carvalho de Melo
5 siblings, 1 reply; 10+ messages in thread
From: Jiri Olsa @ 2020-02-27 12:10 UTC (permalink / raw)
To: Jiri Olsa, Arnaldo Carvalho de Melo
Cc: lkml, Ingo Molnar, Namhyung Kim, Alexander Shishkin,
Peter Zijlstra, Michael Petlan, Ravi Bangoria, Andi Kleen,
Kajol Jain, John Garry
On Mon, Feb 24, 2020 at 09:29:13AM +0100, Jiri Olsa wrote:
> hi,
> while preparing changes for user defined metric expressions
> I also moved the expression manual parser to flex.
>
> The reason is to have an easy and reasonable way to support
> and parse multiple user-defined metric expressions from
> command line or file.
>
> I was posponing the change, but I just saw another update to
> the expr manual scanner (from Kajol Jain), so cherry picked
> just the expr flex code changes to get it out.
>
> Kajol Jain,
> I think it should ease up your change for unknown values marked
> by '?'. Would you consider rebasing your changes on top of this?
>
>
kajoljain found and issue in this one, I'll send v3 as
soon as he confirms the fix
jirka
> v2 changes:
> - handle special chars properly
> - fix return value for expr__parse
>
> Available also in:
> git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
> perf/metric_flex
>
> thanks,
> jirka
>
>
> ---
> Jiri Olsa (5):
> perf expr: Add expr.c object
> perf expr: Move expr lexer to flex
> perf expr: Increase EXPR_MAX_OTHER
> perf expr: Straighten expr__parse/expr__find_other interface
> perf expr: Make expr__parse return -1 on error
>
> tools/perf/tests/expr.c | 10 +++---
> tools/perf/util/Build | 11 ++++++-
> tools/perf/util/expr.c | 112 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> tools/perf/util/expr.h | 8 ++---
> tools/perf/util/expr.l | 87 +++++++++++++++++++++++++++++++++++++++++++++++++++
> tools/perf/util/expr.y | 208 ++++++++++++++++++++++++++++++++-----------------------------------------------------------------------------------------
> tools/perf/util/stat-shadow.c | 4 +--
> 7 files changed, 272 insertions(+), 168 deletions(-)
> create mode 100644 tools/perf/util/expr.c
> create mode 100644 tools/perf/util/expr.l
>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCHv2 0/5] perf expr: Add flex scanner
2020-02-27 12:10 ` [PATCHv2 0/5] perf expr: Add flex scanner Jiri Olsa
@ 2020-02-27 13:46 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 10+ messages in thread
From: Arnaldo Carvalho de Melo @ 2020-02-27 13:46 UTC (permalink / raw)
To: Jiri Olsa
Cc: Jiri Olsa, lkml, Ingo Molnar, Namhyung Kim, Alexander Shishkin,
Peter Zijlstra, Michael Petlan, Ravi Bangoria, Andi Kleen,
Kajol Jain, John Garry
Em Thu, Feb 27, 2020 at 01:10:00PM +0100, Jiri Olsa escreveu:
> On Mon, Feb 24, 2020 at 09:29:13AM +0100, Jiri Olsa wrote:
> > hi,
> > while preparing changes for user defined metric expressions
> > I also moved the expression manual parser to flex.
> >
> > The reason is to have an easy and reasonable way to support
> > and parse multiple user-defined metric expressions from
> > command line or file.
> >
> > I was posponing the change, but I just saw another update to
> > the expr manual scanner (from Kajol Jain), so cherry picked
> > just the expr flex code changes to get it out.
> >
> > Kajol Jain,
> > I think it should ease up your change for unknown values marked
> > by '?'. Would you consider rebasing your changes on top of this?
> >
> >
>
> kajoljain found and issue in this one, I'll send v3 as
> soon as he confirms the fix
Ok, I'll hold off processing those patches then, thanks!
- Arnaldo
> jirka
>
>
> > v2 changes:
> > - handle special chars properly
> > - fix return value for expr__parse
> >
> > Available also in:
> > git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
> > perf/metric_flex
> >
> > thanks,
> > jirka
> >
> >
> > ---
> > Jiri Olsa (5):
> > perf expr: Add expr.c object
> > perf expr: Move expr lexer to flex
> > perf expr: Increase EXPR_MAX_OTHER
> > perf expr: Straighten expr__parse/expr__find_other interface
> > perf expr: Make expr__parse return -1 on error
> >
> > tools/perf/tests/expr.c | 10 +++---
> > tools/perf/util/Build | 11 ++++++-
> > tools/perf/util/expr.c | 112 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > tools/perf/util/expr.h | 8 ++---
> > tools/perf/util/expr.l | 87 +++++++++++++++++++++++++++++++++++++++++++++++++++
> > tools/perf/util/expr.y | 208 ++++++++++++++++++++++++++++++++-----------------------------------------------------------------------------------------
> > tools/perf/util/stat-shadow.c | 4 +--
> > 7 files changed, 272 insertions(+), 168 deletions(-)
> > create mode 100644 tools/perf/util/expr.c
> > create mode 100644 tools/perf/util/expr.l
> >
>
--
- Arnaldo
^ permalink raw reply [flat|nested] 10+ messages in thread