Re: [4/5] powerpc/perf: Change name & type of 'pred' in power_pmu_bhrb_read

[Anshuman Khandual <khandual@linux.vnet.ibm.com>]

On 07/29/2015 08:55 AM, Michael Ellerman wrote:
> On Tue, 2015-30-06 at 08:20:30 UTC, Anshuman Khandual wrote:
>> > Branch record attributes 'mispred' and 'predicted' are single bit
>> > fields as defined in the perf ABI. Hence the data type of the field
>> > 'pred' used during BHRB processing should be changed from integer
>> > to bool. This patch also changes the name of the variable from 'pred'
>> > to 'mispred' making the logical inversion process more meaningful
>> > and readable.
> This whole function is a mess.
> 
> There's no good reason why we're doing the assignment to pred/mispred in two
> places to begin with, so if that was eliminated we wouldn't need a local for
> mispred to begin with.

Not sure whether I got this right. We are assigning mispred once with
the value (val & BHRB_PREDICTION) and then assigning mispred and it's
inversion to two different fields of the branch entry as required.

> 
> Then there's the type juggling, all of which probably works but is fishy and
> horrible.

With this patch and one more (2nd patch of the BHRB SW filter series)
patch, we are trying to make it better.

> 
> You take a u64, bitwise and it with a mask, assign that to a boolean, then take

So that any residual positive value after the "AND" operation will
become logical TRUE for the boolean. We dont use any shifting here
as BHRB_PREDICTION checks for the right most (least significant) bit
in the sequence.

> the boolean, *bitwise* negate that and assign the result to a single bit
> bitfield.

This is getting fixed with a subsequent patch (2nd patch of the BHRB
SW filter series) in a new function called insert_branch.

+static inline void insert_branch(struct cpu_hw_events *cpuhw,
+                       int index, u64 from, u64 to, bool mispred)
+{
+       cpuhw->bhrb_entries[index].from = from;
+       cpuhw->bhrb_entries[index].to = to;
+       cpuhw->bhrb_entries[index].mispred = mispred;
+       cpuhw->bhrb_entries[index].predicted = !mispred;
+}