Hi Tim, Sorry for the slow reply, it's a very busy time for me. You're right that the predict_record should be updated on the BTB not supplying the target address. The patch you attached should fix the bug.
Kevin Quoting Steve Reinhardt <[email protected]>: > 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 > > > _______________________________________________ m5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/m5-dev
