Thanks Kevin. No problem for the delay. I'll update the stats with this new patch and then push it. Hopefully that will be in the next couple of days but possibly not til later on.
Cheers Tim On Tue, 15 Dec 2009 00:44:45 -0000, Kevin Lim <[email protected]> wrote: > 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 > -- 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
