GPIO write not working in Release mode

[MikeMarynowski]

Hi,

The title pretty much says it all. My LCD refused to work when I ran my project in Release mode, so I started digging and found that the exact same code works fine in Debug but not in Release. The code basically just opens an 8-bit GPIO bus on PORT_AN using the MQX GPIO driver and writes all 1's to the port.

I setup user_config.h in the M52259DEMO libs for MQX 3.6 (enabled GPIODEV, disabled ADC, etc) and built build_m52259demo_libs project, so Debug and Release library should have the same features enabled.

CPU is MFC52254, running under a project ported from an M52259DEMO project. Project settings match everywhere in both Release and Debug modes, and CPU is selected as M52254 in all places.

The fopen("gpio:write", ...) function returns a valid result. Hooking up a probe in debug mode shows all PORT_AN lines at 3.3V (11111111) after the write call. In release mode, it goes to something like 01000010 after the write call.

Is there any other possible reason why my program fails to write values to PORT_AN only in Release mode?

Thanks,

--Mike

[MikeMarynowski]

Solved the problem, after days of banging my head against the wall trying to debug this. Your "high execution speed" optimizer is trying to do something clever with my code and is doing it very wrong.

Here is my code before:

void _lcd_set_data_pin_status(uint_8 data)

{

    uint_8 i;

    for (i = 0; i < 8; i++)

    {

       if ((data & (1 << i)) > 0)

       {

          _lcd_data_pins[i] |= GPIO_PIN_STATUS_1;

       }

       else

       {

          _lcd_data_pins[i] &= ~GPIO_PIN_STATUS_1;

       } 

   }

}

The lines inside the if and else blocks don't work properly in Release mode - the value inside the "_lcd_data_pins" array never gets modified, so it stays at whatever it was initialized to. Works fine in debug mode.

Here is what I had to change it to to get around the bug:

void _lcd_set_data_pin_status(uint_8 data)

{

    uint_8 i;

    uint_32 current;

    for (i = 0; i < 8; i++)

    {

       current = _lcd_data_pins[i];

       if ((data & (1 << i)) > 0)

       {

          _lcd_data_pins[i] = current | GPIO_PIN_STATUS_1;

       }

       else

       {

          _lcd_data_pins[i] = current & (~GPIO_PIN_STATUS_1);

       }

   }

}

[mjbcswitzerland]

Hi Mike

Could you please show what GPIO_PIN_STATUS_1 is?

Also please show the disassembled code in each case (in CW click on the file in the file manager and then use right-click to command disassemble).

I have used CW for about 5 years on the Coldfire V2s - always with highest optimisation - and have never experienced a bug of such a sort (there have been bugs but not as blatent as such a case). Comparing the disassembled code it may be possible to understand what is going wrong and verify whether it is a compiler bug or due to the optimiser taking advantage of storage elements not declared as being voilatile when they should be (often the cause of bit-banging problems).

Regards

Mark

[MikeMarynowski]

 Pasting in anything destroys line breaks for some reason, so I've attached the disassembly in a text file.

[MikeMarynowski]

Sorry, I misunderstood your comment about the volatile before. Why would I have to declare the variable as volatile? I understand the use of the volatile keyword in the context of letting a compiler know the value can be changed from somewhere else, but I'm changing the value by either setting or clearing a bit on each loop. What could possibly confuse the compiler here?

[TomE]

> Pasting in anything destroys line breaks for some reason,

That's what the "Insert Code" button is for in the edit title bar.

> so I've attached the disassembly in a text file.

Too long to read. Next time I'd suggest compiling it in both "Release" and "Debug" versions, comparing outputs and then posting a minimal set of differences, or at least commenting the disassembly with what you think is wrong.

> Sorry, I misunderstood your comment about the volatile before. Why would I have to declare the variable as volatile?

So that your code has a chance of working. Had you added "volatile" your code would have started working. Then you could have compared the disassembly and found out what changed.

> I understand the use of the volatile keyword in the context of letting

> a compiler know the value can be changed from somewhere else,

That only applies in "classic computing" where you are coding in a tightly controlled environment, running under an operating system, and where your code is basically doing "arithmetic in memory". It doesn't work all that well there either - see Wikipedia references below.

If you're writing to a hardware port and sequentially write the values "1, 2, 3, 4" to the same location, an optimising compiler is going to decide that sequence has the same effect as just writing "4", and so that's what it is likely to do. But the hardware may have required the four writes.

The other thing optimising compilers do is to change the ORDER of operations. So your code may be writing to hardware registers in a specific order, say the sequence "A=STOP; B=value; A=START;" and the compiler may do the write to "B" and then the two to "A" (if it does both of them at all).

In embedded programming, "volatile" means "do exactly what I typed, in that order".

Here's a better example, and also an example of using the "Insert Code" button:

http://gcc.gnu.org/onlinedocs/gcc-4.4.2/gcc/Volatiles.html#index-volatile-access-2953

volatile int *src=somevalue;*src;

Imagine what an optimising compiler would do to the above with and without "volatile".

I suggest you read the Wikipedia page on this (http://en.wikipedia.org/wiki/Volatile_variable, and click on the "Disassembly Comparison - Show"), and especially follow through to the link to "volatile-considered-harmful.txt" and "volatile-almost-useless-for-multi-threaded-programming".

Tom

[MikeMarynowski]

The problem function C code was pasted above, the line that does the bit setting in the disassembled output is very easy to find by skimming the disassembly as it is commented by the disassembler with the original source lines and there are only like 6 or 7 lines of C code in the entire disassembly output.

I am not qualified to comment on what is wrong, assembly isn't something I've touched in 10 years, and even then it was fairly trivial stuff just to get a basic understanding of how a computer works.

Your tone is rather unpleasant. I am well aware of what volatile means, thank you. I am not touching any registers, I am simply setting or clearing a bit in an array of uint32 variables in RAM. I am doing everything in a single task, in a linear fashion, I have no task or context switching going on. Nothing in that Wikipedia article or what you have said explains why I would need to mark an array of 8 in-memory integers as volatile for a loop that sets or clears a bit in each one.

[MikeMarynowski]

Also, though adding volatile may have well made my code start working, that's only because it would stop what I believe at the moment to be buggy optimizations. Obviously "because your code will start working" isn't the answer I'm seeking, I'm looking for why this situation requires the volatile keyword. Based on everything I know about it, it shouldn't, hence the hunch of an optimizer bug.

[TomE]

OK, I'm sorry, you're right. I made too many assumptions in responding to your post.

Let's say I responded to someone who didn't know the "leave it alone" meaning of "volatile" and who was writing directly to registers. That wasn't you at all.

Not providing the definition of "_lcd_data_pins[i]" or " or "GPIO_PIN_STATUS_1" makes it more difficult than it needs to be though. I assumed they might be writing directly to registers that might require 32-bit access, and the optimisation might have been performing byte-accesses.

I can't see any of your code that performs the "write values to PORT_AN" operation. Port A is an 8-bit register and your code is distributing bits-in-a-byte into Bit 30 of an array of something.

The difference between the two optimisation levels is:

DEBUG (Level 1 Optimizations)"i" is in "-12(a6)" and "d1";  354:          _lcd_data_pins[i] |= GPIO_PIN_STATUS_1; ;0x00000028  0x41F900000000           lea      __lcd_data_pins,a00x0000002E  0x41F01C00               lea      (a0,d1.l*4),a00x00000032  0x2D48FFF8               move.l   a0,-8(a6)0x00000036  0x206EFFF8               movea.l  -8(a6),a00x0000003A  0x2010                   move.l   (a0),d00x0000003C  0x08C0001E               bset     #30,d00x00000040  0x2080                   move.l   d0,(a0)RELEASE (Level 4 Optimizations)"i" is in "d0";  354:          _lcd_data_pins[i] |= GPIO_PIN_STATUS_1; 0x0000001C  0x41F900000000           lea      __lcd_data_pins,a00x00000022  0x7406                   moveq    #6,d20x00000024  0x05F02C00               bset     d2,(a0,d2.l*4)

The first one is correctly using "d1" as "i" in the address calculations of the array elements and the second one looks to be using a constant offset of "6" for all of them.

You should probably report this in the appropriate "CodeWarrior Development Tools" Forum rather than here, possibly with a simpler test-case that demonstrates the problem.

Tom

[MikeMarynowski]

I have posted the continuation of this in the other forum section as you recommended. For your information, here is my post:

https://community.freescale.com/thread/98826

[MikeMarynowski]

Hey,

Thanks for the reply. Sorry, I could have been clearer myself, I figured that indicating I was using the MQX GPIO driver was enough to make it clear what was going on, as there is really only one way to do it.

I am working on a simplified example that demonstrates the problem as we speak.

[MikeMarynowski]

GPIO_PIN_STATUS_1 is a bit flag defined in the MQX libraries. I don't have my MQX dev laptop with me right now, but it is a single bit set in a 32 bit value.

The two code blocks are clearly equivalent, both work without optimizations yet one fails on Release, so it is pretty clear to me that it is a compiler bug.

I will post code shortly.