Hello,
Looking at the code in pfmlib_gen_powerpc.c again, I realized that during the
change to consolidate the Power arch chips code to use common structures, and
get rid of switch statements, I made a mistake with the group_vector and
event_count arrays.
I had made the incorrect assumption that the #defines for PFMLIB_POWER4_PMU (and
similar) had started at 0 and went to 7, but in reality, these numbers start at
110 and go to 117. The result is that arrays are created which have entries
0..109 unused, wasting space. The attached patch offsets the indexes by
FIRST_POWER_PMU which is set to the value of PFMLIB_PPC970_PMU (110).
Also, as per a recent discussion about the use of malloc() in the dispatch call,
I decided to just eliminate the call and use C99's ability to do dynamic
memory allocation on the stack. So this patch undoes the recent change that
checks the return value of malloc(), and the single exit point which was used to
call free(). Since free is no longer needed, I went back to using multiple exit
points, because I think the code is more clear that way.
Also fixed one place where the indentation inside a loop was wrong.
None of these changes are necessary. The code works as is, so this is just a
clean-up and optimization.
Tested on Power5.
Signed-off-by: Corey Ashford <cjash...@us.ibm.com>
Index: pfmlib_gen_powerpc.c
===================================================================
RCS file: /cvsroot/perfmon2/libpfm/lib/pfmlib_gen_powerpc.c,v
retrieving revision 1.15
diff -u -p -r1.15 pfmlib_gen_powerpc.c
--- pfmlib_gen_powerpc.c 7 Aug 2009 13:01:18 -0000 1.15
+++ pfmlib_gen_powerpc.c 7 Aug 2009 21:39:14 -0000
@@ -56,23 +56,25 @@
#include "power6_events.h"
#include "power7_events.h"
+#define FIRST_POWER_PMU PFMLIB_PPC970_PMU
+
static const int num_group_vec[] = {
- [PFMLIB_PPC970_PMU] = PPC970_NUM_GROUP_VEC,
- [PFMLIB_PPC970MP_PMU] = PPC970MP_NUM_GROUP_VEC,
- [PFMLIB_POWER4_PMU] = POWER4_NUM_GROUP_VEC,
- [PFMLIB_POWER5_PMU] = POWER5_NUM_GROUP_VEC,
- [PFMLIB_POWER5p_PMU] = POWER5p_NUM_GROUP_VEC,
- [PFMLIB_POWER6_PMU] = POWER6_NUM_GROUP_VEC,
- [PFMLIB_POWER7_PMU] = POWER7_NUM_GROUP_VEC
+ [PFMLIB_PPC970_PMU - FIRST_POWER_PMU] = PPC970_NUM_GROUP_VEC,
+ [PFMLIB_PPC970MP_PMU - FIRST_POWER_PMU] = PPC970MP_NUM_GROUP_VEC,
+ [PFMLIB_POWER4_PMU - FIRST_POWER_PMU] = POWER4_NUM_GROUP_VEC,
+ [PFMLIB_POWER5_PMU - FIRST_POWER_PMU] = POWER5_NUM_GROUP_VEC,
+ [PFMLIB_POWER5p_PMU - FIRST_POWER_PMU] = POWER5p_NUM_GROUP_VEC,
+ [PFMLIB_POWER6_PMU - FIRST_POWER_PMU] = POWER6_NUM_GROUP_VEC,
+ [PFMLIB_POWER7_PMU - FIRST_POWER_PMU] = POWER7_NUM_GROUP_VEC
};
static const int event_count[] = {
- [PFMLIB_PPC970_PMU] = PPC970_PME_EVENT_COUNT,
- [PFMLIB_PPC970MP_PMU] = PPC970MP_PME_EVENT_COUNT,
- [PFMLIB_POWER5_PMU] = POWER5_PME_EVENT_COUNT,
- [PFMLIB_POWER5p_PMU] = POWER5p_PME_EVENT_COUNT,
- [PFMLIB_POWER6_PMU] = POWER6_PME_EVENT_COUNT,
- [PFMLIB_POWER7_PMU] = POWER7_PME_EVENT_COUNT
+ [PFMLIB_PPC970_PMU - FIRST_POWER_PMU] = PPC970_PME_EVENT_COUNT,
+ [PFMLIB_PPC970MP_PMU - FIRST_POWER_PMU] = PPC970MP_PME_EVENT_COUNT,
+ [PFMLIB_POWER5_PMU - FIRST_POWER_PMU] = POWER5_PME_EVENT_COUNT,
+ [PFMLIB_POWER5p_PMU - FIRST_POWER_PMU] = POWER5p_PME_EVENT_COUNT,
+ [PFMLIB_POWER6_PMU - FIRST_POWER_PMU] = POWER6_PME_EVENT_COUNT,
+ [PFMLIB_POWER7_PMU - FIRST_POWER_PMU] = POWER7_PME_EVENT_COUNT
};
unsigned *pmd_priv_vec;
@@ -137,7 +139,7 @@ static int pfm_gen_powerpc_get_event_cod
unsigned int pmd,
int *code)
{
- if (event < event_count[gen_powerpc_support.pmu_type]) {
+ if (event < event_count[gen_powerpc_support.pmu_type -
FIRST_POWER_PMU]) {
*code = pe[event].pme_code;
return PFMLIB_SUCCESS;
} else
@@ -203,7 +205,7 @@ static void intersect_groups(unsigned lo
{
int i;
- for (i = 0; i < num_group_vec[gen_powerpc_support.pmu_type]; i++) {
+ for (i = 0; i < num_group_vec[gen_powerpc_support.pmu_type -
FIRST_POWER_PMU]; i++) {
result[i] &= operand[i];
}
}
@@ -212,7 +214,7 @@ static int first_group(unsigned long lon
{
int i, bit;
- for (i = 0; i < num_group_vec[gen_powerpc_support.pmu_type]; i++) {
+ for (i = 0; i < num_group_vec[gen_powerpc_support.pmu_type -
FIRST_POWER_PMU]; i++) {
bit = ffsll(group_vec[i]);
if (bit) {
return (bit - 1) + (i * 64);
@@ -370,16 +372,11 @@ static int pfm_gen_powerpc_dispatch_even
int i, j, group;
int counters_used = 0;
unsigned long long mmcr0_val, mmcr1_val;
- unsigned long long *group_vector;
- int result_code = PFMLIB_SUCCESS;
+ unsigned long long
group_vector[num_group_vec[gen_powerpc_support.pmu_type - FIRST_POWER_PMU]];
unsigned int plm;
plm = (input->pfp_events[0].plm != 0) ? input->pfp_events[0].plm :
input->pfp_dfl_plm;
- group_vector = malloc(sizeof(unsigned long long) *
num_group_vec[gen_powerpc_support.pmu_type]);
- if (!group_vector)
- return PFMLIB_ERR_NOMEM;
-
/*
* Verify that all of the privilege level masks are identical, as
* we cannot have mixed levels on POWER
@@ -388,25 +385,21 @@ static int pfm_gen_powerpc_dispatch_even
for (i = 1; i < input->pfp_event_count; i++) {
if (input->pfp_events[i].plm == 0) {
/* it's ok if the default is the same as plm */
- if (plm != input->pfp_dfl_plm) {
- result_code = PFMLIB_ERR_NOASSIGN;
- goto dispatch_exit;
- }
+ if (plm != input->pfp_dfl_plm)
+ return PFMLIB_ERR_NOASSIGN;
} else {
- if (plm != input->pfp_events[i].plm) {
- result_code = PFMLIB_ERR_NOASSIGN;
- goto dispatch_exit;
- }
+ if (plm != input->pfp_events[i].plm)
+ return PFMLIB_ERR_NOASSIGN;
}
}
/* start by setting all of the groups as available */
- memset(group_vector, 0xff, sizeof(unsigned long long) *
num_group_vec[gen_powerpc_support.pmu_type]);
+ memset(group_vector, 0xff, sizeof(unsigned long long) *
num_group_vec[gen_powerpc_support.pmu_type - FIRST_POWER_PMU]);
for (i = 0; i < input->pfp_event_count; i++) {
- mmcr0_val |= mmcr0_counter_off_val[i];
+ mmcr0_val |= mmcr0_counter_off_val[i];
intersect_groups(group_vector,
get_group_vector(input->pfp_events[i].event));
- mmcr1_val |= mmcr1_counter_off_val[i];
+ mmcr1_val |= mmcr1_counter_off_val[i];
}
group = first_group(group_vector);
while (group != -1) {
@@ -453,11 +446,10 @@ static int pfm_gen_powerpc_dispatch_even
break;
try_next_group: ;
}
- if (group == -1) {
+ if (group == -1)
/* We did not find a group that meets the constraints */
- result_code = PFMLIB_ERR_NOASSIGN;
- goto dispatch_exit;
- }
+ return PFMLIB_ERR_NOASSIGN;
+
/* We now have a group that meets the constraints */
mmcr0_val = get_mmcr0(group);
@@ -528,9 +520,7 @@ try_next_group: ;
/* We always use the same number of control regs */
output->pfp_pmc_count = get_num_control_regs();
-dispatch_exit:
- free(group_vector);
- return result_code;
+ return PFMLIB_SUCCESS;
}
------------------------------------------------------------------------------
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