Best way to see "optimizations"

I just want to understand what the optimized version of the code looks like

hidden from you, the code most look like this
        /* If Pressure2 is saturated low*/
        if ((Pressure2>>3) < (signed short)(P2SatLow))
        {
                DAC2MSB = (unsigned char)(((P2SatLow)&0xFF00)>>8);
                DAC2LSB = (unsigned char)((P2SatLow)&0xFF);

                DAC2Error = 1;
goto done
        }

        /* If Pressure2 is saturated high */
        else if ((Pressure2>>3) > (signed short)(P2SatHigh))
        {

                DAC2MSB = (unsigned char)(((P2SatHigh)&0xFF00)>>8);
                DAC2LSB = (unsigned char)((P2SatHigh)&0xFF);

                DAC2Error = 1;
goto done

        }

somewhere below
done:




an example of optimizing:

        /* If Pressure2 is saturated low*/
        if ((Pressure2>>3) < (signed short)(P2SatLow))
        {
                DAC2MSB = (unsigned char)(((P2SatLow)&0xFF00)>>8);
                DAC2LSB = (unsigned char)((P2SatLow)&0xFF);

goto seterr
        }

        /* If Pressure2 is saturated high */
        else if ((Pressure2>>3) > (signed short)(P2SatHigh))
        {

                DAC2MSB = (unsigned char)(((P2SatHigh)&0xFF00)>>8);
                DAC2LSB = (unsigned char)((P2SatHigh)&0xFF);
goto seterr

        }

somewhere below
seterr:       DAC2Error = 1;
done:

my code in optimization level 8 is sitting at 7.3k and I have more code to add, so I can't drop down the optimization level

why are you limited to 8k, that is almost archaic. If I were, for whatever reason, forced to run with a chip that would overflow with "debuggable optimization" (level 2) I would hurry up and get my devboard equipped with another chip.

Erik

Regardless though, I'd like to be able to view what exactly the code is doing.

Well, you'll have to make up your mind: either you have your cake, to look at and admire, or you eat it, to enjoy the taste.

That said, if you're really so hot on getting to see what actually happens, feel free to step through your code in the debugger's machine code view. That'll typically be a tremendous waste of time and effort, but hey, that's yours to waste, right?

Heh, honestly, I wish that was an option. It's a custom ASIC and the specs were negotiated before I was brought on board. I just have to work with what I have at this point.

Though, truth be told, I probably wouldn't have gone with an 8051 core, probably wouldn't have chosen Keil, and DEFINITELY wouldn't have gone with an OTP chip . . . but like I said, we work with what we have.

This is true. It is likely to be a waste of time but who knows, I could learn a bit more about this optimization setting (though, more than likely, I think I'm going to learn to stick with opt level 7 as level 8 seems to be the main culprit of my code "thinking for itself." Heh . . .

Depending on your debugger there may be a mode that shows the ASM generated for each line of C.
Is there and option to output that?

But if have code that works a t one optimization level and not another it may be a compiler bug. The compiler can optimize away a lot of things, but it still must be functionally equivalent. OR, it could be exposing a bug in your program.

But if have code that works a t one optimization level and not another it may be a compiler bug... OR, it could be exposing a bug in your program.

The latter is much more likely, of course. The language standard gives the compiler many opportunities to perform agressive optimizations. Working with a highly optimizing compiler takes some getting used to. That said, my impression is that C51 isn't exactly a highly optimizing compiler. Often the code it produces leaves a lot to be desired.

seems to be the main culprit of my code "thinking for itself."

You make that sound as if that were a bad thing. It's not.

Yes, optimized code can be hard to follow in a debugger. And level 8 isn't even as confusing as it gets. Wait till you see what linker-based optimizations can do to your code flow.

But all that shouldn't concern you all that much, unless you're convinced that the code actually broke because of it. In the case at hand, there's nothing wrong with the generated code ... it just found out that your source contained needless code duplication. It just transformed

if (a) {
something();
other();
} else {
something_else();
other();
}

into the exactly equivalent

if (a) {
something();
} else {
something_else();
}
other();

Heh, yeah, that was more of a tongue in cheek comment. Optimizations are a good thing for sure (let's be honest, if my code wasn't being optimized, I would've had an overflow problem and my code definitely wouldn't have fit).

Realistically, it is possible it's a bug with my code but the function looks pretty straight forward (as is posted above). Can anyone see any possible problems with it?

I have seen something similar to this with the debugger skipping part of my code and last time, it was definitely a code problem on my end so I'm not going to make excuses on this until I understand the problem. I figured I'd run it by you guys though, since the code is fairly simple (two if statements setting variables). Looking at it, I don't see too much it could do wrong (though I can think of other ways to write it and maybe that's what it's actually optimizing it to. And, actually, looking over at what you think is happening, that was more or less how I was going to rewrite it so that makes sense).

Thanks for the help!

But what is the range of your variables? May you get into signed/unsigned troubles with your type casts?

Sorry about that, I always forget to post the declarations of the variables.

 signed short idata Pressure2;
 unsigned short P2SatLow;
 unsigned short P2SatHigh;
 bit DAC2Error;

        /* If Pressure2 is saturated low*/
        if ((Pressure2>>3) < (signed short)(P2SatLow))
        {
                DAC2MSB = (unsigned char)(((P2SatLow)&0xFF00)>>8);
                DAC2LSB = (unsigned char)((P2SatLow)&0xFF);

                DAC2Error = 1;
        }

        /* If Pressure2 is saturated high */
        else if ((Pressure2>>3) > (signed short)(P2SatHigh))
        {

                DAC2MSB = (unsigned char)(((P2SatHigh)&0xFF00)>>8);
                DAC2LSB = (unsigned char)((P2SatHigh)&0xFF);

                DAC2Error = 1;
        }

I think I'm doing the correct cast conversions (if that's the correct term for it) but I could be wrong or maybe there's a "best practice" kind of thing I don't know that I'm missing.

I'm guessing this is all due to the way the code is choosing to optimize this portion based on "code folding" or whatever level 8 is. I just find it interesting that this is the only section that it seems to do it this way.

Oh well, not a huge deal. If anything jumps out at anybody, let me know. If not, no big deal. Thanks again!

Realistically, it is possible it's a bug with my code but the function looks pretty straight forward (as is posted above). Can anyone see any possible problems with it?

No. And I thought we had already told you that a couple of times by this point.

Perhaps if you write the code more "optimally" to start with, you won't get "surprises" from the optimiser...?

       pressure2 = Pressure2 >> 3;

        /* If Pressure2 is saturated low*/
        if (pressure2 < (signed short)(P2SatLow))
        {
                DAC2MSB = (unsigned char)(((P2SatLow)&0xFF00)>>8);
                DAC2LSB = (unsigned char)((P2SatLow)&0xFF);
        }

        /* If Pressure2 is saturated high */
        else if (pressure2  > (signed short)(P2SatHigh))
        {

                DAC2MSB = (unsigned char)(((P2SatHigh)&0xFF00)>>8);
                DAC2LSB = (unsigned char)((P2SatHigh)&0xFF);
        }

        DAC2Error = 1;

Perhaps if you write the code more "optimally" to start with, you won't get "surprises" from the optimiser...?

I disagree. Code should be written for clarity. Optimization should be left to the compiler most of the time.
Whether or not (Pressure2>>3) achieves clarity is another matter, of course.

I agree.

However, in this case, I think the "optimisations" (sic) that I suggested do, in fact, make the code clearer...

Except that you have changed the logic by moving DAC2Error = 1; outside of the if statements, thereby always setting the error - not just when value is above max or below min.