Corey,

On Wed, Nov 25, 2009 at 8:56 PM, Corey Ashford
<cjash...@linux.vnet.ibm.com> wrote:
>> Ok, I see. The issue here is that you are assuming that attribute values
>> are all integers. The way it is setup right now, an attribute can have any
>> type, incl. a string.
>
> In my original email, I had a parenthetical question/comment about
> attributes which have string values.  In the libpfm code, though, the
> attribute type appears only to have the choices of umask, boolean, or
> integer:
>
> typedef enum {
>        PFM_ATTR_NONE=0,        /* no attribute */
>        PFM_ATTR_UMASK,         /* unit mask */
>        PFM_ATTR_MOD_BOOL,      /* register modifier */
>        PFM_ATTR_MOD_INTEGER,   /* register modifier */
>
>        PFM_ATTR_MAX            /* end-marker */
> } pfm_attr_t;
>
> So I am sure I am missing something somewhere.
>
This could be extended with PFM_ATTR_MOD_STRING for instance.

> That said, we could also support strings by returning, in the array, a
> pointer to the string instead of a boolean/integer value.  In this case, it
> would be convenient to return an array of pfm_attr_t values as well, so that
> it would be easier for the tool to display the values.
>
Yes. But this is getting complicated.

>>
>> If I look at the X86 event tables, some events or unit masks do have
>> certain modifiers hardcoded. For instance,
>>
>>    UOPS_ISSUED:STALLED_CYCLES has i=1, cmask=1
>>
>> This unit mask does not exist in the official documentation, but it is
>> added for *convenience* because this is a common usage of the event.
>>
>> With existing libpfm4, if you try to pass
>>
>>   UOPS_ISSUED:STALLED_CYCLES:cmask=2
>>
>> It will fail. It is not clear to me what knowing that cmask=1 would do
>> in your tool. I understand if you are listing events. The tool could
>> obviously check that the user is not passing an already hardcoded
>> attributes, but then the library does that work already.
>
> In this case you are right, it wouldn't provide anything useful, and these
> defaults might be excluded from the return values, since they are not
> modifiable.
>
Ok, so you are looking at this to set some default modifiable attributes
such as a default unit mask. For instance, you'd like to have it such that
if I were to pass only UOPS_ISSUED, the library would not reject but
instead it would use a default unit mask. Note that it does that already, but
without telling you explicitly which one it picked, i.e., there is no API
to retrieve that.

>> It seems that it you pull this knowledge out of the library you will end
>> up duplicating some of the work of the library in your tool.
>
> Of course the user can choose to ignore what the specific defaults are, in
> which case nothing changes in the usage model.
>
> The issue I'm facing is that we have events for which you can choose one
> attribute value or another, and it's not always clear which value the user
> would want to use.  And this can occur on more than one attribute for an
> event.  I want the user to be able to determine what value the library has
> chosen for the default, rather than just shield him from the choices it will
> make.
>

That seems contradictory to me. You are saying it is not clear which one
the user would want to use and then you say the library would pick a default.
How could the library make a better choice than the user? If an event has two
distinct attributes to choose from and they don't do the same thing, then it is
up to the user to choose one.

It seems to me you have to systematically do an a priori check for each
event you want to measure:

       idx = pfm_find_event("UOPS_ISSUED");
       pfm_get_event_dfl_attrs(idx, -1, &info, &n);

And then what?
You reconstruct the event string with all the defaults.
Assuming the default is a unit mask, STALLED_CYCLES.
Then info would contain:
     info->name = "STALLED_CYCLES";
     info->type   = PFM_ATTR_UMASK;

You would have to scan the event string to see if there is a unit
mask. If there is, then you ignore the default. Otherwise you would
append the default unit mask to your string and eventually call
pfm_get_perf_event_encoding().

I am wondering what you do when the number of defaults returned
by the call > 1. I would think you could only get one UMASK attr
but possibly multiple  modifiers PFM_ATTR_MOD_*. I would assume
those are all compatible and are meant to be used together, i.e., that
is what the library would do internally.


> I think this may be important when the user looks at his results and
> scratches his head...  "Why am I getting this count that is obviously not
> what I wanted to count?  Is the tool counting cycles asserted instead of
> edges?" etc.  Then he has to go find the attributes, and try forcing them to
> what he thinks they should be to see if it makes a difference.
>
> One other issue is the notion of mandatory attributes.  For some events, we
> are going to need the user to specify a couple of attributes which supply a
> bit mask.  These masks would have no default value.  It would be nice for
> the tool to have some way of sensing required attributes.
>
There is already that kind of problems on X86 which some events which do
requires some unit masks modifiers. For instance, the cache events needs
MESI modifiers. The library currently checks if some MESI bits are specified
by user, if not it sets them all, i.e., any cache line state. That seems like a
reasonable default.

> So maybe the interface could be generalized a bit more so that instead of
> returning separate arrays, we return a data structure with all of the
> interesting details about the attribute:
>
> struct pfm_attr_info_t {
>    char *name;

Why not just return the attr index, and then the user can use
pfm_get_event_attr(), pfm_get_event_attr_desc().

>    char *desc;

May be overkill.

>    pfm_attr_t type;

>    int has_default; /* boolean */

I don't get has_default?

>    union {
>        const char *dfl_str;
>        int dfl_bool;
>        int dfl_int;
>    };
>    int mandatory; /* 1 = mandatory, 0 = optional */
> }
>
> It would be nice to supply information about the allowed values of the
> attribute, but I don't want to get too carried away here.

That would be complicated. This could be part of the description to guide the
user. There are already some sanity checks on boolean.
>
> So this would be the proposed api call:
>
> pfm_get_event_attr_info(int idx, int attr_idx, pfm_attr_info_t *attr_infos,
> int *n_attrs);
>
> I agree, that's certainly one easy-to-use solution to the plm problem.  I
> was just thinking if we had a more general-purpose mechanism, that could be
> used for all attributes instead of just plm, that we could kill two birds
> with one stone.
>
understood.

------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day 
trial. Simplify your report design, integration and deployment - and focus on 
what you do best, core application coding. Discover what's new with
Crystal Reports now.  http://p.sf.net/sfu/bobj-july
_______________________________________________
perfmon2-devel mailing list
perfmon2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/perfmon2-devel

Reply via email to