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

Reply via email to