Эта функция C написана в плохой форме? - PullRequest
5 голосов
/ 04 декабря 2009
char byte_to_ascii(char value_to_convert, volatile char *converted_value) {

 if (value_to_convert < 10) {
  return (value_to_convert + 48);
 } else {
  char a = value_to_convert / 10;
  double x = fmod((double)value_to_convert, 10.0);
  char b = (char)x;
  a = a + 48;
  b = b + 48;
  *converted_value = a;
  *(converted_value+1) = b;
  return 0;
 }
}

Цель этой функции - взять значение без знака в диапазоне от 0 до 99 и вернуть либо его эквивалент ascii в случае 0-9, либо манипулировать небольшим глобальным массивом символов, на который можно ссылаться из вызывающего кода, следующего за функцией завершение.

Я задаю этот вопрос, потому что два компилятора от одного поставщика интерпретируют этот код по-разному.

Этот код был написан как способ разбить адресные байты, отправленные через RS485, в строки, которые можно легко передать в функцию send-lcd-string.

Этот код написан для архитектуры PIC18 (8-битный UC).

Проблема в том, что бесплатная / ознакомительная версия конкретного компилятора генерирует идеальный код сборки, который работает, хотя и страдает от снижения производительности, но платный и предположительно превосходный компилятор генерирует код более эффективно за счет возможности ссылаться на адреса всех мои байтовые массивы используются для управления графикой на моем ЖК-дисплее.

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

Спасибо.

Ответы [ 11 ]

7 голосов
/ 04 декабря 2009

Я бы определенно не использовал ничего с плавающей запятой на PIC. И я бы не стал использовать какие-либо подразделения. Сколько раз вы видели отправку не-ascii символа на ЖК-дисплей? Можете ли вы сохранить его в памяти ЖК-дисплея, а затем вызвать его по позиции памяти?

Вот как выглядит деление на 10 в моем коде, обратите внимание на 17 циклов, которые необходимо выполнить. Подумайте, сколько времени это займет, и убедитесь, что больше ничего не ждет от этого.

61:                         q = d2 / 10;
 01520  90482E     mov.b [0x001c+10],0x0000
 01522  FB8000     ze 0x0000,0x0000
 01524  2000A2     mov.w #0xa,0x0004
 01526  090011     repeat #17
 01528  D88002     div.uw 0x0000,0x0004
 0152A  984F00     mov.b 0x0000,[0x001c+8]

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

Шахта начинается со строки 223 и адреса памяти 001BC с _ floatsisf, продолжается через несколько дополнительных меток (_fpack, _divsf3 и т. Д.) И заканчивается _funpack, последней строкой 535 и адресом памяти 0042C. Если вы можете обработать (42C-1BC = 0x270 =) 624 байта потерянного места в программе, это здорово, но некоторые чипы имеют всего 2 КБ, и это не вариант.

Вместо плавающей запятой, если это возможно, попробуйте использовать арифметику с фиксированной запятой в базе 2.

Поскольку вы не можете сослаться на все байтовые массивы на вашем ЖК-дисплее, вы проверили, чтобы убедиться, что вы не пытаетесь отправить нулевой (это точный адрес), но он остановлен проверкой кода для конец строки ascii? (это случилось со мной раньше).

4 голосов
/ 04 декабря 2009

Я бы, наверное, написал это как:

char byte_to_ascii(char value_to_convert, volatile char *converted_value)
{
 if (value_to_convert < 10) {
  return value_to_convert + '0';
 } else {
  converted_value[0] = (value_to_convert / 10) + '0';
  converted_value[1] = (value_to_convert % 10) + '0';
  return 0;
 }
}
4 голосов
/ 04 декабря 2009

деление по модулю и целому числу может быть очень и очень дорогим. Я не знаю о вашей конкретной архитектуре, но, думаю, там тоже дорого.

Если вам нужно и то, и другое по делению и по модулю, сделайте одно из них, а другое получите умножением / разницей.

q =p/10;
r = p - q*10;
3 голосов
/ 04 декабря 2009

Это плохая форма для преобразования в плавающее число, вызова fmod и преобразования в целое число вместо простого использования оператора%? Я бы сказал, да. Существуют более удобочитаемые способы замедления программы для удовлетворения некоторых требований к времени, например, спящий цикл for. Неважно, какой компилятор или какая настройка кода ассемблера или что-то еще, это очень запутанный способ контролировать скорость выполнения вашей программы, и я называю это плохой формой.

Если идеальный ассемблерный код означает, что он работает правильно, но он даже медленнее, чем преобразования в числа с плавающей запятой и обратно, тогда используйте целые числа и sleep в цикле for.

Что касается несовершенного кода сборки, в чем проблема? "за счет возможности ссылаться на адреса всех моих байтовых массивов"? Похоже, что тип char * работает в вашем коде, поэтому кажется, что вы можете обращаться ко всем вашим байтовым массивам так, как это позволяет стандарт C. В чем проблема?

2 голосов
/ 04 декабря 2009

Честно говоря, я бы сказал, да ..

Если вы хотите, чтобы b было остатком, либо используйте MOD, либо сверните свои собственные:

char a = value_to_convert / 10;
char b = value_to_convert - (10 * a);

Преобразование в / из чисел с плавающей запятой - это никогда не способ делать что-либо, если только ваши значения не являются поплавками.

Кроме того, я настоятельно рекомендую придерживаться соглашения о явном обращении к вашим типам данных как к «подписанному» или «неподписанному», и оставьте пустой «символ», если на самом деле является символом часть строки). Вы передаете необработанные данные, которые, по моему мнению, должны быть беззнаковыми символами (конечно, при условии, что источник является без знака!). Легко забыть, если что-то должно быть подписано / не подписано, и с пустым символом вы получите всевозможные ошибки переноса.

Большинству 8-битных микросхем требуется множитель вечность (и более чем вечность - деление), поэтому постарайтесь свести их к минимуму.

Надеюсь, это поможет ..

1 голос
/ 05 декабря 2009

Поскольку мы обсуждаем деления на 10 здесь ..

Это мой дубль. Это только простые операции и даже не нужны широкие регистры.

unsigned char divide_by_10 (unsigned char value)
{
  unsigned char q;
  q = (value>>1) + (value>>2);
  q += (q>>4);
  q >>= 3;
  value -= (q<<3)+q+q;
  return q+((value+6)>>4);
}

Cheers, Nils

1 голос
/ 04 декабря 2009

Код, кажется, выполняет две совершенно разные вещи, в зависимости от того, задано ли ему число в диапазоне 0-9 или 10-99. По этой причине я бы сказал, что эта функция написана в плохой форме: я бы разделил вашу функцию на две функции.

0 голосов
/ 20 декабря 2009

Просто чтобы быть невольным, но множественные return операторы из одной и той же функции можно считать плохой формой (MISRA).

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

0 голосов
/ 06 декабря 2009

Да, я считаю, что ваша функция:
char byte&#95;to&#95;ascii(char value&#95;to&#95;convert, volatile char *converted&#95;value) {

if (value&#95;to&#95;convert < 10) {<br> return (value&#95;to&#95;convert + 48);<br> } else {<br></p> <code>char a = value&#95;to&#95;convert / 10;<br> double x = fmod((double)value&#95;to&#95;convert, 10.0);<br> char b = (char)x;<br> a = a + 48;<br> b = b + 48;<br> *converted&#95;value = a;<br> *(converted&#95;value+1) = b;<br> return 0;<br>

}<br> }<br> в плохом состоянии:

Не используйте десятичные числа для символов ASCII, используйте символ, то есть '@' вместо 0x40.
Нет необходимости использовать функцию fmode.

Вот мой пример:
// Предполагая 8-битный октет
char value;<br> char largeValue;<br> value = value&#95;to&#95;convert / 100;<br> value += '0';<br> converted&#95;value[0] = value;<br> largeValue = value&#95;to&#95;convert - value * 100;<br> value = largeValue / 10; value += '0';<br> converted&#95;value[1] = value;<br> largeValue = largeValue - value * 10; value += '0';<br> converted&#95;value[2] = value;<br> converted&#95;value[3] = '\0'; // Null terminator.<br>

Поскольку здесь только 3 цифры, я решил развернуть цикл. Нет веток для прерывания предварительной загрузки инструкций. Нет исключений с плавающей точкой, только целочисленная арифметика.

Если вы ставите пробелы вместо нулей, вы можете попробовать это:
значение = (значение == 0)? '': значение + '0';

0 голосов
/ 04 декабря 2009

PIC не любит делать арифметику указателей.

Как указывает программист Windows, используйте оператор мод (см. Ниже.)

char byte_to_ascii(char value_to_convert, volatile char *converted_value) {

 if (value_to_convert < 10) {
  return (value_to_convert + 48);
 } else {
  char a = value_to_convert / 10;  
  char b = value_TO_convert%10;
  a = a + 48;
  b = b + 48;
  *converted_value = a;
  *(converted_value+1) = b;
  return 0;
 }
}
...