Re: [m5-dev] Review Request: ARM: Identify branches as conditional or unconditional and direct or indirect.
On 2011-03-11 20:10:44, Ali Saidi wrote: src/cpu/o3/bpred_unit_impl.hh, line 354 http://reviews.m5sim.org/r/570/diff/1/?file=10843#file10843line354 Yea i was, that is why I changed it, but I can verify that it wasn't some other bug I'm not going to make any changes to the bpred_unit_impl.hh file... Apparently I fixed the issue I was having somewhere else. - Ali --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/570/#review955 --- On 2011-03-11 15:21:28, Ali Saidi wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/570/ --- (Updated 2011-03-11 15:21:28) Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. Summary --- ARM: Identify branches as conditional or unconditional and direct or indirect. Diffs - src/arch/arm/insts/branch.hh 5138d1e453f1 src/arch/arm/isa/insts/branch.isa 5138d1e453f1 src/arch/arm/isa/templates/branch.isa 5138d1e453f1 src/arch/arm/predecoder.hh 5138d1e453f1 src/arch/arm/types.hh 5138d1e453f1 src/cpu/o3/bpred_unit_impl.hh 5138d1e453f1 Diff: http://reviews.m5sim.org/r/570/diff Testing --- Thanks, Ali ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: ARM: Identify branches as conditional or unconditional and direct or indirect.
--- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/570/#review944 --- src/cpu/o3/bpred_unit_impl.hh http://reviews.m5sim.org/r/570/#comment1313 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. src/cpu/o3/bpred_unit_impl.hh http://reviews.m5sim.org/r/570/#comment1314 Yea, we could panic here instead of asserting. - Ali On 2011-03-11 15:21:28, Ali Saidi wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/570/ --- (Updated 2011-03-11 15:21:28) Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. Summary --- ARM: Identify branches as conditional or unconditional and direct or indirect. Diffs - src/arch/arm/insts/branch.hh 5138d1e453f1 src/arch/arm/isa/insts/branch.isa 5138d1e453f1 src/arch/arm/isa/templates/branch.isa 5138d1e453f1 src/arch/arm/predecoder.hh 5138d1e453f1 src/arch/arm/types.hh 5138d1e453f1 src/cpu/o3/bpred_unit_impl.hh 5138d1e453f1 Diff: http://reviews.m5sim.org/r/570/diff Testing --- Thanks, Ali ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: ARM: Identify branches as conditional or unconditional and direct or indirect.
--- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/570/#review946 --- src/arch/arm/isa/insts/branch.isa http://reviews.m5sim.org/r/570/#comment1318 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. src/arch/arm/isa/insts/branch.isa http://reviews.m5sim.org/r/570/#comment1319 Why the triple quotes? src/arch/arm/predecoder.hh http://reviews.m5sim.org/r/570/#comment1322 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. - Gabe On 2011-03-11 15:21:28, Ali Saidi wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/570/ --- (Updated 2011-03-11 15:21:28) Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. Summary --- ARM: Identify branches as conditional or unconditional and direct or indirect. Diffs - src/arch/arm/insts/branch.hh 5138d1e453f1 src/arch/arm/isa/insts/branch.isa 5138d1e453f1 src/arch/arm/isa/templates/branch.isa 5138d1e453f1 src/arch/arm/predecoder.hh 5138d1e453f1 src/arch/arm/types.hh 5138d1e453f1 src/cpu/o3/bpred_unit_impl.hh 5138d1e453f1 Diff: http://reviews.m5sim.org/r/570/diff Testing --- Thanks, Ali ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: ARM: Identify branches as conditional or unconditional and direct or indirect.
--- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/570/#review949 --- src/arch/arm/isa/insts/branch.isa http://reviews.m5sim.org/r/570/#comment1323 I'll see if that works... seems like it should. src/arch/arm/isa/insts/branch.isa http://reviews.m5sim.org/r/570/#comment1324 i'll change it to a single src/arch/arm/predecoder.hh http://reviews.m5sim.org/r/570/#comment1326 i'll look, but I'm not sure that it does. I'm pretty sure it still causes a mispredict without it. - Ali On 2011-03-11 15:21:28, Ali Saidi wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/570/ --- (Updated 2011-03-11 15:21:28) Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. Summary --- ARM: Identify branches as conditional or unconditional and direct or indirect. Diffs - src/arch/arm/insts/branch.hh 5138d1e453f1 src/arch/arm/isa/insts/branch.isa 5138d1e453f1 src/arch/arm/isa/templates/branch.isa 5138d1e453f1 src/arch/arm/predecoder.hh 5138d1e453f1 src/arch/arm/types.hh 5138d1e453f1 src/cpu/o3/bpred_unit_impl.hh 5138d1e453f1 Diff: http://reviews.m5sim.org/r/570/diff Testing --- Thanks, Ali ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: ARM: Identify branches as conditional or unconditional and direct or indirect.
--- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/570/#review953 --- src/cpu/o3/bpred_unit_impl.hh http://reviews.m5sim.org/r/570/#comment1328 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? - Korey On 2011-03-11 15:21:28, Ali Saidi wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/570/ --- (Updated 2011-03-11 15:21:28) Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. Summary --- ARM: Identify branches as conditional or unconditional and direct or indirect. Diffs - src/arch/arm/insts/branch.hh 5138d1e453f1 src/arch/arm/isa/insts/branch.isa 5138d1e453f1 src/arch/arm/isa/templates/branch.isa 5138d1e453f1 src/arch/arm/predecoder.hh 5138d1e453f1 src/arch/arm/types.hh 5138d1e453f1 src/cpu/o3/bpred_unit_impl.hh 5138d1e453f1 Diff: http://reviews.m5sim.org/r/570/diff Testing --- Thanks, Ali ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: ARM: Identify branches as conditional or unconditional and direct or indirect.
--- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/570/#review955 --- src/arch/arm/predecoder.hh http://reviews.m5sim.org/r/570/#comment1330 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. src/cpu/o3/bpred_unit_impl.hh http://reviews.m5sim.org/r/570/#comment1331 Yea i was, that is why I changed it, but I can verify that it wasn't some other bug - Ali On 2011-03-11 15:21:28, Ali Saidi wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/570/ --- (Updated 2011-03-11 15:21:28) Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. Summary --- ARM: Identify branches as conditional or unconditional and direct or indirect. Diffs - src/arch/arm/insts/branch.hh 5138d1e453f1 src/arch/arm/isa/insts/branch.isa 5138d1e453f1 src/arch/arm/isa/templates/branch.isa 5138d1e453f1 src/arch/arm/predecoder.hh 5138d1e453f1 src/arch/arm/types.hh 5138d1e453f1 src/cpu/o3/bpred_unit_impl.hh 5138d1e453f1 Diff: http://reviews.m5sim.org/r/570/diff Testing --- Thanks, Ali ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
