[V2 2/4] tools/perf: Refactor the code definition of perf reg extended mask in tools side header file

[Athira Rajeev]

> On 01-Oct-2021, at 11:50 AM, Daniel Axtens <dja at axtens.net> wrote:
> 
> Hi Athira,
> 
>> PERF_REG_PMU_MASK_300 and PERF_REG_PMU_MASK_31 defines the mask
>> value for extended registers. Current definition of these mask values
>> uses hex constant and does not use registers by name, making it less
>> readable. Patch refactor the macro values in perf tools side header file
>> by or'ing together the actual register value constants.
>> 
> This looks like a good simplification.
> 
>> -/* Exclude MMCR3, SIER2, SIER3 for CPU_FTR_ARCH_300 */
>> -#define	PERF_EXCLUDE_REG_EXT_300	(7ULL << PERF_REG_POWERPC_MMCR3)
> 
> This file is uAPI - are we allowed to remove a define? Could a program
> built against these headers now fail to compile because we've removed it?

Hi Daniel,

Thanks for the review.
My bad, you are right. I will correct this in version3 by retaining this define and refactoring the macro.

> 
>> -
>> /*
>>  * PERF_REG_EXTENDED_MASK value for CPU_FTR_ARCH_300
>>  * includes 9 SPRS from MMCR0 to PMC6 excluding the
>> - * unsupported SPRS in PERF_EXCLUDE_REG_EXT_300.
>> + * unsupported SPRS MMCR3, SIER2 and SIER3.
>>  */
>> -#define PERF_REG_PMU_MASK_300   ((0xfffULL << PERF_REG_POWERPC_MMCR0) - PERF_EXCLUDE_REG_EXT_300)
>> +#define PERF_REG_PMU_MASK_300	\
>> +	((1ul << PERF_REG_POWERPC_MMCR0) | (1ul << PERF_REG_POWERPC_MMCR1) | \
>> +	(1ul << PERF_REG_POWERPC_MMCR2) | (1ul << PERF_REG_POWERPC_PMC1) | \
>> +	(1ul << PERF_REG_POWERPC_PMC2) | (1ul << PERF_REG_POWERPC_PMC3) | \
>> +	(1ul << PERF_REG_POWERPC_PMC4) | (1ul << PERF_REG_POWERPC_PMC5) | \
>> +	(1ul << PERF_REG_POWERPC_PMC6))
>> 
>> /*
>>  * PERF_REG_EXTENDED_MASK value for CPU_FTR_ARCH_31
>>  * includes 12 SPRs from MMCR0 to PMC6.
>>  */
>> -#define PERF_REG_PMU_MASK_31   (0xfffULL << PERF_REG_POWERPC_MMCR0)
>> +#define PERF_REG_PMU_MASK_31	\
>> +	(PERF_REG_PMU_MASK_300 | (1ul << PERF_REG_POWERPC_MMCR3) | \
>> +	(1ul << PERF_REG_POWERPC_SIER2) | (1ul << PERF_REG_POWERPC_SIER3))
>> 
>> -#define PERF_REG_EXTENDED_MAX  (PERF_REG_POWERPC_PMC6 + 1)
> 
> Likewise for this define?
> 
> I think this might also be an issue for some of your other patches which
> change an include/uapi/ file.

Though I am removing PERF_REG_EXTENDED_MAX define from end of this uapi file, it is moved along with enum definition of perf_event_powerpc_regs.
So we should be good with moving this define from this place.

Thanks
Athira 
> 
> Kind regards,
> Daniel
> 
>> -- 
>> 2.30.1 (Apple Git-130)