Value Comparison Giving the Wrong Result

[peterb2]

    With full optimisations on (I have only tried O3), doing a 'equals to' comparison between a bit-field of a struct and a value is coming back as the wrong result.  The problem is more complicated (subtle?) than that and I will try to explain it with an example of how it is manifesting itself in my particular case.  All the code below is from my example code (attached to this post) for reproducing the problem.

    I am trying to set up the trigger sources for a few eQADC command FIFOs.  The flow of code for how this is happening follows the path shown below.

// The order here is important for having the problem show up
...
result_ok = set_tsel(5, 0x40)     // SIU.ISEL4.R will become 0x40000000
...
result_ok = set_tsel(4, 0x40)     // SIU.ISEL4.R will become 0x40400000
...
result_ok = set_tsel(3, 0x40)     // SIU.ISEL4.R will become 0x40404000

    All these calls to set_tsel() should be returning true but only the first call to set_tsel() is returning true.  The other two calls are coming back false.  The problem I believe is in set_tsel() for how it is generating the result value.  set_tsel() works the following way.  

bool set_tsel(uint8_t tsel, uint8_t value)
{
     bool return_value = false;

     switch(tsel)
     {
     case 2:
          ISEL_TEST.B.CTSEL2_A = value;
          return_value = ((vuint8_t)ISEL_TEST.B.CTSEL2_A == (vuint8_t)(value));
          break;
     case 3:
          ISEL_TEST.B.CTSEL3_A = value;     // CTSEL3_A is 7 bits in size at bit 8 (assuming RHS is LSB)
          return_value = ((vuint8_t)ISEL_TEST.B.CTSEL3_A == (vuint8_t)(value));
          break;
     case 4:
          ISEL_TEST.B.CTSEL4_A = value;
          return_value = ((vuint8_t)ISEL_TEST.B.CTSEL4_A == (vuint8_t)(value));
          break;
     case 5:
          ISEL_TEST.B.CTSEL5_A = value;
          return_value = ((vuint8_t)ISEL_TEST.B.CTSEL5_A == (vuint8_t)(value));
          break;
     default:
          break;
     }

     return return_value;
}

    The code for calculating the result is fairly straightforward, set the result to true when the the register is equal to the incoming value.  The problem I believe is in how the compiler has generated the asm for doing the check.  It is not masking the entire register value correctly for extracting the bitfield that is the focus of the comparison. This bad mask cascades to the result of the comparison being wrong.

// This is the checking of tsel 3
// return_value = ((vuint8_t)ISEL_TEST.B.CTSEL3_A == (vuint8_t)(value));
e_lwz          r3,04(r9)          // r3 is set to the register value. 0x40404000
e_rlwinm     r3,r3,24,25,23     // r3 is rotated right by 8 bits, the mask here is suspect,
                                   // it effectively does nothing. 
                                   // Also ME is less than MB. 
xor               r4,r3,r4          // r4 was originally, 0x00000040. Results in being 0x00404000
cntlzw          r4,r4               // r4 becomes 9, number of leading zeros in r4
se_srwi          r4, #05               // r4 / 32 = 0
se_mr          r3,r4               // r3 (the return_value) is set to false.

To reproduce:

* Start a new S32DS project

* Select MPC5777C (Not sure if this matters)

* Set language for all cores to C++

* Set the library for all cores to NewLib

* Set the optimisation level to O3 (Properties -> C/C++ Build -> Settings -> Standard S32DS C++ Compiler -> Optimisation)

* Copy in the attached files and compile

    You will find the problem to occur when optimisation is O3 and to not be there when the optimisation is off O0.  In tsel.cpp it is possible to switch the code to using ram instead of a register to help show that it is not some quirk on memory access to that particular register.

    If you have any questions please let me know.

Original Attachment has been moved to: tsel.h.zip

Original Attachment has been moved to: tsel.cpp.zip

Original Attachment has been moved to: main_Z7_0.cpp.zip

[stanish · Accepted solution]

Hello Peter,

Thank you very much for such a detailed description of the issue you are facing!

I can confirm this is indeed an issue with the current GCC compiler (S32DS v1.1)

This issue will be addressed in the next version of  S32 Design Studio v1.2. The release date has been postponed by two weeks (16-June-2017) due to some other compiler related fix. See below the disassembly listing for the new compiler with -O3:

29                  ISEL_TEST.B.CTSEL3_A = value;
00811874:   e_lis r7,16384 
00811878:   e_lwz r6,36(r7)  //load ISEL_TEST to register r6
0081187c:   e_rlwimi r6,r4,8,17,23 // rotate value left to the appropriate bit-field, R4 = value
00811880:   e_stw r6,36(r7) // store result (r6) into ISEL_TEST

30                  return_value = ((vuint8_t)ISEL_TEST.B.CTSEL3_A == (vuint8_t)(value));
00811884:   e_lwz r3,36(r7)  // Load ISEL_TEST to r3=0x40404000
00811888:   e_rlwinm r3,r3,24,25,31 //rotate left word imm. and with mask r3=0x40
0081188c:   xor r3,r3,r4   // XOR ISEL_TEST with Value = 0x0
00811890:   cntlzw r3,r3   // count leading 0 of the xor result r3=0x20
00811894:   se_srwi r3,5   // shift right r3 = 1 (True)
31                  break;
00811896:   se_blr ‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍

Regards,

Stan