Make verilog indices consistent #3074

jonfowler · 2025-11-07T15:45:44Z

This makes the order of indices when using imap, consistent between the Clash code and generated Verilog code. Currently the generated verilog code uses the reverse index.

Still TODO:

Write a changelog entry (see changelog/README.md)
Do other Vec components (map, zipWith, etc.)

christiaanb · 2025-11-07T15:57:12Z

So the reason for this is that for this line:

clash-compiler/clash-lib/prims/verilog/Clash_Sized_Vector.primitives.yaml

Line 110 in 12169a7

    
                 for (~SYM[1]=0; ~SYM[1] < ~LENGTH[~TYPO]; ~SYM[1] = ~SYM[1] + 1) begin : ~GENSYM[imap][2]

synthesis tools will generate the labels: imap0, imap1, ... which are labels you will use when creating timing/placement contstraints.

With this change, the generated label imap0 will correspond to the Index n value 0, which is passed to the the function consuming the first (index 0) element of the input vector and producing the first vector in the result vector.

Before this change, for e.g. a 4-elem vector, the component that consume index 0 would get the imap3 label.

jonfowler · 2025-11-09T22:22:26Z

I got this error on empty Vec for foldr after 90e9c92

  tests
    shouldwork
      Vector
        VEmpty
          Verilog
            clash (gen):                            FAIL (2.03s)
              Program /__w/clash-compiler/clash-compiler/dist-newstyle/build/x86_64-linux/ghc-9.6.7/clash-ghc-1.9.0/x/clash/build/clash/clash
               failed with error-code 1.
              
              Full invocation:
              
                /__w/clash-compiler/clash-compiler/dist-newstyle/build/x86_64-linux/ghc-9.6.7/clash-ghc-1.9.0/x/clash/build/clash/clash
                 --verilog -i/__w/clash-compiler/clash-compiler/./tests/shouldwork/Vector VEmpty -fclash-hdldir /tmp/clash-test_VEmpty-3555a1d1ca9f996d/hdl -odir /tmp/clash-test_VEmpty-3555a1d1ca9f996d -hidir /tmp/clash-test_VEmpty-3555a1d1ca9f996d -fclash-debug DebugSilent -fclash-ignore-broken-ghcs -Werror=unrecognised-pragmas -DCLASH_OPAQUE=OPAQUE
                
              
              Stderr was:
              
                Not specializing TopEntity: VEmpty.topEntity[8214565720323800434]
                
                <no location info>: error:
                    Clash error call:
                    Clash.Netlist.BlackBox.Util(902): vecLen of a non-vector-like type: Void (Just (Vector 2 (Void (Just (Product "GHC.Tuple.Prim.()" Nothing [])))))
                    CallStack (from HasCallStack):
                      error, called at src/Clash/Netlist/BlackBox/Util.hs:902:7 in clash-lib-1.9.0-inplace:Clash.Netlist.BlackBox.Util

Not sure exactly how to interpret it but maybe this is caused by MaxIndex on the empty Vec (types are confusing though!)? Makes me suspicious I have also broken a bunch of other zero-sized things (which maybe aren't tested?)

jonfowler · 2025-11-10T09:03:36Z

Not sure exactly how to interpret it but maybe this is caused by MaxIndex on the empty Vec (types are confusing though!)? Makes me suspicious I have also broken a bunch of other zero-sized things (which maybe aren't tested?)

Yes ~MAXINDEX just didn't support all the cases that ~LEN does, fixed here: 5b00a73

jonfowler · 2025-11-10T09:41:51Z

synthesis tools will generate the labels: imap0, imap1, ... which are labels you will use when creating timing/placement contstraints.

Just to clarify this, it will generate imap[0], imap[1],.. using array syntax with the indices given by the value of the genvar. This is specified in the hierarchical section of Verilog specification.

jonfowler · 2025-11-10T10:19:16Z

@christiaanb @martijnbastiaan this is ready for review.

It is passing all the tests, it would be good if you could give it a good looking at as I think there are a bunch of edge cases that are not tested (most notably zero-sized Vectors).

jonfowler · 2025-11-10T10:32:47Z

clash-lib/prims/verilog/Clash_Sized_Vector.primitives.yaml

        ~OUTPUTUSAGE[0] ~TYPEL[~TYPO] ~GENSYM[map_out][5];

-        assign ~SYM[3] = ~SIZE[~TYP[2]]'d~MAXINDEX[~TYPO] - ~SYM[1][0+:~SIZE[~TYP[2]]] + ~ARG[2];
+        assign ~SYM[3] = ~SYM[1][0+:~SIZE[~TYP[2]]] + ~ARG[2];


@martijnbastiaan can you check my understanding here,
Is imap_go used when the imap has been partially unrolled, and ~ARG[2] is the offset for how much unrolling has been done.

I guess I am not going to get consistent indices in this case (and I'm kind of surprised this unrolling would happen, as imap is OPAQUE)

The compiler generates imap_go when you do things like:

case imap f xs of Cons y ys -> ...

It is implemented here:

clash-compiler/clash-ghc/src-ghc/Clash/GHC/Evaluator/Primitive.hs

Lines 4198 to 4215 in 12169a7

$(namePat 'Clash.Sized.Vector.imap) -- :: forall n a b . KnownNat n => (Index n -> a -> b) -> Vec n a -> Vec n b

| isSubj

, nTy : aTy : bTy : _ <- tys

, (tyArgs,tyView -> TyConApp vecTcNm _) <- splitFunForallTy ty

, let (tyArgs',_) = splitFunForallTy (Either.rights tyArgs !! 1)

, TyConApp indexTcNm _ <- tyView (Either.rights tyArgs' !! 0)

, Right n <- runExcept (tyNatSize tcm nTy)

, let iLit = mkIndexLit (Either.rights tyArgs' !! 0) nTy n 0

-> reduceWHNF $

mkApps (Prim (PrimInfo "Clash.Sized.Vector.imap_go" (vecImapGoTy vecTcNm indexTcNm) WorkNever SingleResult NoUnfolding))

[Right nTy

,Right nTy

,Right aTy

,Right bTy

,Left (valToTerm (args !! 1))

,Left (valToTerm (args !! 2))

,Left iLit

]

But when you look at the evaluator rule of imap_go

clash-compiler/clash-ghc/src-ghc/Clash/GHC/Evaluator/Primitive.hs

Lines 4217 to 4223 in 12169a7

"Clash.Sized.Vector.imap_go"

| isSubj

, nTy : mTy : aTy : bTy : _ <- tys

, f : xs : (Suspend nArg) : _ <- args

, DC dc vArgs <- xs

, Right n' <- runExcept (tyNatSize tcm nTy)

, Right m <- runExcept (tyNatSize tcm mTy)

You see it is only evaluated when the vector argument is itself a Cons x xs data-constructor.

Thanks for the explanation.

I have just changed the block_name and generated comment to imap_go so it is clear if this path gets taken.

clash-lib/prims/verilog/Clash_Sized_Vector.primitives.yaml

changelog/2025-11-10T09_31_42+00_00_consistent_verilog_indices

jonfowler · 2025-11-10T14:06:05Z

@christiaanb just to say I'm finished with this, so once the CI has finished I think it's ready to go in if you're happy with it.

I have also done some full-design builds internally, and this passes all tests.

jonfowler · 2025-11-17T11:51:28Z

@christiaanb / @martijnbastiaan looks like gitlab CI is stuck (or I don't know whether this needs kicking off manually?)

DigitalBrains1 · 2025-11-17T12:04:45Z

@kloonbot run_ci 593809e

Indeed, GitLab CI needs to be started manually for PR's coming from a fork instead of the clash-compiler repo itself. It can only be triggered by authorised contributors.

martijnbastiaan

Thank you! I'll extend the changelog a little bit with an example to make it more clear what's happening. Other than that I think this is good to go.

Sadly I don't believe this can be back ported to 1.8, as it would break paths/constraints.

jonfowler · 2025-11-18T11:37:50Z

Sadly I don't believe this can be back ported to 1.8, as it would break paths/constraints.

Not a problem - we already have this patched in and then we will just remove that when we get to the latest version.

jonfowler force-pushed the verilog-prims-index-ordering branch 3 times, most recently from fbf8e58 to 8ca68d5 Compare November 7, 2025 18:27

jonfowler changed the title ~~WIP: make verilog indexes consistent~~ Make verilog indices consistent Nov 10, 2025

jonfowler commented Nov 10, 2025

View reviewed changes

christiaanb approved these changes Nov 10, 2025

View reviewed changes

clash-lib/prims/verilog/Clash_Sized_Vector.primitives.yaml Outdated Show resolved Hide resolved

changelog/2025-11-10T09_31_42+00_00_consistent_verilog_indices Outdated Show resolved Hide resolved

jonfowler added 7 commits November 10, 2025 11:04

make verilog imap index consistent

71a31f6

some fixes to reversing indices

eca9c90

foldr fix?

811934c

update MaxIndex to support more vecLen cases

cf89d65

add changelog

b51efa2

update changelog

bf3fc7f

make it clear imap_go is being generated

593809e

jonfowler force-pushed the verilog-prims-index-ordering branch from 950dedd to 593809e Compare November 10, 2025 11:13

martijnbastiaan approved these changes Nov 17, 2025

View reviewed changes

martijnbastiaan mentioned this pull request Nov 21, 2025

Indices for map/imap/zipWith are reversed in generated Verilog #3088

Open

	$(namePat 'Clash.Sized.Vector.imap) -- :: forall n a b . KnownNat n => (Index n -> a -> b) -> Vec n a -> Vec n b
	\| isSubj
	, nTy : aTy : bTy : _ <- tys
	, (tyArgs,tyView -> TyConApp vecTcNm _) <- splitFunForallTy ty
	, let (tyArgs',_) = splitFunForallTy (Either.rights tyArgs !! 1)
	, TyConApp indexTcNm _ <- tyView (Either.rights tyArgs' !! 0)
	, Right n <- runExcept (tyNatSize tcm nTy)
	, let iLit = mkIndexLit (Either.rights tyArgs' !! 0) nTy n 0
	-> reduceWHNF $
	mkApps (Prim (PrimInfo "Clash.Sized.Vector.imap_go" (vecImapGoTy vecTcNm indexTcNm) WorkNever SingleResult NoUnfolding))
	[Right nTy
	,Right nTy
	,Right aTy
	,Right bTy
	,Left (valToTerm (args !! 1))
	,Left (valToTerm (args !! 2))
	,Left iLit
	]

	"Clash.Sized.Vector.imap_go"
	\| isSubj
	, nTy : mTy : aTy : bTy : _ <- tys
	, f : xs : (Suspend nArg) : _ <- args
	, DC dc vArgs <- xs
	, Right n' <- runExcept (tyNatSize tcm nTy)
	, Right m <- runExcept (tyNatSize tcm mTy)

Make verilog indices consistent #3074

Are you sure you want to change the base?

Make verilog indices consistent #3074

Conversation

jonfowler commented Nov 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Still TODO:

Uh oh!

christiaanb commented Nov 7, 2025

Uh oh!

jonfowler commented Nov 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jonfowler commented Nov 10, 2025

Uh oh!

jonfowler commented Nov 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jonfowler commented Nov 10, 2025

Uh oh!

jonfowler Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

christiaanb Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

christiaanb Nov 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

christiaanb Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

jonfowler Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

jonfowler Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jonfowler commented Nov 10, 2025

Uh oh!

jonfowler commented Nov 17, 2025

Uh oh!

DigitalBrains1 commented Nov 17, 2025

Uh oh!

martijnbastiaan left a comment

Choose a reason for hiding this comment

Uh oh!

jonfowler commented Nov 18, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

jonfowler commented Nov 7, 2025 •

edited

Loading

jonfowler commented Nov 9, 2025 •

edited

Loading

jonfowler commented Nov 10, 2025 •

edited

Loading

christiaanb Nov 10, 2025 •

edited

Loading