Re: [gem5-dev] Review Request 3727: cpu: disallow speculative update of the conditional branch predictor tables (o3)
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3727/#review9150 --- Ship it! Ship It! - Nathanael Premillieu On Nov. 18, 2016, 3:14 p.m., Arthur Perais wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3727/ > --- > > (Updated Nov. 18, 2016, 3:14 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > --- > > Changeset 11707:1d085f66c4ca > --- > > cpu: disallow speculative update of the conditional branch predictor tables > (o3) > > The Minor and o3 cpu models share the branch prediction > code. Minor relies on the BPredUnit::squash() function > to update the branch predictor tables on a branch mispre- > diction. This is fine because Minor executes in-order, so > the update is on the correct path. However, this causes the > branch predictor to be updated on out-of-order branch > mispredictions when using the o3 model, which should not > be the case. > > This patch guards against speculative update of the branch > prediction tables. On a branch misprediction, BPredUnit::squash() > calls BpredUnit::update(..., squashed = true). The underlying > branch predictor tests against the value of squashed. If it is > true, it restores any speculatively updated internal state > it might have (e.g., global/local branch history), then returns. > If false, it updates its prediction tables. Previously, exist- > ing predictors did not test against the "squashed" parameter. > > To accomodate for this change, the Minor model must now call > BPredUnit::squash() then BPredUnit::update(..., squashed = false) > on branch mispredictions. Before, calling BpredUnit::squash() > performed the prediction tables update. > > The effect is a slight MPKI improvement when using the o3 > model. A further patch should perform the same modifications > for the indirect target predictor and BTB (less critical). > > > Diffs > - > > src/cpu/minor/fetch2.cc 1d085f66c4ca > src/cpu/pred/2bit_local.hh 1d085f66c4ca > src/cpu/pred/2bit_local.cc 1d085f66c4ca > src/cpu/pred/bi_mode.hh 1d085f66c4ca > src/cpu/pred/bi_mode.cc 1d085f66c4ca > src/cpu/pred/bpred_unit.hh 1d085f66c4ca > src/cpu/pred/bpred_unit.cc 1d085f66c4ca > src/cpu/pred/tournament.hh 1d085f66c4ca > src/cpu/pred/tournament.cc 1d085f66c4ca > > Diff: http://reviews.gem5.org/r/3727/diff/ > > > Testing > --- > > Fast regressions (SE) > booting Linux (FS) > > > Thanks, > > Arthur Perais > > ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request 3727: cpu: disallow speculative update of the conditional branch predictor tables (o3)
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3727/#review9130 --- Ship it! Seems reasonable to me, though I'm no branch predictor expert. - Jason Lowe-Power On Nov. 18, 2016, 3:14 p.m., Arthur Perais wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3727/ > --- > > (Updated Nov. 18, 2016, 3:14 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > --- > > Changeset 11707:1d085f66c4ca > --- > > cpu: disallow speculative update of the conditional branch predictor tables > (o3) > > The Minor and o3 cpu models share the branch prediction > code. Minor relies on the BPredUnit::squash() function > to update the branch predictor tables on a branch mispre- > diction. This is fine because Minor executes in-order, so > the update is on the correct path. However, this causes the > branch predictor to be updated on out-of-order branch > mispredictions when using the o3 model, which should not > be the case. > > This patch guards against speculative update of the branch > prediction tables. On a branch misprediction, BPredUnit::squash() > calls BpredUnit::update(..., squashed = true). The underlying > branch predictor tests against the value of squashed. If it is > true, it restores any speculatively updated internal state > it might have (e.g., global/local branch history), then returns. > If false, it updates its prediction tables. Previously, exist- > ing predictors did not test against the "squashed" parameter. > > To accomodate for this change, the Minor model must now call > BPredUnit::squash() then BPredUnit::update(..., squashed = false) > on branch mispredictions. Before, calling BpredUnit::squash() > performed the prediction tables update. > > The effect is a slight MPKI improvement when using the o3 > model. A further patch should perform the same modifications > for the indirect target predictor and BTB (less critical). > > > Diffs > - > > src/cpu/minor/fetch2.cc 1d085f66c4ca > src/cpu/pred/2bit_local.hh 1d085f66c4ca > src/cpu/pred/2bit_local.cc 1d085f66c4ca > src/cpu/pred/bi_mode.hh 1d085f66c4ca > src/cpu/pred/bi_mode.cc 1d085f66c4ca > src/cpu/pred/bpred_unit.hh 1d085f66c4ca > src/cpu/pred/bpred_unit.cc 1d085f66c4ca > src/cpu/pred/tournament.hh 1d085f66c4ca > src/cpu/pred/tournament.cc 1d085f66c4ca > > Diff: http://reviews.gem5.org/r/3727/diff/ > > > Testing > --- > > Fast regressions (SE) > booting Linux (FS) > > > Thanks, > > Arthur Perais > > ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
[gem5-dev] Review Request 3727: cpu: disallow speculative update of the conditional branch predictor tables (o3)
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3727/ --- Review request for Default. Repository: gem5 Description --- Changeset 11707:1d085f66c4ca --- cpu: disallow speculative update of the conditional branch predictor tables (o3) The Minor and o3 cpu models share the branch prediction code. Minor relies on the BPredUnit::squash() function to update the branch predictor tables on a branch mispre- diction. This is fine because Minor executes in-order, so the update is on the correct path. However, this causes the branch predictor to be updated on out-of-order branch mispredictions when using the o3 model, which should not be the case. This patch guards against speculative update of the branch prediction tables. On a branch misprediction, BPredUnit::squash() calls BpredUnit::update(..., squashed = true). The underlying branch predictor tests against the value of squashed. If it is true, it restores any speculatively updated internal state it might have (e.g., global/local branch history), then returns. If false, it updates its prediction tables. Previously, exist- ing predictors did not test against the "squashed" parameter. To accomodate for this change, the Minor model must now call BPredUnit::squash() then BPredUnit::update(..., squashed = false) on branch mispredictions. Before, calling BpredUnit::squash() performed the prediction tables update. The effect is a slight MPKI improvement when using the o3 model. A further patch should perform the same modifications for the indirect target predictor and BTB (less critical). Diffs - src/cpu/minor/fetch2.cc 1d085f66c4ca src/cpu/pred/2bit_local.hh 1d085f66c4ca src/cpu/pred/2bit_local.cc 1d085f66c4ca src/cpu/pred/bi_mode.hh 1d085f66c4ca src/cpu/pred/bi_mode.cc 1d085f66c4ca src/cpu/pred/bpred_unit.hh 1d085f66c4ca src/cpu/pred/bpred_unit.cc 1d085f66c4ca src/cpu/pred/tournament.hh 1d085f66c4ca src/cpu/pred/tournament.cc 1d085f66c4ca Diff: http://reviews.gem5.org/r/3727/diff/ Testing --- Fast regressions (SE) booting Linux (FS) Thanks, Arthur Perais ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
