volatile всё-таки нужен или нет?, всё работает, но страшно Опции

[sigmaN]

Имеется программный ШИМ:

Код

typedef struct
{
    uint8_t    counter;
    uint8_t    top;
    uint8_t    compare_val;
    uint8_t    pin;
    uint8_t    enabled;
    volatile uint8_t    *port;        
} pwm_channel;

static pwm_channel    channels[PWM_MAXCHANNELS];


ISR(TIMER1_COMPA_vect)
{
    uint8_t    i;
    
    for ( i = 0; i < chnls_cnt; i++ )
    {        
        if ( ! channels[i].enabled )
            continue;
        
        channels[i].counter++;
        if ( channels[i].counter == channels[i].compare_val )
        {
            *channels[i].port |= 1 << channels[i].pin;        
        }
        else
        {    
            if ( channels[i].counter >= channels[i].top )
            {
                channels[i].counter = 0;
                *channels[i].port &= ~(1 << channels[i].pin);                                
            }    
        }                        
    }    
}

При этом channels[i].enabled, channels[i].top и channels[i].compare_val меняются посредством вызова функции spwm_configchannel(), нужно ли делать их volatile и почему всё работает и без этого?
Верно ли утверждение, что все глобальные переменные, доступ к которым осуществляется как в прерывании, так и в основном цикле нужно обязательно объявлять с volatile?
Я в принципе так всегда и делал, но вот тут вдруг всё работает даже с -O3

P.S. скурил Nine ways to break your systems code using volatile.. страшно. Щас ещё баръеры памяти расставлять побегу )

[xemul]

При этом channels[i].enabled, channels[i].top и channels[i].compare_val меняются посредством вызова функции spwm_configchannel(), нужно ли делать их volatile и почему всё работает и без этого?
Верно ли утверждение, что все глобальные переменные, доступ к которым осуществляется как в прерывании, так и в основном цикле нужно обязательно объявлять с volatile?

volatile должны быть объявлены глобальные переменные, которые и _изменяются_ в прерывании, и используются вне прерывания.
channels[i].enabled, channels[i].top и channels[i].compare_val в прерывании не изменяются, и им volatile'ность не требуется.
Используется ли вне прерывания channels[i].counter++, не понятно.
Тем не менее, прерывание TIMER1_COMPA при исполнении spwm_configchannel() может привести к сбою формирования ШИМ, если не подстелить соломки.
(н-р, из приведённого кода не очевидно, что в spwm_configchannel() после "channels[i].enabled = 0" выполняется что-то, устанавливающее пин в определённое состояние, вроде "*channels[i].port &= ~(1 << channels[i].pin);", или что не приключится пропуск такта ШИМ при изменении channels[i].compare_val, или что ...; но, возможно, оно Вам и не актуально)

[Сергей Борщ]

Верно ли утверждение, что все глобальные переменные, доступ к которым осуществляется как в прерывании, так и в основном цикле нужно обязательно объявлять с volatile?

Да, верно.

volatile должны быть объявлены глобальные переменные, которые и _изменяются_ в прерывании, и используются вне прерывания.
channels[i].enabled, channels[i].top и channels[i].compare_val в прерывании не изменяются, и им volatile'ность не требуется.

А вот это неверно. Если оставить их без volatile, то компилятор может в основном цикле просто закешировать их значение в регистрах и никогда не складывать в память. А может вообще выкинуть все операции с этими переменными, ибо в основном цикле есть только запись в эти переменные, но нет чтения. Следовательно, результат этой записи никому не нужен и считать его тоже не нужно.

но вот тут вдруг всё работает даже с -O3

Если хотите после обновления компилятора или изменения какой-нибудь галочки в его настройках или изменения исходника долго и с удовольствием искать, почему же все вдруг перестало работать - оставьте так. Если же будете добавлять volatile, то в местах, где есть несколько доступов к одной переменной вроде вот этого:

CODE

        channels[i].counter++;
        if ( channels[i].counter == channels[i].compare_val )

используйте временную не-volatile переменную:

CODE

        uint8_t counter_cache = channels[i].counter++;
        if ( counter_cache == channels[i].compare_val )

[xemul]

А вот это неверно. Если оставить их без volatile, то компилятор может в основном цикле просто закешировать их значение в регистрах и никогда не складывать в память. А может вообще выкинуть все операции с этими переменными, ибо в основном цикле есть только запись в эти переменные, но нет чтения. Следовательно, результат этой записи никому не нужен и считать его тоже не нужно.

Тогда необходимо объявлять volatile любую переменную, которая в одной единице компиляции только читается, а в другой - только пишется.

[sigmaN]

Используется ли вне прерывания channels[i].counter++, не понятно.
Тем не менее прерывание TIMER1_COMPA при исполнении spwm_configchannel() может привести к сбою формирования ШИМ, если не подстелить соломки.

да да, знаю. В моём случае никаких проблем это не вызывает.

Если хотите после обновления компилятора или изменения какой-нибудь галочки в его настройках или изменения исходника долго и с удовольствием искать, почему же все вдруг перестало работать - оставьте так.

Вот да! Я так и думал!

Если же будете добавлять volatile, то в местах, где есть несколько доступов к одной переменной вроде вот этого: используйте временную не-volatile переменную:

Хороший совет. Спасибо!

[Herz]

Если же будете добавлять volatile, то в местах, где есть несколько доступов к одной переменной вроде вот этого:

Код

        channels[i].counter++;
        if ( channels[i].counter == channels[i].compare_val )

используйте временную не-volatile переменную:

Код

        uint8_t counter_cache = channels[i].counter++;
        if ( counter_cache == channels[i].compare_val )

Это чтобы при прерывании между обращениями к переменной не пришлось работать с разными её значениями?

[_Pasha]

Немного о другом.
Зачем в теле цикла инкрементировать счетчик для каждого канала, если он для всех один?
У Вас есть варианты, когда каналы совсем независимые? channels[i].top разный?
ps

CODE

typedef struct
{
uint8_t compare_val;
uint8_t pin;
uint8_t enabled;
volatile uint8_t *port;
} pwm_channel;

static volatile pwm_channel channels[PWM_MAXCHANNELS];
static volatile uint8_t counter, top;

ISR(TIMER1_COMPA_vect)
{
uint8_t i;
if(++counter > top) counter = 0;

for ( i = 0; i < chnls_cnt; i++ )
{
if ( ! channels[i].enabled ) continue;
if(counter == channels[i].compare_val)
*channels[i].port |= 1 << channels[i].pin;
else if (!counter)
*channels[i].port &= ~(1 << channels[i].pin);
}
}

Сообщение отредактировал _Pasha - Mar 9 2013, 12:01

[sigmaN]

Это чтобы при прерывании между обращениями к переменной не пришлось работать с разными её значениями?

но ведь в примере мы уже в прерывании. Это чтобы полностью не связывать руки оптимизатору. Программист то точно знает, что после channels[i].counter++; эта переменная не изменится и её можно закешировать для дальнейшего использования в цикле.

У Вас есть варианты, когда каналы совсем независимые? channels[i].top разный?

Да. Каналы абсолютно независимы. И top и compare_val задаются индивидуально. Это пасхалка, чтоб музычку сыграть на форсунках )))

[Herz]

но ведь в примере мы уже в прерывании. Это чтобы полностью не связывать руки оптимизатору. Программист то точно знает, что после channels[i].counter++; эта переменная не изменится и её можно закешировать для дальнейшего использования в цикле.

Тогда ничего не понял. Если она точно не изменится, зачем её кэшировать?

[sigmaN]

В приведенном примере после инкремента счётчика я в цикле дважды сравниваю его с другими переменными. А ведь может быть такое, что не два раза, а 30 раз или 100. И здесь кэширование в локальную переменную даст выигрыш, потому что компилятор сможет оптимизировать доступ, вместо того чтобы каждый раз вычитывать её из памяти. И как раз тот факт, что эта переменная не изменится(и только программист это знает наверняка) позволяет нам её закешировать и дать компилятору добро на оптимизацию.

[Herz]

Понятно, спасибо. Хотя я отнёс бы такой случай к исключительным и кэшировал, только если действительно нужно много обращений. 30 или 100? Даже не могу себе такое представить...

[sigmaN]

Ну 30 или 100 это я так, для контраста придумал )
Хотя... не так уж и фантастично звучит, когда цикл проходится по нескольку десятков объектов...может быть не в прерывании, может быть прерывание запрещено или стоит какой-то иной флаг, который гарантирует, что сейчас можно volatile переменную безболезненно закешировать и выиграть на этом...а может быть и не на AVR всё это происходит ))
Тут как-бы сама идея интересная же.

[XVR]

Тогда необходимо объявлять volatile любую переменную, которая в одной единице компиляции только читается, а в другой - только пишется.

Нет. Компилятор производит межпроцедурный анализ, что бы определить, какие переменные и где читаются и пишутся. Если компилятор не может произвести такой анализ (ну например у него нет режима оптимизации полной программы, такого как LTO), то он будет придерживаться пессимистических предположений, и считать что все (ну почти все) глобальные переменные могут измениться при вызове неизвестной ему функции. Но сами места вызовов он все же видит. Т.е. он считает, что если никаких функций не вызывалось, то и измениться ничего глобального не могло (я несколько упрощаю). С прерываниями все гораздо хуже - компилятор не знает, когда они происходят, так что либо он должен считать, что где угодно может произойти прерывание, которое может изменить что угодно (а это сразу ставит крест на любых оптимизациях с участием не локальных переменных), либо отдать это на откуп программисту (с модификатором volatile)

[xemul]

Нет...

Вообще-то, примерно это я и написал (и даже перечислил, то, что не требует объявления volatile, т.к. волатильно _не_изменяется_; даже то, что волатильно изменяется, но опять же не требует volatile).
Отквоченная Вами фраза - доказательство (точнее, в данном случае, опровержение другого утверждения) от противного, ПМСМ, довольно очевидное.

[sigmaN]

volatile должны быть объявлены глобальные переменные, которые и _изменяются_ в прерывании, и используются вне прерывания.
channels[i].enabled, channels[i].top и channels[i].compare_val в прерывании не изменяются, и им volatile'ность не требуется.
Используется ли вне прерывания channels[i].counter++, не понятно.

Хотелось-бы пруфлинк, если можно. Сколько копал на тему volatile - нигде не видел подобного разделения на то где требуется, а где не требуется volatile.