|
Плохой код при обмен данных между двумя структурами |
|
|
|
Aug 12 2009, 12:08
|

Частый гость
 
Группа: Участник
Сообщений: 83
Регистрация: 4-08-09
Из: Болгария / София
Пользователь №: 51 737

|
У меня есть 2 структуры out и pos которые хранят несколько байтов. Где то в коде хочу установить данные в out структуре: CODE out.shadowX1 = pos.X1; /* shadow register X1 */ out.shadowY1 = pos.Y1; /* shadow register Y1 */ out.shadowX2 = pos.X2; /* shadow register X2 */ out.shadowY2 = pos.Y2; /* shadow register Y2 */ out.shadowX1 = pos.X1; /* shadow register X1 */ Адреса всех элементов известны в момент компиляции так что я ожидал каждое присвоение потребляет 2 команды. С удивлением нашел такой код: CODE 0000B8 8160 LD R22,Z 0000BA 936C ST X,R22 out.shadowY1 = pos.Y1; /* shadow register Y1 */ 0000BC 8161 LDD R22,Z+1 0000BE EAA1 LDI R26,0xA1 0000C0 E0B1 LDI R27,0x01 0000C2 936C ST X,R22 out.shadowX2 = pos.X2; /* shadow register X2 */ 0000C4 8162 LDD R22,Z+2 0000C6 EAA2 LDI R26,0xA2 0000C8 E0B1 LDI R27,0x01 0000CA 936C ST X,R22 out.shadowY2 = pos.Y2; /* shadow register Y2 */ 0000CC 8163 LDD R22,Z+3 0000CE EAA3 LDI R26,0xA3 0000D0 E0B1 LDI R27,0x01 0000D2 936C ST X,R22 Я знаю что структуры "любят" работать с индекс регистры, но здесь нет никакой логики (оптимизация кода на макс по скорости). Попробовал поставить: CODE *(uint8_t *)(&out.shadowX1) = *(uint8_t *)(&pos.X1); но результат тот же. Как можно вразумить компилера не делать такие дурные вещи?
Сообщение отредактировал Student2 - Aug 12 2009, 12:09
|
|
|
|
|
 |
Ответов
(15 - 29)
|
Aug 12 2009, 16:32
|

Йа моск ;)
     
Группа: Модераторы
Сообщений: 4 345
Регистрация: 7-07-05
Из: Kharkiv-city
Пользователь №: 6 610

|
Цитата LDS и дольше на такт и длиннее вдвое. Длиннее - это ладно, оптимизация по скорости. А вот про дольше - не согласен, вот свежак AVR Instruction Set 0856H–AVR–07/09: Цитата LDS – Load Direct from Data Space ... Words: 2 (4 bytes) Cycles: 2
--------------------
"Практика выше (теоретического) познания, ибо она имеет не только достоинство всеобщности, но и непосредственной действительности." - В.И. Ленин
|
|
|
|
|
Aug 13 2009, 04:03
|

Частый гость
 
Группа: Участник
Сообщений: 83
Регистрация: 4-08-09
Из: Болгария / София
Пользователь №: 51 737

|
Обновил компилер на 5.30. С первого момента я был щастлив увидеть что код для которы говорил уже сделан как надо. Но потом увидел другие нехорошие вещи. Если обыем кода (speed optimization) при 5.20 был 7810 то с 5.30 он уже 7834. Ето не так страшно если имеем в виду что оптимизация по скорости. Все таки нашел места где переменные на абсолютном адресе снова загружаются через индекс при том в перерывание. Например: uint8_t index_RW; /* статик переменная */ ..... /* interrupt routine */ index_RW++; ..... компилировалась в интерупте как : CODE index_RW++; 000108 01FD MOVW R30,R26 00010A 8101 LDD R16,Z+1 00010C 9503 INC R16 00010E 8301 STD Z+1,R16 000110 C01D RJMP 0x14C Так что не спешите идти к 5.30
|
|
|
|
|
Aug 13 2009, 05:20
|

Частый гость
 
Группа: Участник
Сообщений: 83
Регистрация: 4-08-09
Из: Болгария / София
Пользователь №: 51 737

|
Цитата(Rst7 @ Aug 13 2009, 08:05)  И какую альтернативу Вы предлагаете? Нет альтернатива - надо идти к 5.30 - чем быстрее тем лучше. Но надо быть осторожным чтобы не сломать уже работающие проекты. Успел решить проблему с index_RX ! Если поставить переменную как external и код работает как надо - переменная надо дефинировать в другом модуле. Переменная index_RX была дефинирована только в I2C модуле. Может быть это какая то проблема с компилере - все переменные локальные для данного модуля (но статик) если недоступные для других модулей обрабатываются как структуры (это только гипотеза но дает объяснение эффекта).
Сообщение отредактировал Student2 - Aug 13 2009, 05:22
|
|
|
|
|
Aug 13 2009, 08:13
|

Гуру
     
Группа: Модераторы
Сообщений: 8 455
Регистрация: 15-05-06
Из: Рига, Латвия
Пользователь №: 17 095

|
Цитата(Student2 @ Aug 13 2009, 08:20)  Успел решить проблему с index_RX ! А в чем была проблема? Вам не нравится, что компилятор использует LDD/STD вместо LDS/STS? Так код с LDD/STD даже с учетом добавочной перегрузки R26 в R30 получался на 2 байта короче, чем Код LDS R16, index_RW INC R16 STS index_RW, R16 Мне кажется компилятор делал правильно.
--------------------
На любой вопрос даю любой ответ"Write code that is guaranteed to work, not code that doesn’t seem to break" ( C++ FAQ)
|
|
|
|
|
Aug 13 2009, 08:20
|

Частый гость
 
Группа: Участник
Сообщений: 83
Регистрация: 4-08-09
Из: Болгария / София
Пользователь №: 51 737

|
Цитата(Сергей Борщ @ Aug 13 2009, 11:13)  А в чем была проблема? Вам не нравится, что компилятор использует LDD/STD вместо LDS/STS? Так код с LDD/STD даже с учетом добавочной перегрузки R26 в R30 получался на 2 байта короче, чем Я согласен с Вами! Ошибка моя.... Тоже лучше для капсулации кода не показывать переменные (private) для других секции кода если этого не надо - так что возвратил все как было сначале.
Сообщение отредактировал Student2 - Aug 13 2009, 08:26
|
|
|
|
|
Aug 13 2009, 10:20
|

Йа моск ;)
     
Группа: Модераторы
Сообщений: 4 345
Регистрация: 7-07-05
Из: Kharkiv-city
Пользователь №: 6 610

|
Цитата А в чем была проблема? Вам не нравится, что компилятор использует LDD/STD вместо LDS/STS? Так код с LDD/STD даже с учетом добавочной перегрузки R26 в R30 получался на 2 байта короче, чем Короче, но медленнее. А разговор вроде изначально шел об оптимизации по скорости. Вообще, я считаю, что IAR в режиме оптимизации по скорости недостаточно агрессивно использует LDS/STS. Часто это приводит к освобождению регистровой пары Z или X, и как результат - к общему увеличению быстродействия. А вообще, код надо было немного по другому писать, щас попробую показать как (минут через 30-40).
--------------------
"Практика выше (теоретического) познания, ибо она имеет не только достоинство всеобщности, но и непосредственной действительности." - В.И. Ленин
|
|
|
|
|
Aug 13 2009, 10:29
|

Частый гость
 
Группа: Участник
Сообщений: 83
Регистрация: 4-08-09
Из: Болгария / София
Пользователь №: 51 737

|
Цитата(Rst7 @ Aug 13 2009, 13:20)  А вообще, код надо было немного по другому писать, щас попробую показать как (минут через 30-40). Я буду благодарен для помощи!
|
|
|
|
|
Aug 13 2009, 10:58
|

Йа моск ;)
     
Группа: Модераторы
Сообщений: 4 345
Регистрация: 7-07-05
Из: Kharkiv-city
Пользователь №: 6 610

|
Ну вот из Вашего тестового проекта сделал вот так CODE #include "system.h" #include "memory.h" #include "I2C.h"
uint8_t indexADDR; uint8_t _index_RW; uint8_t stateI2C;
#pragma vector=TWI_vect __interrupt void TWI_ISR( void ) { uint8_t *pointer; uint8_t index_RW=_index_RW; uint8_t s=out.status; do { uint8_t status_TWI = TWSR; uint8_t data_TWI = TWDR; uint8_t flagGoTo_IDLE = 0; switch (stateI2C) { case 2: { if (status_TWI == 31) { if(index_RW == 0) { TWDR = s; s&=69;
out.shadow.X1 = pos.X1; /* !!!!!!!!!!!!!!! */ out.shadow.Y1 = pos.Y1; /* !!!!!!!!!!!!!!!!!! */ out.shadow.X2 = pos.X2; /* !!!!!!!!!!!!!!!!!!*/ out.shadow.Y2 = pos.Y2; /* !!!!!!!!!!!!!!!!! */ out.shadow.X1Size = pos.X1Size; /* !!!!!!!!!!!!!!! */ out.shadow.Y1Strength = pos.Y1Strength; /* !!!!!!!!!!!!!!! */ out.shadow.X2Size = pos.X2Size; /* !!!!!!!!!!!!!!! */ out.shadow.Y2Strength = pos.Y2Strength; /* !!!!!!!!!!!!!!! */ index_RW++; } else if(index_RW < 8) { pointer = (uint8_t*)(&out) + index_RW; TWDR = *pointer; index_RW++; } else if(index_RW < 12) { if (index_RW & 1) { pointer = (uint8_t*)(®.reference) + (index_RW - 55); } else { pointer = (uint8_t*)(®.rawSignal) + (index_RW - 24); } TWDR = *pointer; index_RW++; } else { TWDR = 34; } } else { flagGoTo_IDLE++; } break; } case 3: { if (status_TWI == 70) { index_RW = indexADDR; TWDR = index_RW; stateI2C = 88; } else if(status_TWI == 39) { stateI2C = 55; } else { } break; } case 4: { if (status_TWI == 75) {
if(index_RW == 0) { s&= data_TWI | 45; index_RW++; } else if (index_RW < 51) { index_RW++; } else if (index_RW <= 56) { pointer = (uint8_t*)(&out) + index_RW; *pointer = data_TWI; index_RW++; } else { } } else { flagGoTo_IDLE++; } break; } case 5: { if (status_TWI == 67) { indexADDR = index_RW = data_TWI; stateI2C = 45; } break; } } TWCR = 12; } while (TWCR_TWINT); out.status = s; _index_RW=index_RW; }
CODE \ In segment CODE, align 2, keep-with-next 10 __interrupt void TWI_ISR( void ) \ TWI_ISR: 11 { \ 00000000 93FA ST -Y, R31 \ 00000002 93EA ST -Y, R30 \ 00000004 936A ST -Y, R22 \ 00000006 935A ST -Y, R21 \ 00000008 934A ST -Y, R20 \ 0000000A 933A ST -Y, R19 \ 0000000C 932A ST -Y, R18 \ 0000000E 931A ST -Y, R17 \ 00000010 930A ST -Y, R16 \ 00000012 B76F IN R22, 0x3F \ 00000014 9100.... LDS R16, (indexADDR + 1) \ 00000018 9120.... LDS R18, `out` \ 0000001C E050 LDI R21, 0 12 uint8_t *pointer; 13 uint8_t index_RW=_index_RW; 14 uint8_t s=out.status; 15 do 16 { 17 uint8_t status_TWI = TWSR; \ ??TWI_ISR_0: \ 0000001E 9110.... LDS R17, _A_TWSR 18 uint8_t data_TWI = TWDR; \ 00000022 9130.... LDS R19, _A_TWDR 19 uint8_t flagGoTo_IDLE = 0; 20 switch (stateI2C) \ 00000026 9140.... LDS R20, (indexADDR + 2) \ 0000002A 5042 SUBI R20, 2 \ 0000002C F051 BREQ ??TWI_ISR_1 \ 0000002E 954A DEC R20 \ 00000030 F409 BRNE $+2+2 \ 00000032 C04C RJMP ??TWI_ISR_2 \ 00000034 954A DEC R20 \ 00000036 F409 BRNE $+2+2 \ 00000038 C055 RJMP ??TWI_ISR_3 \ 0000003A 954A DEC R20 \ 0000003C F409 BRNE $+2+2 \ 0000003E C063 RJMP ??TWI_ISR_4 \ 00000040 C06A RJMP ??TWI_ISR_5 21 { 22 case 2: { 23 24 if (status_TWI == 31) \ ??TWI_ISR_1: \ 00000042 311F CPI R17, 31 \ 00000044 F009 BREQ $+2+2 \ 00000046 C067 RJMP ??TWI_ISR_5 25 { 26 if(index_RW == 0) \ 00000048 2300 TST R16 \ 0000004A F529 BRNE ??TWI_ISR_6 27 { 28 TWDR = s; \ 0000004C 9320.... STS _A_TWDR, R18 29 s&=69; \ 00000050 7425 ANDI R18, 0x45 30 31 32 out.shadow.X1 = pos.X1; /* !!!!!!!!!!!!!!! */ \ 00000052 9100.... LDS R16, pos \ 00000056 9300.... STS (`out` + 1), R16 33 out.shadow.Y1 = pos.Y1; /* !!!!!!!!!!!!!!!!!! */ \ 0000005A 9100.... LDS R16, (pos + 1) \ 0000005E 9300.... STS (`out` + 2), R16 34 out.shadow.X2 = pos.X2; /* !!!!!!!!!!!!!!!!!!*/ \ 00000062 9100.... LDS R16, (pos + 2) \ 00000066 9300.... STS (`out` + 3), R16 35 out.shadow.Y2 = pos.Y2; /* !!!!!!!!!!!!!!!!! */ \ 0000006A 9100.... LDS R16, (pos + 3) \ 0000006E 9300.... STS (`out` + 4), R16 36 37 out.shadow.X1Size = pos.X1Size; /* !!!!!!!!!!!!!!! */ \ 00000072 9100.... LDS R16, (pos + 4) \ 00000076 9300.... STS (`out` + 5), R16 38 out.shadow.Y1Strength = pos.Y1Strength; /* !!!!!!!!!!!!!!! */ \ 0000007A 9100.... LDS R16, (pos + 5) \ 0000007E 9300.... STS (`out` + 6), R16 39 out.shadow.X2Size = pos.X2Size; /* !!!!!!!!!!!!!!! */ \ 00000082 9100.... LDS R16, (pos + 6) \ 00000086 9300.... STS (`out` + 7), R16 40 out.shadow.Y2Strength = pos.Y2Strength; /* !!!!!!!!!!!!!!! */ \ 0000008A 9100.... LDS R16, (pos + 7) \ 0000008E 9300.... STS (`out` + 8), R16 41 42 index_RW++; \ ??TWI_ISR_7: \ 00000092 E001 LDI R16, 1 \ 00000094 C040 RJMP ??TWI_ISR_5 43 } 44 else if(index_RW < 8) \ ??TWI_ISR_6: \ 00000096 3008 CPI R16, 8 \ 00000098 F448 BRCC ??TWI_ISR_8 45 { 46 pointer = (uint8_t*)(&out) + index_RW; 47 TWDR = *pointer; \ 0000009A E0F0 LDI R31, 0 \ 0000009C 2FE0 MOV R30, R16 \ 0000009E .... SUBI R30, LOW((-(`out`) & 0xFFFF)) \ 000000A0 .... SBCI R31, (-(`out`) & 0xFFFF) >> 8 \ ??TWI_ISR_9: \ 000000A2 8110 LD R17, Z \ 000000A4 9310.... STS _A_TWDR, R17 48 index_RW++; \ ??TWI_ISR_10: \ 000000A8 9503 INC R16 \ 000000AA C035 RJMP ??TWI_ISR_5 49 } 50 else if(index_RW < 12) \ ??TWI_ISR_8: \ 000000AC 300C CPI R16, 12 \ 000000AE F450 BRCC ??TWI_ISR_11 51 { 52 if (index_RW & 1) \ 000000B0 FB00 BST R16, 0 \ 000000B2 F41E BRTC ??TWI_ISR_12 53 { 54 pointer = (uint8_t*)(®.reference) + (index_RW - 55); \ 000000B4 .... LDI R30, LOW((reg + 41)) \ 000000B6 .... LDI R31, HIGH((reg + 41)) \ 000000B8 C002 RJMP ??TWI_ISR_13 55 } 56 else 57 { 58 pointer = (uint8_t*)(®.rawSignal) + (index_RW - 24); \ ??TWI_ISR_12: \ 000000BA .... LDI R30, LOW((reg - 24)) \ 000000BC .... LDI R31, HIGH((reg - 24)) \ ??TWI_ISR_13: \ 000000BE 0FE0 ADD R30, R16 \ 000000C0 1FF5 ADC R31, R21 59 } 60 61 TWDR = *pointer; \ 000000C2 CFEF RJMP ??TWI_ISR_9 62 index_RW++; 63 } 64 else 65 { 66 TWDR = 34; \ ??TWI_ISR_11: \ 000000C4 E212 LDI R17, 34 \ 000000C6 9310.... STS _A_TWDR, R17 \ 000000CA C025 RJMP ??TWI_ISR_5 67 } 68 } 69 else 70 { 71 flagGoTo_IDLE++; 72 } 73 74 break; 75 } 76 77 case 3: { 78 79 if (status_TWI == 70) \ ??TWI_ISR_2: \ 000000CC 3416 CPI R17, 70 \ 000000CE F431 BRNE ??TWI_ISR_14 80 { 81 index_RW = indexADDR; \ 000000D0 9100.... LDS R16, indexADDR 82 TWDR = index_RW; \ 000000D4 9300.... STS _A_TWDR, R16 83 84 stateI2C = 88; \ 000000D8 E518 LDI R17, 88 \ 000000DA C01B RJMP ??TWI_ISR_15 85 } 86 else if(status_TWI == 39) \ ??TWI_ISR_14: \ 000000DC 3217 CPI R17, 39 \ 000000DE F4D9 BRNE ??TWI_ISR_5 87 { 88 89 stateI2C = 55; \ 000000E0 E317 LDI R17, 55 \ 000000E2 C017 RJMP ??TWI_ISR_15 90 } 91 else 92 { 93 } 94 break; 95 } 96 97 case 4: { 98 99 if (status_TWI == 75) \ ??TWI_ISR_3: \ 000000E4 341B CPI R17, 75 \ 000000E6 F4B9 BRNE ??TWI_ISR_5 100 { 101 102 if(index_RW == 0) \ 000000E8 2300 TST R16 \ 000000EA F419 BRNE ??TWI_ISR_16 103 { 104 s&= data_TWI | 45; \ 000000EC 623D ORI R19, 0x2D \ 000000EE 2323 AND R18, R19 105 index_RW++; \ 000000F0 CFD0 RJMP ??TWI_ISR_7 106 } 107 else if (index_RW < 51) \ ??TWI_ISR_16: \ 000000F2 3303 CPI R16, 51 \ 000000F4 F2C8 BRCS ??TWI_ISR_10 108 { 109 index_RW++; 110 } 111 else if (index_RW <= 56) \ 000000F6 3309 CPI R16, 57 \ 000000F8 F470 BRCC ??TWI_ISR_5 112 { 113 pointer = (uint8_t*)(&out) + index_RW; 114 *pointer = data_TWI; \ 000000FA E0F0 LDI R31, 0 \ 000000FC 2FE0 MOV R30, R16 \ 000000FE .... SUBI R30, LOW((-(`out`) & 0xFFFF)) \ 00000100 .... SBCI R31, (-(`out`) & 0xFFFF) >> 8 \ 00000102 8330 ST Z, R19 115 index_RW++; \ 00000104 CFD1 RJMP ??TWI_ISR_10 116 } 117 else 118 { 119 } 120 } 121 else 122 { 123 flagGoTo_IDLE++; 124 } 125 126 break; 127 } 128 129 case 5: { 130 131 if (status_TWI == 67) \ ??TWI_ISR_4: \ 00000106 3413 CPI R17, 67 \ 00000108 F431 BRNE ??TWI_ISR_5 132 { 133 indexADDR = index_RW = data_TWI; \ 0000010A 2F03 MOV R16, R19 \ 0000010C 9330.... STS indexADDR, R19 134 stateI2C = 45; \ 00000110 E21D LDI R17, 45 \ ??TWI_ISR_15: \ 00000112 9310.... STS (indexADDR + 2), R17 135 } 136 break; 137 } 138 } 139 TWCR = 12; \ ??TWI_ISR_5: \ 00000116 E01C LDI R17, 12 \ 00000118 9310.... STS _A_TWCR, R17 140 } while (TWCR_TWINT); \ 0000011C 9110.... LDS R17, _A_TWCR \ 00000120 FD17 SBRC R17, 7 \ 00000122 CF7D RJMP ??TWI_ISR_0 141 out.status = s; \ 00000124 9320.... STS `out`, R18 142 _index_RW=index_RW; \ 00000128 9300.... STS (indexADDR + 1), R16 143 } \ 0000012C BF6F OUT 0x3F, R22 \ 0000012E 9109 LD R16, Y+ \ 00000130 9119 LD R17, Y+ \ 00000132 9129 LD R18, Y+ \ 00000134 9139 LD R19, Y+ \ 00000136 9149 LD R20, Y+ \ 00000138 9159 LD R21, Y+ \ 0000013A 9169 LD R22, Y+ \ 0000013C 91E9 LD R30, Y+ \ 0000013E 91F9 LD R31, Y+ \ 00000140 9518 RETI
Основная идея в том, что статические переменные загрузить в их локальные копии (в начале процедуры), а потом выгрузить (в конце процедуры). В некоторых случаях это лишнее (например, доступ к indexADDR явно не надо так оформлять, у него одно считывание и одна запись, однако, немного поправил в месте записи - indexADDR = index_RW = data_TWI). Стало заметно веселее - компилятор перестал пользоваться регистровой парой X.
--------------------
"Практика выше (теоретического) познания, ибо она имеет не только достоинство всеобщности, но и непосредственной действительности." - В.И. Ленин
|
|
|
|
|
  |
1 чел. читают эту тему (гостей: 1, скрытых пользователей: 0)
Пользователей: 0
|
|
|