[Question] !ENABLE_ICACHE_PACKET_BREAKING && !PERFECT_BP about scarab HOT 8 CLOSED

I need to look at the code to understand this potential bug/incompatability better. But the bug aside, I think you should get the behavior you want by leaving ENABLE_ICACHE_PACKET_BREAKING as enabled and setting CFS_PER_CYCLE to the number of CF instructions that you want to support per packet. By default, CFS_PER_CYCLE is set to 3, so not sure why you are seeing that scarab breaks after every CF instruction. If you want to break after predicted taken branches, you can set FETCH_BREAK_ON_TAKEN.

In the default PARAM file in the src directory, which is probably the most tested configuration in the past couple of years, we use:
--fetch_break_on_taken 1
--cfs_per_cycle 6

from scarab.

ok, I found a way to address the problem by setting PB_BREAK_AFTER after a mispred/misfetch has been detected in the icache_stage:

  if(mispred || misfetch ||
     (USE_LATE_BP && (late_mispred || late_misfetch))) {
    ic->off_path = TRUE;
    if(FETCH_OFF_PATH_OPS) {
       ....
    } else {
      *break_fetch = BREAK_OFFPATH;
      packet_break = PB_BREAK_AFTER;    <----
    }

Let me know if this makes sense and I can send a pull request. I think the patch should be ok be cause the intention seems to be to break anyways on a mispred/misfetch. The path below ensures that we break immediately after the mispred/misfetched branch

from scarab.

@siavashzk The problem is that ENABLE_ICACHE_PACKET_BREAKING causes breaks on multiple conditions. The mentioned CFS_PER_CYCLE break is not the problem but the same ENABLE_ICACHE_PACKET_BREAKING is used a couple lines below again to also break on all CF instructions. BTW, this has a huge impact on IPC and I think it is unrealistic for most architectures.

from scarab.

Yes, I agree breaking on every branch is unrealistic. I don’t think the intended behavior of the knob was to break after every CF instruction. I’ll look into this tomorrow.

from scarab.

Hi Hiener,

The packet_build code is confusing, but it turns out that @spruett had looked into this before and has a detailed response on how it works in this past issue. The summary is that break_fetch and packet_break serve different purposes, and on every CF instructions, only packet_breat is set, not break_fetch. So, based on our current understanding and what we observe in our debug runs, we are not breaking after every branch. Are you seeing different behavior?

Regarding the bug with the !ENABLE_ICACHE_PACKET_BREAKING && !PERFECT_BP combo, I think your fix looks good, and just did it myself.

from scarab.

Siavash and I realized there is one other point on this matter worth mentioning. It is possible you are seeing IPC differences because by disabling ENABLE_ICACHE_PACKET_BREAKING you may be changing the icache access pattern. Please see below.

Again, the way the code is written is very confusing here, so I am going to apologize in advance for the confusing variable names.

There are two variables of interest here: break_fetch and packet_break.

If break_fetch is set, this essentially ends the cycle at the current instruction. In other words, icache_issue_ops will continue to issue ops in the SAME cycle UNTIL break_fetch is set, which will cause the while loop in update_icache_stage to end.

packet_break serves a completely different purpose. Each time packet_break is set, this will cause another i-cache access to be modeled (see update_icache_stage function). This is what the knob ENABLE_ICACHE_PACKET_BREAKING is enabling/disabling. When ENABLE_ICACHE_PACKET_BREAKING=true, there will be an i-cache access when:

1. Any branch is seen (see line packet_build.c:247) or
2. When a cache line boundary is reached (see line packet_build:226)

When ENABLE_ICACHE_PACKET_BREAKING=false, then the number of icache accesses may go down, because the icache_issue_ops function is not properly returning to update_icache_stage to model the next cache access. In the worst case, if you fetch multiple taken branches in one cycle, you could end up skipping an access to an i-cache line all together.

One last thing to note: right now packet_break is set after any branch (packet_build.c:247). It may be more accurate to change this to only be set after TAKEN branches. This is likely increasing accesses to the same cache line, and thereby increase the ICACHE_HIT stat, which will affect the perceived cache hit rate and power modeling.

from scarab.

from scarab.

Closing this due to inactivity. Feel free to open it back up again.

from scarab.