Modulo operator is not executed by avr-g++ on template parameter.

Go To Last Post
6 posts / 0 new
Author
Message
#1
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

The following template is a very simple ring buffer.

template<typename T, typename U, int N>
class RingBufferUnchecked
{
public:
    RingBufferUnchecked(): ptrWr_m(0), size_m(0) {}

    void put(T c)
    {
        buffer_m[ptrWr_m] = c;
        ++ptrWr_m;
        ptrWr_m %= N;
        if(!isFull()) ++size_m;
    }
    
    void clear() {
        ptrWr_m =  0;
        size_m = 0;
    }

    bool isEmpty() {return size() == 0;}
    bool isFull() {return size() >= N;}

    U size() { return size_m; }

    const T operator[](U idx)
    { 
        U i = (ptrWr_m - 1 - idx) % N;
        return buffer_m[i]; 
    }

private:
    volatile U ptrWr_m;
    volatile U size_m;
    volatile T buffer_m[N];
};

The size of the history buffer is a template parameter (N) as well as the type of the pointer (U) and the data (T).

operator[] is a random access to the history buffer. Index is relative to the write pointer, so index 0 means the latest entry, and further indices go back in history.

Everything works nice, if the modulo of the turnover calculation stands in a new line:

    const T operator[](U idx)
    {
        U i = (ptrWr_m - 1 - idx);
        i %= N;
        return buffer_m[i];
    }

However with a single line expression, the modulo is not executed:

    const T operator[](U idx)
    {
        U i = (ptrWr_m - 1 - idx) % N;
        return buffer_m[i];
    }

I tested it with avr-gdb, and pretty sure, this is what happens. What do I miss here? Can it be related with the fact that N is a template parameter?

This topic has a solution.
Last Edited: Mon. Feb 11, 2019 - 03:49 PM
This reply has been marked as the solution. 
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1

norien wrote:

    const T operator[](U idx)
    {
        U i = (ptrWr_m - 1 - idx);
        i %= N;
        return buffer_m[i];
    }

 

Perhaps i can end up a negative number?  Maybe add N:

 

    const T operator[](U idx)
    {
        U i = (ptrWr_m + N - 1 - idx) % N;
        return buffer_m[i];
    }

 

 

--Mike

 

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

The c'tor code (right under public:) appears to have both opening AND closing brace on the one line so the code that follows is surely orphaned? Can't understand how this actually compiles?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

clawson wrote:
The c'tor code (right under public:) appears to have both opening AND closing brace on the one line so the code that follows is surely orphaned? Can't understand how this actually compiles?
It doesn't.

OP did not copy and paste.

 

Regardles, I want some types and some numbers and how they were obtained.

Iluvatar is the better part of Valar.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Thanks for your time to check it. You  are quite right, two lines were accidentally deleted in the copy-paste. I copied it from an ssh-vim environment end had to reshape it, then I did not realize it. My only excuse is that I did it after hours of debugging this problem.

 

I re copy-pasted the code in the initial question. I double checked it and it is now completely identical to my code.

 

Apologies again.

 

The ringbuffer is used by a global ErrorHandler object. Every call of ErrorHandler::set(ErrorCode err) stacks this code into the ring buffer:

class ErrorHandler
{
public:
    ErrorHandler(OStream* ostream): ostream_m(ostream){}
    enum ErrorCode {
        OK = 0,
        INSTRUCTION_NOT_FOUND = 1,
        INSTRUCTION_LONG = 2,
        TEST_OK = 3,
        TEST_FAILED = 4,
        ARG_ERROR = 5,
        ARG_NOT_FOUND = 6,
        CONFIG_ERROR = 7,
        RBUFF_OFL = 8,
        WBUFF_OFL = 9
    };
    typedef unsigned char Index; /// Change type here if you need more than 256 entries.                                                                                                                                                      

    /*! Get PGM_P address of the text message that belongs to en error code.
     * \param err Error code.
     * \return Address of the text message string in PROGMEM.
     */
    PGM_P textMessage(ErrorCode err);                                                                                                                                                                                                         

    void set(const ErrorCode err) {buffer_m.put(err);}
    Index size() {return buffer_m.size();}
    ErrorCode operator[](const Index i) { return buffer_m[i]; }                                                                                                                                                                               

    void assert(const char* text);                                                                                                                                                                                                            

    void printAt(const Index idx);
    void printLastError();
    void printAllErrors();
private:
    static const char MSG_OK[];
    static const char MSG_INSTRUCTION_NOT_FOUND[];
    static const char MSG_INSTRUCTION_LONG[];
    static const char MSG_TEST_OK[];
    static const char MSG_TEST_FAILED[];
    static const char MSG_ARG_ERROR[];
    static const char MSG_ARG_NOT_FOUND[];
    static const char MSG_CONFIG_ERROR[];
    static const char MSG_RBUFF_OFL[];
    static const char MSG_WBUFF_OFL[];                                                                                                                                                                                                        

    static PGM_P const msg_c[];                                                                                                                                                                                                               

    RingBufferUnchecked<ErrorCode, Index, 8> buffer_m;
    OStream* ostream_m;                                                                                                                                                                                                                       

};  

 

Last Edited: Mon. Feb 11, 2019 - 08:54 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

avr-mike, thanks, it looks like you've actually hit it. I checked the compiled lss files, and it looks like the answer is the good old signed-unsigned conversion convention in C++.

Everything in the parenthesis is unsigned, but the template parameter N is signed int. When idx is greater than ptrWr - 1, the result turns to -1. Thus the result before the modulo in

 

U i = (ptrWr_m + N - 1 - idx) % N;

will be -1. The modulo is executed on this number, and  (-1 % 8)  is -1, the result is going to be converted to signed int: 0xffff. And apparently to use it as an index is out of range.

As opposed to this:

U i = (ptrWr_m - 1 - idx);
i %= N;

where the modulo will be executed on +255 and will give 7 - which is a good index.

Frankly, I am ashamed, because any operation between signed-unsigned integers should be used with great care.