[PATCH V8 03/10] powerpc, perf: Re organize BHRB processing

[Daniel Axtens]

> +void update_branch_entry(struct cpu_hw_events *cpuhw,
> +			int index, u64 from, u64 to, int pred)
> +{
> +	cpuhw->bhrb_entries[index].from = from;
> +	cpuhw->bhrb_entries[index].to = to;
> +	cpuhw->bhrb_entries[index].mispred = pred;
> +	cpuhw->bhrb_entries[index].predicted = ~pred;
> +}

I realise you're copying existing code, but:
 - could you please rename pred? If we assign .mispred to pred
and .predicted to ~pred, we should pick a different name for pred.
 - I'm really uncomfortable with the bitwise inverting a signed integer.
Can you explain what is going on here? Looking at
include/uapi/linux/perf_event.h, this seems to be a single bit flag:
shouldn't this then be a logical flip rather than a bitwise one?
(Furthermore, looking at that header, why is pred an int at all? Why not
a bool?)

> +
>  /* Processing BHRB entries */
>  static void power_pmu_bhrb_read(struct cpu_hw_events *cpuhw)
>  {
> -	u64 val;
> -	u64 addr;
> +	u64 val, addr, tmp;
Please don't use 'tmp' here. As far as I can tell, you use this variable
to compute the 'to' address. The name should reflect that.

Regards,
Daniel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 860 bytes
Desc: This is a digitally signed message part
URL: <http://lists.ozlabs.org/pipermail/linuxppc-dev/attachments/20150610/416e8c5c/attachment.sig>