Re: [PATCH] arm64: perf: Simplify the ARMv8 PMUv3 event attributes

[Shaokun Zhang <zhangshaokun@hisilicon.com>]

Hi Robin,

Sorry for the noise, it works on both GCC 5.4 and 7.3 using Will's method.

Thanks,
Shaokun

On 2019/11/4 10:02, Shaokun Zhang wrote:
> Hi Robin,
> 
> On 2019/11/1 19:11, Robin Murphy wrote:
>> On 2019-11-01 10:55 am, Will Deacon wrote:
>>> On Fri, Nov 01, 2019 at 10:54:21AM +0000, Robin Murphy wrote:
>>>> On 2019-11-01 10:36 am, Will Deacon wrote:
>>>>> On Fri, Nov 01, 2019 at 08:53:19AM +0000, Mark Rutland wrote:
>>>>>> On Thu, Oct 31, 2019 at 04:08:04PM +0000, Will Deacon wrote:
>>>>>>> On Wed, Oct 30, 2019 at 11:46:17AM +0800, Shaokun Zhang wrote:
>>>>>>>> For each PMU event, there is a ARMV8_EVENT_ATTR(xx, XX) and
>>>>>>>> &armv8_event_attr_xx.attr.attr. Let's redefine the ARMV8_EVENT_ATTR
>>>>>>>> to simplify the armv8_pmuv3_event_attrs.
>>>>>>>>
>>>>>>>> Cc: Will Deacon <will@kernel.org>
>>>>>>>> Cc: Mark Rutland <mark.rutland@arm.com>
>>>>>>>> Signed-off-by: Shaokun Zhang <zhangshaokun@hisilicon.com>
>>>>>>>> ---
>>>>>>>>    arch/arm64/kernel/perf_event.c | 189 ++++++++++++++---------------------------
>>>>>>>>    1 file changed, 65 insertions(+), 124 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
>>>>>>>> index a0b4f1bca491..d0f084939bcf 100644
>>>>>>>> --- a/arch/arm64/kernel/perf_event.c
>>>>>>>> +++ b/arch/arm64/kernel/perf_event.c
>>>>>>>> @@ -159,132 +159,73 @@ armv8pmu_events_sysfs_show(struct device *dev,
>>>>>>>>    }
>>>>>>>
>>>>>>> [...]
>>>>>>>
>>>>>>>> +    (&((struct perf_pmu_events_attr[]) { \
>>>>>>>> +        { .attr = __ATTR(name, 0444, armv8pmu_events_sysfs_show, NULL), \
>>>>>>>> +          .id = config, } \
>>>>>>>> +    })[0].attr.attr)
>>>>>>>
>>>>>>> I don't get the need for the array here. Why can't you do:
>>>>>>>
>>>>>>>     (&((struct perf_pmu_events_attr) {
>>>>>>>         .attr = ...,
>>>>>>>         .id = ...,
>>>>>>>     }).attr.attr)
>>>>>>
>>>>>> You need want &(obj.attr.attr) rather than &(obj).attr.attr, i.e.
>>>>>>
>>>>>> #define ARMV8_EVENT_ATTR(name, config) \
>>>>>>     (&((struct perf_pmu_events_attr) { \
>>>>>>         .attr = __ATTR(name, 0444, armv8pmu_events_sysfs_show, NULL), \
>>>>>>         .id = config, \
>>>>>>     }.attr.attr))
>>>>>> ... which compiles for me.
>>>>>
>>>>> Weird, the following compiles fine for me with both GCC and clang:
>>>>>
>>>>> #define ARMV8_EVENT_ATTR(name, config)                        \
>>>>>     (&((struct perf_pmu_events_attr) {                    \
>>>>>         .attr = __ATTR(name, 0444, armv8pmu_events_sysfs_show, NULL),    \
>>>>>         .id = config,                            \
>>>>>     }).attr.attr)
>>>>
>>>> You know that the expressions are equivalent because unary "&" has lower
>>>> precedence than ".", right? ;)
>>>
>>> Right, which is why it's weird that Shaokun claims that the version I posted
>>> doesn't compile. I assume it didn't build for Mark either, hence his extra
>>> brackets.
>>
>> Because different compilers have different ideas of whether "obj" is a valid thing to dereference at all, regardless of where you put parentheses. From what I remember, the array trick was the only way to convince older GCCs to treat the floating struct initialiser as an actual object definition. I guess newer versions are a bit more lenient.
>>
> 
> Thanks for your detailed explanations, sounds great! Both GCC 5.4 and 7.3 are
> unhappy without the array trick.
> 
> Thanks,
> Shaokun
> 
>> Robin.
>>
>> .
>>


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel