We are running a survey to help us improve the experience for all of our members. If you see the survey appear, please take the time to tell us about your experience if you can.
Hello everyone,
So, I have a program I'm working on an 8051W core chip using Keil's C51 software and have a loop that looks 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; } /* 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; }
My first case has Pressure2 saturated low so it goes through the DAC2MSB and LSB commands but then enters the DAC2Error portion in the Sat High routine. Now, from a functional standpoint, it doesn't matter, it all works out exactly the same. Regardless though, I'd like to be able to view what exactly the code is doing. I currently have optimization level 8 (which has code folding, I believe, which is what is causing it to combine these two. When I go to optimization level 7, it enters the correct DAC2Error code in the SatLow section).
Unfortunately, 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 (well, I could for now but I'd have to remove it down the road when I add additional code). At this point, I just want to understand what the optimized version of the code looks like but I'm not sure if that's possible.
Let me know if any of you have any suggestions about how to examine this. Thanks!
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!
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;
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.
Oops!