Does this snippet not do the same thing as the commented code below?

Also, I'm wondering how this code worked before now, because this is the function called whenever a branch is resolved but formerly it only checked the most recent prediction.

Shouldn't we also just panic here since if this condition is true, the the simulation is pretty much broke right?

The commented out code didn't work for some reason, although I agree it probably should have.

As for how it worked before, you tell me.. you committed it. Somehow we got incredibly lucky and in between the instruction getting to decode, no other branches were fetched and put in the pred_hist. 

Yea, we could panic here instead of asserting.

The ternary operator here seems redundant since rounding down the ARM pc would be a nop. Beyond that, there should be a space after the comma and before the 4.

Why the triple quotes?

This is the size fix I was talking about before, I'm pretty sure. This should make the RFE microcode change you posted before unnecessary.

I'll see if that works... seems like it should.

i'll change it to a single

i'll look, but I'm not sure that it does. I'm pretty sure it still causes a mispredict without it.

OK, I just looked at this again.

The reason the code works is that before it gets to this part of the code, the BPUnit calls a generic squash that will take care of all branches after this (I guess that's why I left the comment above that line saying squash AFTER this ...)

It's not lucky so to speak, because the only thing left at the top of the prediction history is the actual squashing instruction and it *should* be OK to not have to search through the prediction history list here (phew!). 

Regrettably, I left the DPRINTF there preceding the assert because it's real annoying if you break at that point in the code and the assert doesnt give you enough information to trace the problem. 

However, I guess that's mostly a development issue so it's probably the right thing to do is WACK all of that code and leave in a assert or panic at that spot ensuring that the seqNum is the prediction history at the front.

Are you seeing cases where you have to search through the list there to get the branch prediction right?

That would solve the general case, but I still don't think it fixes rfe safely. If a load faults you could still end taking the fault with part of the rfe observed which would be incorrect behavior. 

Yea i was, that is why I changed it, but I can verify that it wasn't some other bug