Перейти к содержанию
    

Камень в огород ненавистников inline-ассемблера ;) Или превратности си-оптимизации.

Наступил тут намедни на очень неприятный баг особенность работы си-компилятора IAR. О чём речь:

Испокон веку Давно во всех своих программах (для IAR for Cortex-M) использую критические секции, описанные следующим образом:

//Critical sections
#define CPU_SR_ALLOCATE() u32 cpu_sr  //Allocate storage for CPU status register
#define ENTR_CRT_SECTION() do { cpu_sr = __get_PRIMASK(); __disable_interrupt(); } while (0)
#define EXIT_CRT_SECTION() __set_PRIMASK(cpu_sr)

//Использование в программе:
void func()
{
  CPU_SR_ALLOCATE();
  ...  //код за пределами крит.секции
  ENTR_CRT_SECTION();
  ...  //код внутри крит.секции
  EXIT_CRT_SECTION();
  ...  //код за пределами крит.секции
}

где: __get_PRIMASK(), __set_PRIMASK(), __disable_interrupt() - стандартные intrinsic-функции IAR.

Использовал их и всё вроде бы ничего - прокатывало. Но недавно наступил на неприятные грабли. :suicide2:

Участок кода, в котором была функция, содержащая в себе крит.секцию:

Спойлер
void main()
{
  ...
  sysTimer = 0;
  SysTickInit(CCLK_MH / (OS_TICKS_PER_SEC * SYSTICK_DIV));
  IntCpuEna();
  #ifdef LOG
  LogInit(LOG_RATE);
  #endif
  LogCR0(COL_BLUE "--%s--" COL_BLACK " Version firmware: %u.%03u" COL_GRAY " (CRC:" COL_BLACK "%08X" COL_GRAY ")",
    concat(FIRMWARE_TARGET_STR_, FIRMWARE_TARGET),
    VERSION_FIRMWARE_MSB, VERSION_FIRMWARE_LSB, __checksum);
  if (Iap(IAP_CMD_READ_DEV_SN) == IAP_RES_CMD_SUCCESS)
    LogCR0(COL_BLACK "Device SN: " COL_NAVY "%08X.%08X.%08X.%08X", d0.iap.out[0], d0.iap.out[1], d0.iap.out[2], d0.iap.out[3]);
  ...
}

static uint Iap(uint cmd)
{
  CPU_SR_ALLOCATE();
  d0.iap.cmd = cmd;
  ENTR_CRT_SECTION();
  ptrIAP(&d0.iap.cmd, &d0.iap.status);
  EXIT_CRT_SECTION();
  return d0.iap.status;
}

 

Был скомпилён в следующее безобразие:

Спойлер
...
 sysTimer = 0;                                                                                                      
0x.... 0x....      LDR.W    R1,??DataTable18                                                                        
0x2000             MOVS     R0,#+0                                                                                  
0x6008             STR      R0,[R1, #+0]                                                                            
 SysTickInit(CCLK_MH / (OS_TICKS_PER_SEC * SYSTICK_DIV));                                                           
0xF3EF 0x8910      MRS      R9,PRIMASK                                                                              
0xF44F 0x40FA      MOV      R0,#+32000                                                                              
0x.... 0x....      BL       _Z11SysTickInitm                                                                        
 IntCpuEna();                                                                                                       
0x.... 0x....      BL       IntCpuEna                                                                               
 #ifdef LOG                                                                                                         
 LogInit(LOG_RATE);                                                                                                 
0xF44F 0x20E1      MOV      R0,#+460800                                                                             
0x.... 0x....      BL       _Z14ServiceLogInitm                                                                     
 #endif                                                                                                             
 LogCR0(COL_BLUE "--%s--" COL_BLACK " Version firmware: %u.%03u" COL_GRAY " (CRC:" COL_BLACK "%08X" COL_GRAY ")",   
   concat(FIRMWARE_TARGET_STR_, FIRMWARE_TARGET),                                                                   
   VERSION_FIRMWARE_MSB, VERSION_FIRMWARE_LSB, __checksum);                                                         
0x9400             STR      R4,[SP, #+0]                                                                            
0x2302             MOVS     R3,#+2                                                                                  
0x2200             MOVS     R2,#+0                                                                                  
0x.... 0x....      ADR.W    R1,?_1                                                                                  
0x.... 0x....      ADR.W    R0,?_0                                                                                  
0x.... 0x....      LDR.W    R4,??DataTable18_10                                                                     
0x.... 0x....      BL       _Z12ServiceLogCRPKcz                                                                    
 if (Iap(IAP_CMD_READ_DEV_SN) == IAP_RES_CMD_SUCCESS)                                                               
0xF44F 0x7008      MOV      R0,#+544                                                                                
0x1825             ADDS     R5,R4,R0                                                                                
0x203A             MOVS     R0,#+58                                                                                 
0x6028             STR      R0,[R5, #+0]                                                                            
0xB672             CPSID    I                                                                                       
0xF44F 0x700D      MOV      R0,#+564                                                                                
0x4420             ADD      R0,R4,R0                                                                                
0x9001             STR      R0,[SP, #+4]                                                                            
0x4601             MOV      R1,R0                                                                                   
0x.... 0x....      LDR.W    R8,??DataTable18_11  ;; 0x1fff1ff1                                                      
0x4628             MOV      R0,R5                                                                                   
0x47C0             BLX      R8                                                                                      
0xF389 0x8810      MSR      PRIMASK,R9
...

 

Где функция IntCpuEna() - внутри себя разрешает прерывания (написана на ассемблере). До её вызова по коду прерывания запрещены.

И получается ситуация, что запоминается в R9 состояние регистра PRIMASK при ещё запрещённых прерываниях. В то время как ожидалось, что он запомнит состояние PRIMASK после разрешения оных (каковое есть после IntCpuEna()). Соответственно - ожидаемого восстановления состояния разрешённости прерываний (командой MSR PRIMASK, R9) не происходит. Ну и программа начинает себя вести неправильно (проще говоря - виснет).

Причём(!!!!!!) :  Внутри SysTickInit() есть обращения к регистрам периферии (естественно объявленным volatile), а внутри LogInit() и LogCR0() - множество таких же критических секций (входов и выходов в них) и обращений к регистрам периферии. Но это не помешало компилятору перебросить MRS R9,PRIMASK выше вызовов всех этих функций.

Но как же объяснить компилятору, что нельзя переносить точки вызова intrinsic-функций??? Понятно, что можно cpu_sr объявить как volatile. Или весь макрос ENTR_CRT_SECTION() преобразовать в inline volatile функцию. Но это будет крайне некрасиво (с точки зрения оптимизации такого кода). Хотелось бы остаться в парадигме тривиального макроса, не прибегая к inline-функциям.

Можно в конце концов наверное даже впендюрить в макрос какие-то pragma временного отключения оптимизации, действием на один макрос (если такие есть). Но это ещё более кривое решение.  :umnik2:

Вышел из ситуации посредством inline-ассемблера:

#define ENTR_CRT_SECTION() asm(" MRS      %0, PRIMASK\n" " CPSID    I" : "=&r"(cpu_sr) ::)

хотя сам не люблю его использовать, но ничего лучшего не придумал. После чего всё стало компилиться корректно: MRS Rx,PRIMASK теперь не убегает далеко от CPSID I.

Или может кто-нить предложит более корректное решение? На intrinsic-функциях или как-то иначе.

 

PS: Уже после написания множества буков в этом посте, подумал, что возможно для данной темы более подходит раздел: https://electronix.ru/forum/index.php?app=forums&module=forums&controller=forums&id=137

Прошу уважаемых модераторов не судить строго за досадный промах и если посчитаете нужным - перенести тему в соответствующий раздел.

Изменено пользователем haker_fox
Тему перенёс. Тут явно прослеживается проблема среды разработки IAR.

Поделиться сообщением


Ссылка на сообщение
Поделиться на другие сайты

5 минут назад, VladislavS сказал:

asm volatile("....") не пробовали?

Так asm и без volatile работает нормально. Исходная реализация была на intrinsic-функциях. Чтобы без inline asm.

Просмотрел кучу листингов - с реализацией ENTR_CRT_SECTION() посредством inline asm, компилятор нигде ни на такт не разнёс MRS Rx,PRIMASK от CPSID I.

 

PS: Всё написанное конечно было сказано касательно компиляции на максимальной оптимизации (Balanced).

Поделиться сообщением


Ссылка на сообщение
Поделиться на другие сайты

12 минут назад, VladislavS сказал:

Можно компилятор "напугать" каким-нибудь барьером типа ISB.

Проверил: В этом конкретном месте он напугался, но в других местах оказался смелее:

0xF3EF 0x8910      MRS      R9,PRIMASK                      
 CPU_SR_ALLOCATE();                                         
 do {                                                       
   u32 c = ptr->cfg, i = ptr->pin_port;                     
       ??PinSelNV_0: (+1)                                   
0xF890 0xE000      LDRB     LR,[R0, #+0]                    
0x7845             LDRB     R5,[R0, #+1]                    
   u32 i1, i2 = i & 31;                                     
   u32 volatile *bbio = BITBAND_RAM(&GPIO[i1 = i >> 5], i2);
0xEA4F 0x145E      LSR      R4,LR,#+5                       
0xEB08 0x1644      ADD      R6,R8,R4, LSL #+5               
0xF106 0x7688      ADD      R6,R6,#+17825792                
0xF00E 0x0C1F      AND      R12,LR,#0x1F                    
0x0176             LSLS     R6,R6,#+5                       
0xEB06 0x068C      ADD      R6,R6,R12, LSL #+2              
   ENTR_CRT_SECTION();                                      
0xF3BF 0x8F6F      ISB                                      
0xB672             CPSID    I                               

Получено при:

#define ENTR_CRT_SECTION() do { __ISB(); cpu_sr = __get_PRIMASK(); __disable_interrupt(); } while (0)

Да и как такого напугаешь? Если он даже volatile-обращений к памяти не боится! Что ему какая-то ISB??? Пыль под ногами!  :new_russian:

Он видимо не из пугливых....

Поделиться сообщением


Ссылка на сообщение
Поделиться на другие сайты

А что там в описании __get_PRIMASK()? Только на моей памяти эти описания переделывались ARM-ом несколько раз, и один я точно помню был с косяком.

Поделиться сообщением


Ссылка на сообщение
Поделиться на другие сайты

У IAR это встроенная функция. 

 #define __arm_rsr           __iar_builtin_rsr
 #define __get_PRIMASK()             (__arm_rsr("PRIMASK"))

 

Поделиться сообщением


Ссылка на сообщение
Поделиться на другие сайты

12 минут назад, Arlleex сказал:

А что там в описании __get_PRIMASK()? Только на моей памяти эти описания переделывались ARM-ом несколько раз, и один я точно помню был с косяком.

Вроде ничего криминального:

__get_PRIMASK
Syntax unsigned long __get_PRIMASK(void);
Description Returns the value of the PRIMASK register. This intrinsic function can only be used in 
privileged mode and it requires a Cortex-M device.

PS: IAR 7.80.4

Поделиться сообщением


Ссылка на сообщение
Поделиться на другие сайты

Нет нет, я про Сишное описание/определение (пардон) из заголовочных файлов))

36 минут назад, VladislavS сказал:

У IAR это встроенная функция. 

 #define __arm_rsr           __iar_builtin_rsr
 #define __get_PRIMASK()             (__arm_rsr("PRIMASK"))

 

Ааа... Хреново😒

Поделиться сообщением


Ссылка на сообщение
Поделиться на другие сайты

Похоже, что для выполнения строки

if (Iap(IAP_CMD_READ_DEV_SN) == IAP_RES_CMD_SUCCESS)

у компилятора есть вся необходимая информация уже после выполнения строки

SysTickInit(CCLK_MH / (OS_TICKS_PER_SEC * SYSTICK_DIV));

поэтому он и считает, что имеет право оптимизировать её.

Он не стал выполнять переход на Iap() как на функцию и развернул её, а потом с оптимизировал и вынес наверх. Может попытаться заставить его не разворачивать, например убрать static у функции Iap (у меня в проверяем коде сработало).

Можно кусок проекта с неработающим кодом, чтобы проверить, а то не получается повторить ошибку.

Поделиться сообщением


Ссылка на сообщение
Поделиться на другие сайты

А я думаю, что реализация intrinsic у IAR кривая(( не понимает, что MRS/MSR имеет побочные эффекты, несмотря на отсутсвие доступов к памяти...

Поделиться сообщением


Ссылка на сообщение
Поделиться на другие сайты

2 часа назад, technik-1017 сказал:

Он не стал выполнять переход на Iap() как на функцию и развернул её, а потом с оптимизировал и вынес наверх. Может попытаться заставить его не разворачивать, например убрать static у функции Iap

Дело не оптимизации или разворачивании Iap(). В этом ничего страшного нет. Дело в том, что он переместил intrinsic-функции, которые были внутри Iap(). Переместил их точки вызова, причём так что - переместил даже через volatile обращения к памяти и через вызовы других intrinsic-функций.

Ознакомьтесь внимательнее с приведённым листингом. Там всё видно.

1 час назад, Arlleex сказал:

А я думаю, что реализация intrinsic у IAR кривая(( не понимает, что MRS/MSR имеет побочные эффекты, несмотря на отсутсвие доступов к памяти...

Возможно. Возможно не хватает спецификатора volatile при их объявлении.

Поделиться сообщением


Ссылка на сообщение
Поделиться на другие сайты

9 часов назад, jcxz сказал:

функция IntCpuEna() - внутри себя разрешает прерывания (написана на ассемблере). До её вызова по коду прерывания запрещены.

Критическая секция - это вещь непрерывная. Без относительно того, на каком языке она написана и каким компилятором скомпилирована. Прерывания запрещаются в ее начале и разрешаются в конце. Нельзя "сделать паузу" в критической секции - временно разрешить прерывания, а потом запретить их вновь. Получится две отдельных критических секции, с неизвестной временной паузой между ними, из-за возможных прерываний. Так же, крайне нежелательно, когда вход в критическую секцию находится в одной процедуре, а выход из нее в другой. В идеале, это должен быть единый линейный участок кода, без всяких ветвлений, переходов и вызовов других процедур. Как можно более короткий. Лишних команд там быть не должно - только самые необходимые.
Поскольку эти, простые и довольно очевидные, положения в программе не соблюдены - компилятор, видимо, пытается исправить ситуацию самостоятельно. Как может. И как умеет...

Поделиться сообщением


Ссылка на сообщение
Поделиться на другие сайты

Исходное состояние прерываний при их запрете нужно запоминать. А при разрешении - восстанавливать записанное.

Я сделал так

//Запрет прерываний
uint32_t MyDisableIrq()
{uint32_t IrqStat=__get_PRIMASK();
 __disable_interrupt();
 return IrqStat;
};


//Разрешение прерываний
void MyEnableIrq(uint32_t IrqStat)
{ __set_PRIMASK(IrqStat);
};

И проблемы ушли.

uint32_t MyIrqState=MyDisableIrq();

...

MyEnableIrq(MyIrqState);

 

Поделиться сообщением


Ссылка на сообщение
Поделиться на другие сайты

4 часа назад, quark сказал:

Поскольку эти, простые и довольно очевидные, положения в программе не соблюдены - компилятор, видимо, пытается исправить ситуацию самостоятельно.

:shok:  Вы это о чём??? Где именно "не соблюдены"? Где эти "временно разрешить прерывания, а потом запретить их вновь"?

Укажите конкретные строки, пожалуйста!

Вы приведённый код вообще смотрели??? :fool:

32 минуты назад, rudy_b сказал:

Исходное состояние прерываний при их запрете нужно запоминать. А при разрешении - восстанавливать записанное.

Ещё один... :unknw:

Уважаемые "пейсатели": НАЧИНАЕМ НАКОНЕЦ ЧИТАТЬ О ЧЁМ ИДЁТ РЕЧЬ В ТЕМЕ, В КОТОРУЮ ОТВЕЧАЕМ !!!

И не нести ахинею.

Поделиться сообщением


Ссылка на сообщение
Поделиться на другие сайты

Присоединяйтесь к обсуждению

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

Гость
Ответить в этой теме...

×   Вставлено с форматированием.   Вставить как обычный текст

  Разрешено использовать не более 75 эмодзи.

×   Ваша ссылка была автоматически встроена.   Отображать как обычную ссылку

×   Ваш предыдущий контент был восстановлен.   Очистить редактор

×   Вы не можете вставлять изображения напрямую. Загружайте или вставляйте изображения по ссылке.

×
×
  • Создать...