Optimization Assembly Problems

Where are your data types?

And what was the value of Pressure1?

How do you know the code doesn't select the 'else' part?

By the way - why write:

if (commData == 0x61)   /* 0x61 = 'a' */

instead of

if (commData == 'a')

Doesn't it feel strange to use a construct that forces you to have a comment to tell you the important information?

Where are your data types?

And what was the value of Pressure1?

How do you know the code doesn't select the 'else' part?

By the way - why write:

if (commData == 0x61)   /* 0x61 = 'a' */

instead of

if (commData == 'a')

Doesn't it feel strange to use a construct that forces you to have a comment to tell you the important information?

Per,

Thank you for your reply. The Pressure1, DiffPressure1, and DigPressure1 are all unsigned shorts(though it should be noted, there are currently being stored in the idata section, not sure if that is causing issues). Here is the code below:

        unsigned short idata Pressure1 = 0;
        unsigned short idata DigPressure1 = 0;
        unsigned short idata DiffPressure1 = 0;

The reason I'm using 0x61 is because the program we are interfacing with has to be entered in hex form (hence the 0x61), though you are correct. I could use the 'a' and place the 0x61 in the comment section.

The value of Pressure1 is zero (as that section of my code is not processed if DAC1DifVoltn is equal to one (which it is, it's initialized in an init function at the beginning of the program). I have been simulating and placing breakpoints in each location, which is how I know I'm getting different values based on the optimization level.

If I'm doing something incorrectly or you have an alternate suggestion, please let me know. Thanks!

It can be dangerous to try to look at variable values at high optimization levels, since the compiler can switch execution sequence and throw away actions without side effects. And same memory/register can represent one variable in one part of the code, and another variable at a different point in the code.

Do you get wrong values actually sent out on the serial port?

Yes, I do get incorrect values on the serial port. That's what actually alerted me of this issue.

Interestingly enough, when I go into the loops that just right shift pressure1 by 3 places, I don't seem to have these issues (which definitely points to an issue with this section. I just don't understand what would cause it to store incorrect data here. Could the issue be because DigPressure1 is an idata register or something odd like that?). In fact, I more or less get garbage code rather than a consistent output, from what I can see (it takes a new sample every 4ms or so, so it's giving me new data faster than I'm requesting it).

The whole situation is very odd, since it seems to only affect one branch.

You use an 8-bit processor. But your variables are larger than 8 bits. So how do you perform atomic operations on them? Are any of the variables volatile or getting updated by an interrupt handler?

Have you made sure you have room enough for your stack?

Here is a screenshot of the assembly code being executed:

imgh.us/DiffAssemblyCode.jpg

As far as your other questions go, I will admit, I don't know how to answer your questions regarding atomic operations or my stack (though, to be fair, I haven't had this issue in any other variable storage location, even when I try and shift a variable by 3 spaces and then place it into a variable, as shown in my else statement (which appears to work correctly).

Regarding variables and interrupts, I have a pressure value that is updated in an ISR, which is then processed by a separate function and stored into Pressure1. Thus far, I haven't had any issues related to Pressure1.

DiffPressure is created by subtracting Pressure1 from Pressure2 and then processed to calculate the desired differential voltage. It is then stored in DiffPressure1 (though I am currently working with a saturated differential channel, hence the 0x0FF8). I haven't used any volatile functions at this point as I haven't needed them. Not sure if this error is a result of this but I can step through everything and all the values are correct and the program steps through without any issues but that line I mentioned in a previous post is causing an issue when trying to store 1 unsigned int into another (it didn't have any problems when I tried to store Pressure1>>3 into this same DigPressure1 so I'm guessing the issue is with the DiffPressure1 variable. Except it has the correct value, it's just unable to store it into DigPressure1 correctly, which is just odd.

It's kind of frustrating because storing one variable into another seems to be about as easy as it gets (especially when compared to the complexity of my current code). Maybe it's a really simple solution but, unfortunately, I can't see it yet.

Thanks for your help, Per!

I think it is an atomicity issue that show up when the timing changes due to optimization.

Erik

My only objection to that is when I am testing this code, I am hardcoding values into those variables and still having the issue. I think you might still be right about me having a potential issue with atomicity but I think I might still have some other issue directly related to storing DiffPressure1 into DigPressure1. Or could that still be an atomicity issue, even with hardcoded values?

Your disassembly screenshot shows that the code contains absolute register accesses. IIRC you have to be careful with assigning register banks to ISR's. Or I could be wrong and this is handled automatically by the compiler. Anyway, you can test this theory by ticking 'Don't use absolute register accesses' checkbox in compiler settings. Of course, less efficient code will be generated because of that.

My only objection to that is when I am testing this code, I am hardcoding values into those variables and still having the issue.
when doing so, do you disable the interrupt associated with In CommBuff ISR:

I do not but ISRs must be directly called when in simulation mode so I assume variable values would not be updating at the same time a variable is being called on (maybe this is a false assumption). Or is my understanding of the term "atomicity" incorrect? (if I understand it correctly, it's having a value called while it's being updated, causing incorrect values to be read in).

There are other ways I could approach this, with the CommBUff ISR collecting the commData and then using that data external to the CommBuff ISR. It would cause a delay in communication (since my program would have to execute before checking the value) and there is a possibility a previous piece of data could be overwritten before it's handled (though I could place all input values into a queue or array). I'm not sure if this would be a better approach (I guess if it solves my issue, it is). I still have trouble wrapping my head around why it would be an atomicity issue when in simulation mode though (once again, assuming my understanding is correct).

For all intents and purposes, this is the call I am using to call the CommBuff ISR:

/*I have a check that finds a value saturated and outputs this value */
DiffPressure1 = 0x0FF8;
COMBUF = 'a';
commBuff_ISR();

So the value of DiffPressure1 is definitely stored prior to the commBuff ISR being called.

An ISR should never be called directly.

The ISR should be activated by a hardware event of some kind.

Some interrupt sources will trig automatically when simulating - for example timer interrupts, ADC conversion done interrupts etc. Some interrupts requires that you have a simulation script to trig the event - such as changing a state on a processor pin, or sending a character to the UART.

Atomicity issues happens when the main loop reads one byte of a multi-byte variable, and an interrupt happens and the interrupt handler modifies this multi-byte variable. When the interrupt ends, the main loop will continue with the read. So some bytes was read before the update and some after the update.

Same thing can happen when the processor have a timer or similar with a 16-bit counter that ticks continuously, and the code reads these two bytes in the wrong order in relation to how the timer was designed. Then the counter can tick from 0x12ff to 0x1300 and the main loop either got 0x1200 or 0x13ff. Note that the 16-bit peripherial registers have a design so when the 8-bit code reads a specific "first" byte, the hardware latches the other byte into a special latch to make sure the two 8-bit reads will catch a correct 16-bit value. Reading in wrong order makes this behind-the-scenes latch functionality fail.

So atomicity issues comes from variables being updated asynchronously, in relation to the code that want to use the variables. Interrupts are asynchronous in relation to the main loop. Peripherial hardware registers are asynchronous to both main loop and interrupt handlers.

Atomicity isn't just relating to multiple reads to get the full content of a larger variable. It can also happen from read/modify/write situations. The code loads the variable (potentially atomic) and then modifies the value and writes back the changed value.

If the main loop performs a load, and the code then gets interrupted, then the ISR can also do a read+modify+write before returning to main. So main then do an update of a value that is already non-current and then writes back a broken value.

Thanks for the information on atomicity. I believe I am handling this correctly as when I receive a 'a' character, I am updating the values into temp destinations (such as DigPressure1) so if the other variables are updated, it won't affect the pressure values I took at that time. I don't modify DiffPressure1 or Pressure1 though so I don't believe I will cause any value disruptions.

Regarding the direct calls to the ISR, I only do that in simulation mode (as you said, ISRs should be handled by hardware). Since there is no hardware in simulation mode though, the code won't execute if the ISRs are not called (since a majority of my code is interrupt/flag driven in that code won't execute until a variable is updated and a bit is set). In simulation mode, I just directly call the ISRs because I don't know any other way to simulate a hardware call (up til now, it hasn't caused any issues).

Don't worry, I rely on hardware for ISRs once they're programmed on a chip!

Since there is no hardware in simulation mode though, the code won't execute if the ISRs are not called (since a majority of my code is interrupt/flag driven in that code won't execute until a variable is updated and a bit is set). In simulation mode, I just directly call the ISRs because I don't know any other way to simulate a hardware call (up til now, it hasn't caused any issues).

You're grievously underestimating this simulator. Keil has simulations for just about all peripherals of just about all known '51 derivates, including fully simulated interrupts.

Calling IRQ functions directly really is a bad idea, if only because it fails to simulate the most important aspect that differentiates interrupts from normal code flow: the fact that interrupts can and will happen anywhere. And of course the registers of the peripherals that those interrupts are supposed to have come from will be in the wrong state, so the interrupts themselves can't even work properly.

You end up with a simulated program that is wildly different from the real thing, rendering any results of such simulation suspect, and many of them useless.

Wow, if there's a way to simulate hardware interrupts, you're right, that would absolutely be the best way to approach it. I didn't realize Keil had these capabilities. Is it pretty easy to setup?