Your logic looks fine to me... I think Kevin would have to speak up to say if there's any intended behavior here or if this is truly a bug.
Steve On Tue, Dec 1, 2009 at 7:33 AM, Timothy M Jones <[email protected]> wrote: > Hi everyone, > > I've noticed a slight problem in using the branch predictor that's causing > it to update its state with the incorrect outcome of the branch. I'm > looking at O3CPU here. The problem comes in bpred_unit_impl.hh in > predict(). If the BTB doesn't supply a target address, then pred_taken is > set to 'false'. This is done without updating the predict_record which > could have the branch down as predicted taken. If the branch then isn't > taken, the predictor is updated but with the possibly incorrect value from > the predict_record. > > I've attached a couple of patches to show what I mean. The first, > 'test-bpred.patch' adds in a new trace flag 'TestBPred' simply to isolate > the problem and show it up. The second, 'btb-invalid-fix-prediction.patch' > contains the fix. For example, using > build/ALPHA_SE/tests/opt/quick/00.hello/alpha/linux/o3-timing I get the > following output first: > > info: Entering event queue @ 0. Starting simulation... > 49000: system.cpu.BPredUnit: First predicted 0x120000170 taken, now not > taken > target 0x120000170 > 52500: system.cpu.commit: Committed 0x120000170 > 53000: system.cpu.BPredUnit: Updating bpred for insn 0x120000170 as taken > 53000: system.cpu.commit: Committed 0x120000174 > 53500: system.cpu.commit: Committed 0x120000178 > .... > > As you can see, 0x120000170 obviously wasn't taken but the predictor is > updated as though it was because the initial prediction was changed when the > BTB couldn't supply a target address. After applying the fix we get this: > > info: Entering event queue @ 0. Starting simulation... > 49000: system.cpu.BPredUnit: First predicted 0x120000170 taken, now not > taken > target 0x120000170 > 52500: system.cpu.commit: Committed 0x120000170 > 53000: system.cpu.BPredUnit: Updating bpred for insn 0x120000170 as not > taken > 53000: system.cpu.commit: Committed 0x120000174 > 53500: system.cpu.commit: Committed 0x120000178 > .... > > Now the predictor is correctly updated with the knowledge that the branch > wasn't taken. > > Is my assessment of this right? > > Cheers > Tim > -- > The University of Edinburgh is a charitable body, registered in > Scotland, with registration number SC005336. > > > _______________________________________________ > m5-dev mailing list > [email protected] > http://m5sim.org/mailman/listinfo/m5-dev > > _______________________________________________ m5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/m5-dev
