Локальные переменные. Опции

[Jenya7]

Прочитал следующее в ARM System Developer's Guide.

ARMv4-based processors can efficiently load and store 8-, 16-, and 32-bit data. However,
most ARM data processing operations are 32-bit only. For this reason, you should use
a 32-bit datatype, int or long, for local variables wherever possible. Avoid using char and
short as local variable types, even if you are manipulating an 8- or 16-bit value. The one
exception is when you want wrap-around to occur.

И переделал все локальные переменные на uint32_t.
Вопрос - как я понимаю это касается и аргументов передаваемых в функцию?

[ViKo]

Прочитал следующее в ARM System Developer's Guide.

И переделал все локальные переменные на uint32_t.
Вопрос - как я понимаю это касается и аргументов передаваемых в функцию?

Да. И возвращаемых значений тоже.
Не касается это лишь управляющих структур, массивов, которые используются в программе. Чтобы место экономить.

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

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

[Jenya7]

Да. И возвращаемых значений тоже.
Не касается это лишь управляющих структур, массивов, которые используются в программе. Чтобы место экономить.

Ну а вот например

Код

uint32_t MessageChecksum(unsigned char *p, uint32_t len)
{
    uint32_t csum = 0;
    while (len)
    {
      csum+=(*p)&0xFF;
      p++;
      len--;
    }
    return csum;
}

тут у меня unsigned char потому что это касается UART я передаю данные 8 бит. или все равно привести к uint32_t?

uint_fastXX_t это если я захочу перенести код на AVR

[ViKo]

тут у меня unsigned char потому что это касается UART я передаю данные 8 бит. или все равно привести к uint32_t?

Конечно, нет. Где нужен char, там должен быть char.
Тем более, вы передаете указатель (32-битовый).

[Jenya7]

спасибо. придется засучить рукава. этож тысячи строк кода надо править.

Сообщение отредактировал Jenya7 - Jan 20 2015, 12:06

[CrimsonPig]

Ну а вот например

Код

uint32_t MessageChecksum(unsigned char *p, uint32_t len)
{
    uint32_t csum = 0;
    while (len)
    {
      csum+=(*p)&0xFF;
      p++;
      len--;
    }
    return csum;
}

тут у меня unsigned char потому что это касается UART я передаю данные 8 бит. или все равно привести к uint32_t?

Если есть ну очень сильное желание, то этот код можно соптимизировать, чтобы он работал с 32-битными значениями. А оно сильно надо ? Этот код критичен к скорости выполнения ?
Да, тогда еще возникнет ряд проблем, если размер массива не будет кратен 4 байтам, придется вводить пляски с бубном. Кроме того, могут возникнуть проблемы с выравниванием всего массива входных данных по границе слова. Если char* автоматически выравнен по границе байта, то uint32* будет выравнен по границе 4-х байтов. Если попытаться вычислить сумму начиная с невыравненного адреса, будут проблемы, в т.ч и с производительностью

Общие замечания:
1. в качестве первого параметра надо использовать const unsigned char*, облегчается жизнь себе и компилятору
2. эта контрольная сумма - дерьмо. Она не обнаруживает перестановку байтов, например. Есть гораздо более надежные методы и довольно простые - adler, fletcher checksum, ну и классические CRC

Сообщение отредактировал CrimsonPig - Jan 20 2015, 12:09

[Jenya7]

Если есть ну очень сильное желание, то этот код можно соптимизировать, чтобы он работал с 32-битными значениями. А оно сильно надо ? Этот код критичен к скорости выполнения ?
Да, тогда еще возникнет ряд проблем, если размер массива не будет кратен 4 байтам, придется вводить пляски с бубном. Кроме того, могут возникнуть проблемы с выравниванием всего массива входных данных по границе слова. Если char* автоматически выравнен по границе байта, то uint32* будет выравнен по границе 4-х байтов. Если попытаться вычислить сумму начиная с невыравненного адреса, будут проблемы, в т.ч и с производительностью

Общие замечания:
1. в качестве первого параметра надо использовать const unsigned char*, облегчается жизнь себе и компилятору
2. эта контрольная сумма - дерьмо. Она не обнаруживает перестановку байтов, например. Есть гораздо более надежные методы и довольно простые - adler, fletcher checksum, ну и классические CRC

это poor man CRC - быстро и просто без заморочек.
а вообще прибор работает от батарейки поэтому скорость любой функции для меня важна - быстро отработал и пошел спать.
а чем const упрощает жизнь кроме того что не дает модифицировать строку?

Сообщение отредактировал Jenya7 - Jan 20 2015, 12:17

[CrimsonPig]

это poor man CRC - быстро и просто без заморочек.
а вообще прибор работает от батарейки поэтому скорость любой функции для меня важна - быстро отработал и пошел спать.
а чем const упрощает жизнь кроме того что не дает модифицировать строку?

1. "Просто и без заморочек" - это очень хорошо... только вот с обнаружением ошибок будет весьма плохо.
2. про батарейку - вопрос спорный "быстро отработал" - не значит "мало съел". "Скорость любой функции" соптимизировать невозможно. Например, после долгого вырывания волос из попы автор таки соптимизирует свой алгоритм для работы с 32-битными числами. При этом размер кода вырастет в 2 раза, скорость выполнения увеличится на 0.1%, в то время как эта функция как вызывалась раз в секунду, так и вызывается. Оно того стоило ? Все равно потребление электричества надо будет мерять. Не факт что оптимизация по скорости уменьшит потребление энергии.
3. если гениальный автор с бодуна в своей функции напишет не val=*p++, а *p++ = val, и не протестирует (а некоторые вещи почти невозможно протестировать), то впоследствии может быть горько и обидно. Для меня использование const - это как личная гигиена.

[Jenya7]

1. "Просто и без заморочек" - это очень хорошо... только вот с обнаружением ошибок будет весьма плохо.
2. про батарейку - вопрос спорный "быстро отработал" - не значит "мало съел". "Скорость любой функции" соптимизировать невозможно. Например, после долгого вырывания волос из попы автор таки соптимизирует свой алгоритм для работы с 32-битными числами. При этом размер кода вырастет в 2 раза, скорость выполнения увеличится на 0.1%, в то время как эта функция как вызывалась раз в секунду, так и вызывается. Оно того стоило ? Все равно потребление электричества надо будет мерять. Не факт что оптимизация по скорости уменьшит потребление энергии.
3. если гениальный автор с бодуна в своей функции напишет не val=*p++, а *p++ = val, и не протестирует (а некоторые вещи почти невозможно протестировать), то впоследствии может быть горько и обидно. Для меня использование const - это как личная гигиена.

We saw in Section 5.2.1 that converting local variables from types char or short to type int increases performance and reduces code size.

обратите внимание на reduces code size.

[CrimsonPig]

обратите внимание на reduces code size.

Книжки надо не только читать, но еще и понимать.
Например, замена uint_8 на uint_32 в этом примере вполне себе сможет уменьшить размер кода и сделать мир лучше:
for(uint8_t i=0; i<100, ++i)
{ [do something] }

Что будем делать тут?

uint32_t CalcCRC(void* apData, uint32_t aNumBytes);

uint8_t buffer[100];
uint32_t crc = CalcCRC(buffer, 64); //-- очень хорошо, правда ?
crc = CalcCRC(buffer+17, 13); //-- а тут ? Весело? как насчет оптимизации CalcCRC для работы с uint32_t* ?

Сообщение отредактировал CrimsonPig - Jan 20 2015, 12:48

[Jenya7]

Книжки надо не только читать, но еще и понимать.
Например, замена uint_8 на uint_32 в этом примере вполне себе сможет уменьшить размер кода и сделать мир лучше:
for(uint8_t i=0; i<100, ++i)
{ [do something] }

Что будем делать тут?

uint32_t CalcCRC(void* apData, uint32_t aNumBytes);

uint8_t buffer[100];
uint32_t crc = CalcCRC(buffer, 64); //-- очень хорошо, правда ?
crc = CalcCRC(buffer+17, 13); //-- а тут ? Весело? как насчет оптимизации CalcCRC для работы с uint32_t* ?

нет извините меня. то что вы передаете в функцию в качестве аргумента это ваша ответственность. на то вы и программист.

[CrimsonPig]

нет извините меня. то что вы передаете в функцию в качестве аргумента это ваша ответственность. на то вы и программист.

А вот и не извиню, такая я свинья если благородный дон вдруг посмотрит на имплементацию нормально написанной функции, типа библиотечной strlen, то он вполне может рассчитывать увидеть там не
while (*p++), а вполне себе развесистый код, который пытается считать длину строки наиболее оптимальным способом с учетом выравнивания данных, учета невыравненных хвостов итп. В общем случае, этот код вообще платформо-зависим для достижения макс. производительности.
При этом strlen не заботит, передают ли ей указатель на выровненные или невыровненные данные.
Опять же, такая оптимизация может быть избыточна. Если всегда работать с правильно выровненными данными, многи вещи становятся проще, правда можно получить exception при попытке поковыряться с байтами в середине буфера

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

тут у меня unsigned char потому что это касается UART я передаю данные 8 бит.

А складываете вы их всегда в массив unsigned char? И берете всегда из массива unsigned char? Эта функция по идее считает CRC чего угодно. Вот и передавать ей надо "указатель на что угодно", т.е void const *, а внутри заводите уже временную переменную типа "указатель на байт" и присваивайте ей переданный указатель с явным приведением типа. Это позволит написать

Код

typedef struct { ...... } message_t;

message_t Message = {.....};
Checksum = MessageChecksum(&Message, sizeof(Message));

избежав явного приведения типа (unsigned char const *)&Message в большинстве точек вызова. А это разгружает исходник, делая его более легкочитаемым.

или все равно привести к uint32_t?

Привести к uint32_t что? Вы передает указатель. От того, на какой тип этот указатель указывает его размер не изменится. А вот если размер ваших сообщений по спецификации протокола не может превышать 255 или 65535 байт, то параметр len имеет смысл делать uint_fast8_t или uint_fast16_t.

uint_fastXX_t это если я захочу перенести код на AVR

Не обязательно AVR. Вы можете захотеть его перенести на 16- 32- или 64-битную архитектуру, которая умеет делать арифметику с числами 8- или 16-битной разрядности (MSP430 или персоналку). И чтобы этот код и на другой архитектуре тоже был оптимальным без перелопачивания тысяч строк кода, имеет смысл сразу писать его грамотно (или хотя бы стараться писать грамотно).

[CrimsonPig]

А складываете вы их всегда в массив unsigned char? И берете всегда из массива unsigned char? Эта функция по идее считает CRC чего угодно. Вот и передавать ей надо "указатель на что угодно", т.е void const *, а внутри заводите уже временную переменную типа "указатель на байт" и присваивайте ей переданный указатель с явным приведением типа. Это позволит написать

Ик... найдите 3 отличия
1. void*
2. const void *
3. void* const
4. const void* const

void Foo(const void* apData), так ведь ?

[AHTOXA]

void Foo(const void* apData), так ведь ?

У Сергея void const *, это то же самое.

[CrimsonPig]

У Сергея void const *, это то же самое.

Хм, действительно, что-то такого вида записи я не встречал...

[Jenya7]

А складываете вы их всегда в массив unsigned char? И берете всегда из массива unsigned char? Эта функция по идее считает CRC чего угодно. Вот и передавать ей надо "указатель на что угодно", т.е void const *, а внутри заводите уже временную переменную типа "указатель на байт" и присваивайте ей переданный указатель с явным приведением типа. Это позволит написать

Код

typedef struct { ...... } message_t;

message_t Message = {.....};
Checksum = MessageChecksum(&Message, sizeof(Message));

избежав явного приведения типа (unsigned char const *)&Message в большинстве точек вызова. А это разгружает исходник, делая его более легкочитаемым.

а struct как определить? typedef struct {unsigned char paket[BUF_SIZE]} message_t; ?

[AHTOXA]

а struct как определить? typedef struct {unsigned char paket[BUF_SIZE]} message_t; ?

Да как вам надо, так и определите.
Суть в том, что если функция подсчёта crc принимает в качестве аргумента указатель на void, то при передаче любого другого указателя в эту функцию к нему не надо применять приведение типа.
Поясню. Пусть у нас есть две функции подсчёта crc:

Код

uint32_t crc1(const void * buf, size_t size);
uint32_t crc2(const uint8_t * buf, size_t size);

И пусть мы хотим посчитать crc следующих объектов:

Код

int intArray[10];
uint8_t byteBuf[20];
struct config_t
{
  uint32_t serial;
  uint32_t baudrate;
}config;

Так вот. Используя первую функцию, мы можем написать:

Код

crc1(intArray, 10*sizeof(int));
crc1(byteBuf, 20);
crc1(&config, sizeof(config));

Для второй функции нам в двух случаях придётся делать приведение типа:

Код

crc2((const uint8_t *)intArray, 10*sizeof(int));
crc2((const uint8_t *)&config, sizeof(config));

Что касаемо добавления const к неизменяемым параметрам. Это очень полезное правило.
Во-первых, это помогает компилятору в плане оптимизации. (Он имеет право не перечитывать такие объекты после вызова функции)
И, во-вторых, это помогает использующему эту функцию программисту быстрее понять логику работы функции. (И не надо думать об этом программисте, как о каком-то абстрактном человеке (и пусть это будет "его ответственность"). В подавляющем большинстве случаев этим программистом будете вы )
Пример: void * memcpy (void * destination, const void * source, size_t num);
(Если вы случайно перепутаете параметры, то есть шанс, что компилятор вас поправит).

[ViKo]

Используя первую функцию, мы можем написать:

Код

crc1(intArray, 10*sizeof(int));
crc1(byteBuf, 20);
crc1(&config, sizeof(config));

Можем написать проще.

Код

crc1(intArray, sizeof(intArray);
crc1(byteBuf, sizeof(byteBuf);
crc1(&config, sizeof(config);

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

а struct как определить? typedef struct {unsigned char paket[BUF_SIZE]} message_t; ?

Нет. Вот почему вы пытаетесь пакет (последовательность байтов) втиснуть внутрь сообщения? Ваше сообщение должно содержать осмысленную информацию, а в нужный момент просто рассматриваться как последобательность байтов:

Код

enum command_t
{
      LOAD,
      SAVE,
      RUN,
      STOP,
      SLAP_NEAREST_INNOCENT_BYSTANDER,
};

__attribute__((__packed__))
typedef struct
{
    uint16_t    Serial_number;
    command_t   Command;
    uint32_t    Data;
} message_t;

void send()
{
     // работаем с осмысленной информацией.
     message_t Message = { 12345, RUN, 54321 };
     Checksum = MessageChecksum(&Message, sizeof(Message));

    {
         // а вот в этом блоке работаем с сообщением как с последовательностью байтов:

         uint8_t const * pPacket = (uint8_t const *)&Message;

         for(uint_fast8_t i = 0; i < sizeof(message_t); ++i)
               UART_send(*pPacket++);

         UART_send(Checksum);
    }
}

[Jenya7]

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

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

а в том же документе не рекомендуют работать с упакованными структурами - влияют на скорость.

Любой инструмент надо применять к месту. Понятное дело, что не нужно любую структуру делать упакованной. Тут мы заводим ее для того, чтобы возложить на компилятор рутинную работу заполнения нужных полей в пакете (в котором поля идут подряд). Если вы будете заполнять их вручную - вы получите слабочитаемую простыню исходника и точно такой же, если не больший, код на выходе.