AnsweredAssumed Answered

Value Comparison Giving the Wrong Result

Question asked by Peter Barbara on May 1, 2017
Latest reply on May 30, 2017 by stanish

    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

Outcomes