Re: [Simulavr-devel] [PATCH] Add BREAK-instruction, causes simulavr to halt.

[Petr Hluzín]

On 5 January 2012 22:52, Stan Behrens
<address@hidden> 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