On 5 January 2012 22:52, Stan Behrens <lists.nongnu.org-simulavr-de...@sbeh.de> wrote: > Hi Petr, > > On 05/01/12 21:39, Petr Hluzín wrote: >> Why does your patch add instructionBREAKJustExecuted to AvrDevice and >> then have the if() condition in AvrDevice::Step()? >> It would be easier to do the tracing in avr_op_BREAK::operator()() and >> return BREAK_POINT. Function AvrDevice::Step() and class AvrDevice >> would be left as is. > Yeah, this was what the first version of my patch did. I had to change > that, because the PC was not incremented this way. Result was, your > stuck at BREAK forever once you reach it.
You are right, now I see. The condition if(cpuCycles != BREAK_POINT) in AvrDevice::Step() would become false. However when AvrDevice::Step() is entered and evaluates if(hwWait) condition to false and evaluates "else if(cpuCycles <= 0)" to true, then cpuCycles is 0. If there are no breakpoints and simulation does not end then a pending interrupt may cause cpuCycles = 4 and DecodedInstruction is not executed. If there is no pending interrupt then then DecodedInstruction are executed but current code does never return BREAK_POINT or other negative value. No matter if interrupt is scheduled or not the condition if(cpuCycles != BREAK_POINT) is always true and therefore we can redefine what should the code do when an instruction wants to do a breakpoint. I propose this: If a BREAK instruction at address X is executed then GDB will show a instruction at address X+2 as the next one. This will be accomplished by: 1) removing condition if(cpuCycles != BREAK_POINT) 2) doing "return BREAK_POINT+1" from avr_op_BREAK::operator()() We would get rid of one conditional jump in performance sensitive path, better understandable code and quite good behavior of BREAK instruction. The other possibility is to have the condition as is and do "core->PC++; return 0;" avr_op_BREAK::operator()(). It is slightly more code and is a workaround for condition which is probably useless. I am asking you folks because I do not know why the condition if(cpuCycles != BREAK_POINT) actually exists. (Also we could get rid of the "cpuCycles--;" just after execution and either update all instructions or tests in Step(). But that is cosmetics.) -- Petr Hluzin _______________________________________________ Simulavr-devel mailing list Simulavr-devel@nongnu.org https://lists.nongnu.org/mailman/listinfo/simulavr-devel