On Wed, May 13, 2020 at 12:04:55AM -0700, Ian Rogers wrote: SNIP
> > +METRICS FILE > > +------------ > > +The file with metrics has following syntax: > > + > > + NAME = EXPRESSION ; > > + NAME = EXPRESSION ; > > + ... > > + > > +where NAME is unique identifier of the metric, which is later used in > > +perf stat as -M option argument (see below). > > + > > +The EXPRESSION is the metric's formula with following grammar: > > + > > + EXPR: EVENT > > + EXPR: EXPR if EXPR else EXPR > > Not introduced by this patch, but this patch is exposing it as an API. yea, I was thinking about this and I think we will put a disclaimer in here that this is not an API and the interface can change.. it's really mostly intended to help out with running a custom metric which is not compiled in ... I don't want to be commited to support old API > This notion of if-else is really weird. For one thing there are no > comparison operators. The unit test doesn't really help: > ret |= test(&ctx, "1+1 if 3*4 else 0", 2); > What kind of comparison is "3*4"? If 0.0 causes the else clause then will > -0.0? > A typical expression I see written in C is to give a ratio such: > value = denom == 0 ? 0 : nom / denom; > I've worked around encoding this by extending expr.y locally. AFAICS it's used only with #SMT_on in the condition, aybe we could limit the condition only for #SMT_on term? > > > + EXPR: NUMBER > > + EXPR: EXPR | EXPR > > + EXPR: EXPR & EXPR > > + EXPR: EXPR ^ EXPR > > Again, it's odd that these cast the double to a long and then assign > the result back to a double. is this even used anywhere? perhaps it was added just to be complete SNIP > > + 2.002460174 0.86 23.37 > > 0.86 > > + 3.003969795 1.03 23.93 > > 1.03 > > + ... > > A feature request would be to allow metrics in terms of other metrics, > not just events :-) For example, it is common to sum all cache > hit/miss events. It is laborious to copy that expression for hit rate, > miss rate, etc. > > Perhaps the expression parsing code should be folded into the event > parsing code. nice idea, but let's finish straighten up what we have first ;-) I'll try to go through all the fixes/tests you posted and let's get it in first thanks, jirka