[andrew_b 14]

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

Почему это так?

Verilog гораздо сильнее, чем VHDL провоцирует на написание всякого говнокода.

[Nick_K 0]

1 hour ago, nice_vladi said:

Разработчики АСИК и ПЛИС действительно так пишут?

Да, действительно так пишут. Я бы сказал, что ещё и побольше и посложнее. Ибо одно из правил проектирования/программирования гласит, что переменные должны иметь осмысленное название. Поэтому встретить переменный с именем s_fsm_state или c_pattrn_decode вполне реально, от чего и код становится запутаннее.

Возможно для обучающей литературы этого не стоит делать, а просто упростить максимально. Но есть авторы-лентяи, которые выдерут кусок рабочего кода и в лучшем случае поменяют название code_loader_long на cl_l.

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

 

10 minutes ago, andrew_b said:

Verilog гораздо сильнее, чем VHDL провоцирует на написание всякого говнокода.

Написание говнокода не зависит от выбранной платформы или языка. Это умение разработчика понятно донести смысл разрабатываемого элемента как для машинного интерпретатора, так и для другого разработчика.

И если сделать грубое усреднение: именно говнокодность измеряет уровень джуна мидла или сеньйора.

[des00 25]

2 hours ago, nice_vladi said:

При виде строк типа 17, 18, 32-36 вообще пропадает всякое желание читать и разгребать это всё.

а что именно в этих строках не так? приоритет полностью прописан скобками, разночтений никаких. Что именно не нравится то?

[lexx 0]

Я бы рекомендовал выделить код вида

fm_state == чем-то => wire read = fm_state == чем-то.

И потом уже его использовать, либо же

parameter READ=что-то

и потом уже сравнивать

fm_state == READ.

А так бывает разное, весь код не переделать. Бывает на разное натыкаешься

[Mad_kvmg 0]

2 hours ago, nice_vladi said:

Разработчики АСИК и ПЛИС действительно так пишут?

Первые два always норм, даже имена есть.  

А вот assign и вправду говнокод. 

Вообще, за тем как описывать FSM это к классику http://www.sunburst-design.com/papers/CummingsSNUG2003SJ_SystemVerilogFSM.pdf 

https://habr.com/ru/post/347928/ 

[nice_vladi 1]

11 minutes ago, des00 said:

а что именно в этих строках не так? приоритет полностью прописан скобками, разночтений никаких. Что именно не нравится то?

Больше двух выражений в условных операторах if-else - нечитабельно;

FSM на два состояния, к тому же не case, а опять же if-else - на мой взгляд криво;

Строки 31-36 - многоэтажный assign - вообще непонятно, что там происходит.

Ну, это все ИМХО. Каждый ...пишет как хочет. Но это ж учебник. Я по нему учить хочу, а не разгребать какую-то невнятицу в коде. Как говорил один программист "Пишите проще" =)

[des00 25]

38 minutes ago, nice_vladi said:

Больше двух выражений в условных операторах if-else - нечитабельно;

FSM на два состояния, к тому же не case, а опять же if-else - на мой взгляд криво;

Это только на ваш взгляд, как по мне, обычный, понятный код. Вот инверсный приоритет условий тут зря, немногие это по коду видят, а так,обычное, классическое описание автомата на 2 состояния.

Quote

Строки 31-36 - многоэтажный assign - вообще непонятно, что там происходит.

Ну, это все ИМХО. Каждый ...пишет как хочет. Но это ж учебник. Я по нему учить хочу, а не разгребать какую-то невнятицу в коде. Как говорил один программист "Пишите проще" =)

Да все просто, ошибка там наборщика. точка с запятой на запятую заменена, слово assign опущено.

ЗЫ. код довольно читаем, просто он не полный. вот и все)

ЗЗЫ. ну либо при сканировании книги у сканера проблемы были или еще как. Я у Яника Берджерона ошибки подобные, который он признавал, так что все ошибаются)

UPD. тут даже ошибки нет, assign можно писать один раз.

  logic  a, b, c;
  logic  d, e, f;

  assign a = c, b = d, c = f;

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

[Mad_kvmg 0]

1 hour ago, des00 said:

 

  logic  a, b, c;
  logic  d, e, f;

  assign a = c, b = d, c = f;

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

Хм, тоже забыл/незнал, но всё равно, IMHO, говнокод получается, если выражение сложнее, чем присвоение одного оператора другому. 

[des00 25]

10 minutes ago, Mad_kvmg said:

MHO, говнокод получается, если выражение сложнее, чем присвоение одного оператора другому. 

это вы говнокода не видели) когда описано непонятно что и непонятно как, с кучей кроссвязей и прочим. А тут, обычный читаемый код. Ну либо у меня инверсия сознания, этот код я пробежал глазами, понял что там написано, во что синтезируется. это заняло где то пару минут времени

[RobFPGA 27]

Приветствую

3 minutes ago, des00 said:

Ну либо у меня инверсия сознания, этот код я пробежал глазами,

Нет - это уже привычка -  у любом gкоде видеть женщину в красном 

Удачи! Rob.

[lexx 0]

34 minutes ago, RobFPGA said:

Нет - это уже привычка -  у любом gкоде видеть женщину в красном 

Хочется видеть...

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

[yaghtn 0]

2 hours ago, des00 said:

а что именно в этих строках не так? приоритет полностью прописан скобками, разночтений никаких. Что именно не нравится то?

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

 

И ещё, блок кода 12-22 делает предположения о списке возможных состояний переменной state. Если сглючит, и state влетит в состояние отличное от sA sB, то блок перестанет работать вплоть до пересброса. Короче, я уверен что после всех case-условий нужно добавлять default

 

Описывать машину как список правил перехода В состояния, или описывать как список перехода ИЗ состояний? Мне ближе второе. При первом подходе подозреваю больше возможностей для ошибок.

 

Бросается в глаза смешивание стилей именования, ВерблюжийСтиль и через_подчеркивание. Это признак говнокода.

Как и отсутствие самодескриптивности кода. Выражения sA не отличается по стилю от inAeq. Что такое sA - переменная, константа? Можно только делать предположения по контексту. Неужто нельзя написать STATE_A ?

 

Таб должен быть 8 пробелов,  тогда слишком сложные конструкции перестают влезать в экран, и просятся к разбиению на строки.

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

 

 

 

[andrew_b 14]

28 минут назад, yaghtn сказал:

Таб должен быть 8

Таба быть вообще не должно. Иначе код у читающего, который не знает длину таба пишущего, вызывает кровь из глаз. Пробелы всегда и везде выглядят одинаково.

[one_eight_seven 3]

57 minutes ago, yaghtn said:

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

С чего это вдруг стало желательно и кем? Priority Routing как раз работает в ситуациях, где множества перекрываются.

[yaghtn 0]

43 minutes ago, one_eight_seven said:

Priority Routing как раз работает

А может быть и нет. А может быть работает wired-or или wand...  Про это нужно знать и вспоминать каждый раз чтобы не писать лишний else перед вторым if?

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

48 minutes ago, one_eight_seven said:

С чего это вдруг стало желательно и кем?

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

Я бы переписал логику первого блока, если не трогать именование, например так:

always_ff@(posedge ck, negedge reset_l) 
    if(~reset_l)
  		state <= sA;
	else begin
		if(state == sA)begin
			if(~go_l)
				state <= sB;
		end
		else begin //if(state==sB)
			if(inAeq)
				state <= sA;
			else //default
				state <= sB;
		end
	end