unpack no longer warns

[rowanG077]

FIXED: Clash no longer gives Dubious primitive instantiation warning when using unpack #2386.

unpack will never go through fromIntegral during synthesis.

Fixes: #2386

Still TODO:

• Write a changelog entry (see changelog/README.md)
• Check copyright notices are up to date in edited files
• Let CI run for GHC < 8.8
• unit test

[rowanG077]

The blackbox for unpackCUShort# doesn't get triggered. It uses the generic path. I'm not sure why this is the case.

[rowanG077]

Please run the full test suite. I couldn't get tests to run.

[alex-mckenna]

CUShort is a newtype and was probably just ripped apart by Clash (since it still doesn't properly support casts). For the other types like Int8, they use data (and did when they were just Int and not the new fixed-width primitive types) so they would be hit in the evaluator.

[martijnbastiaan]

I'm not entirely convinced this would pass CI, even after fixing the wrong-name issue it encounters now (see comment). Feel free to use the GitLab runners to run the tests. To safe some resources you could temporarily comment:

clash-compiler/.gitlab-ci.yml

Lines 38 to 57 in 3819cba

	tests-8.6:
	extends: .common-trigger
	variables:
	GHC_VERSION: 8.6.5
	MULTIPLE_HIDDEN: "no"
	tests-8.8:
	extends: .common-trigger
	variables:
	GHC_VERSION: 8.8.4
	tests-8.10:
	extends: .common-trigger
	variables:
	GHC_VERSION: 8.10.7
	tests-9.0:
	extends: .common-trigger
	variables:
	GHC_VERSION: 9.0.2

[martijnbastiaan]

Suggested change

	, ( $(bLit "0000000000000000000000000000000000000000000000000000000110100101")
	, ( 0b0000000000000000000000000000000000000000000000000000000110100101

Shouldn't this just work?

[leonschoorl]

You'd have to do annotate each one with its size: 0b.... :: BitVector n

[martijnbastiaan]

I was hoping type inference would take care of that as they're mentioned in topEntity. Right nvm, it is all packed.

[martijnbastiaan]

Suggested change

	, mach2@Machine{mStack=[],mTerm=Literal (Int8Literal i)} <- whnf eval tcm True (setTerm arg $ stackClear mach)
	, mach2@Machine{mStack=[],mTerm=Literal (Int8Literal i)} <-
	whnf eval tcm True (setTerm arg $ stackClear mach)

Keeps the line limit sensible

[martijnbastiaan]

Hmm, I doubt this is true for VHDL. Aren't Ints typed as signed?

[alex-mckenna]

I was also under the impression int is signed and word is unsigned in netlist / HDL

[martijnbastiaan]

@rowanG077 Do you intend on finishing this PR? :)

[rowanG077]

@martijnbastiaan A yeah. I definitely want to finish this. It totally slipped my mind.