BPred: Fix RAS to handle predicated call/return instructions.

Information
Submitter:	Ali Saidi
Repository:	gem5
Branch:
Bugs:
Depends On:
Reviewers
Groups:	Default
People:

Description

BPred: Fix RAS to handle predicated call/return instructions.

Change RAS to fix issues with predicated call/return instructions.
Handled all cases in the life of a predicated call and return instruction.

Testing Done

Works for all regressions tests, however dependent branch predictor patch exposes x86 issues

Without wanting to dive into the branch predictor code I'll assume this is functionally correct, especially if you've run regressions. There are some style nits which I've pointed out. I'm also assuming *this* change doesn't break X86_FS on O3, and that this can be applied separately. I'm sure you're fixing a legitimate problem, but for my information could you please explain what the problem a little more specifically? There's no need to go into the gory details, I'd just like to be able to tell in the future if I'm running into whatever this is fixing. The branch predictor was just not handling returns from calls correctly?

Ali Saidi Dec. 13, 2011, 11:41 p.m. (Dec. 13, 2011, 11:41 p.m.)

This change requires the previous BP change and can't be supplied separately, but I'm just adding it to review board so I could point someone towards it.  The previous implementation assumed that if you executed a call or return that it must be taken (they're not conditional in any way on Alpha). However, with ARM (and perhaps others) the return and call instructions are conditionally executed, so you could predict the call/return incorrectly and then mess up the order on the RAS. This addresses those cases. 

Separately, it seems like these branch predictor changes and Nilay's changes to get o3 working with ruby are all dependent on fixing x86.

Nilay Vaish Dec. 14, 2011, 12:01 a.m. (Dec. 14, 2011, 12:01 a.m.)

What needs to be fixed in x86?

I saw an error with a non-speculative instruction getting squashed even
though it had no fault associated with it. Adding memory fences to I/O
instructions does away with that.

Ali Saidi Dec. 14, 2011, 1:33 a.m. (Dec. 14, 2011, 1:33 a.m.)

Didn't you run into a problem where your change caused a segfault in user code right after linux boot?

Gabe Black Dec. 14, 2011, 3:48 a.m. (Dec. 14, 2011, 3:48 a.m.)

Yes, I'm pretty sure he did. It's not really x86 that needs fixed it's O3, but it is getting in the way a lot.

Nilay Vaish Dec. 14, 2011, 4:23 a.m. (Dec. 14, 2011, 4:23 a.m.)

I am trying to zero in on the instruction when the segmentation
fault happens.

src/cpu/o3/bpred_unit_impl.hh (Diff revision 1)

should be on the same line as the }

src/cpu/o3/bpred_unit_impl.hh (Diff revision 1)

change to hist_it->usedRAS?

src/cpu/o3/bpred_unit_impl.hh (Diff revision 1)

space after ",", here and elsewhere

src/cpu/o3/bpred_unit_impl.hh (Diff revision 1)

ditto, and below

src/cpu/o3/bpred_unit_impl.hh (Diff revision 1)

Does this actually exceed 80 columns if it's on one line?

src/cpu/pred/tournament.cc (Diff revision 1)

Too few spaces in the indent, the {}s aren't really needed (although I'm not too hung up on that), and you could use a ?: if you wanted.

src/cpu/pred/tournament.cc (Diff revision 1)

The spacing is off here too. Maybe a reviewboard thing?

Don't see any obvious problems (i.e., it's not obvious there are no problems...) other than the style nits.  Need to go through and make sure there's a space after commas in arg lists, particularly after the first arg of DPRINTF.

src/cpu/o3/bpred_unit_impl.hh (Diff revision 1)

Is there an extra blank line here?

You have a pending review.

Review Board 2.0.15

This change has been marked as submitted.

Screenshots

Files

Status: Closed (submitted)