Skip to content

Make verilog indices consistent #3074

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Nov 21, 2025
Merged

Make verilog indices consistent #3074

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
e9426a5
make verilog imap index consistent
jonfowler Nov 7, 2025
0b20a42
some fixes to reversing indices
jonfowler Nov 8, 2025
0dea5e7
foldr fix?
jonfowler Nov 9, 2025
e4ec8ed
update MaxIndex to support more vecLen cases
jonfowler Nov 10, 2025
f3a140a
add changelog
jonfowler Nov 10, 2025
1ee2ebb
update changelog
jonfowler Nov 10, 2025
b0ddf5a
make it clear imap_go is being generated
jonfowler Nov 10, 2025
f0c173f
Refer to issue in chagnelog
martijnbastiaan Nov 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog/2025-11-10T09_31_42+00_00_consistent_verilog_indices
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
CHANGED: Vector functions in Verilog now have indices consistent with the Vec inputs/outputs (before they were reversed). This is convenient when writing design constraints, which can now be referenced consistently like `imap[0],imap[1],..`. See [#3088](https://github.com/clash-lang/clash-compiler/issues/3088).
41 changes: 23 additions & 18 deletions clash-lib/prims/verilog/Clash_Sized_Vector.primitives.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,15 +86,16 @@
// map begin
genvar ~GENSYM[i][1];
~GENERATE
for (~SYM[1]=0; ~SYM[1] < ~LENGTH[~TYPO]; ~SYM[1] = ~SYM[1] + 1) begin : ~GENSYM[map][2]~IF~SIZE[~TYP[1]]~THEN
for (~SYM[1]=0; ~SYM[1] < ~LENGTH[~TYPO]; ~SYM[1] = ~SYM[1] + 1) begin : ~GENSYM[map][2]
localparam ~GENSYM[vec_index][5] = ~MAXINDEX[~TYPO] - ~SYM[1];~IF~SIZE[~TYP[1]]~THEN
wire ~TYPEL[~TYP[1]] ~GENSYM[map_in][3];
assign ~SYM[3] = ~VAR[vec][1][~SYM[1]*~SIZE[~TYPEL[~TYP[1]]]+:~SIZE[~TYPEL[~TYP[1]]]];~ELSE ~FI
assign ~SYM[3] = ~VAR[vec][1][~SYM[5]*~SIZE[~TYPEL[~TYP[1]]]+:~SIZE[~TYPEL[~TYP[1]]]];~ELSE ~FI
~OUTPUTUSAGE[0] ~TYPEL[~TYPO] ~GENSYM[map_out][4];
~INST 0
~OUTPUT <= ~SYM[4]~ ~TYPEL[~TYPO]~
~INPUT <= ~SYM[3]~ ~TYPEL[~TYP[1]]~
~INST
assign ~RESULT[~SYM[1]*~SIZE[~TYPEL[~TYPO]]+:~SIZE[~TYPEL[~TYPO]]] = ~SYM[4];
assign ~RESULT[~SYM[5]*~SIZE[~TYPEL[~TYPO]]+:~SIZE[~TYPEL[~TYPO]]] = ~SYM[4];
end
~ENDGENERATE
// map end
Expand All @@ -108,18 +109,19 @@
genvar ~GENSYM[i][1];
~GENERATE
for (~SYM[1]=0; ~SYM[1] < ~LENGTH[~TYPO]; ~SYM[1] = ~SYM[1] + 1) begin : ~GENSYM[imap][2]
localparam ~GENSYM[vec_index][6] = ~MAXINDEX[~TYPO] - ~SYM[1];
wire [~SIZE[~INDEXTYPE[~LIT[0]]]-1:0] ~GENSYM[map_index][3];~IF~SIZE[~TYP[2]]~THEN
wire ~TYPEL[~TYP[2]] ~GENSYM[map_in][4];
assign ~SYM[4] = ~VAR[vec][2][~SYM[1]*~SIZE[~TYPEL[~TYP[2]]]+:~SIZE[~TYPEL[~TYP[2]]]];~ELSE ~FI
assign ~SYM[4] = ~VAR[vec][2][~SYM[6]*~SIZE[~TYPEL[~TYP[2]]]+:~SIZE[~TYPEL[~TYP[2]]]];~ELSE ~FI
~OUTPUTUSAGE[1] ~TYPEL[~TYPO] ~GENSYM[map_out][5];

assign ~SYM[3] = ~SIZE[~INDEXTYPE[~LIT[0]]]'d~MAXINDEX[~TYPO] - ~SYM[1][0+:~SIZE[~INDEXTYPE[~LIT[0]]]];
assign ~SYM[3] = ~SYM[1][0+:~SIZE[~INDEXTYPE[~LIT[0]]]];
~INST 1
~OUTPUT <= ~SYM[5]~ ~TYPEL[~TYPO]~
~INPUT <= ~SYM[3]~ ~INDEXTYPE[~LIT[0]]~
~INPUT <= ~SYM[4]~ ~TYPEL[~TYP[2]]~
~INST
assign ~RESULT[~SYM[1]*~SIZE[~TYPEL[~TYPO]]+:~SIZE[~TYPEL[~TYPO]]] = ~SYM[5];
assign ~RESULT[~SYM[6]*~SIZE[~TYPEL[~TYPO]]+:~SIZE[~TYPEL[~TYPO]]] = ~SYM[5];
end
~ENDGENERATE
// imap end
Expand All @@ -129,25 +131,26 @@
kind: Declaration
type: 'imap_go :: (Index n -> a -> b) -> Vec m a -> Index n -> Vec m b'
template: |-
// imap begin
// imap_go begin
genvar ~GENSYM[i][1];
~GENERATE
for (~SYM[1]=0; ~SYM[1] < ~LENGTH[~TYPO]; ~SYM[1] = ~SYM[1] + 1) begin : ~GENSYM[imap][2]
for (~SYM[1]=0; ~SYM[1] < ~LENGTH[~TYPO]; ~SYM[1] = ~SYM[1] + 1) begin : ~GENSYM[imap_go][2]
localparam ~GENSYM[vec_index][6] = ~MAXINDEX[~TYPO] - ~SYM[1];
wire ~TYP[2] ~GENSYM[map_index][3];~IF~SIZE[~TYP[1]]~THEN
wire ~TYPEL[~TYP[1]] ~GENSYM[map_in][4];
assign ~SYM[4] = ~VAR[vec][1][~SYM[1]*~SIZE[~TYPEL[~TYP[1]]]+:~SIZE[~TYPEL[~TYP[1]]]];~ELSE ~FI
assign ~SYM[4] = ~VAR[vec][1][~SYM[6]*~SIZE[~TYPEL[~TYP[1]]]+:~SIZE[~TYPEL[~TYP[1]]]];~ELSE ~FI
~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];
Copy link
Contributor Author

@jonfowler jonfowler Nov 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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)

Copy link
Member

@christiaanb christiaanb Nov 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The compiler generates imap_go when you do things like:

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

Copy link
Member

@christiaanb christiaanb Nov 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
]

Copy link
Member

@christiaanb christiaanb Nov 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

@jonfowler jonfowler Nov 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the explanation.

Copy link
Contributor Author

@jonfowler jonfowler Nov 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

~INST 0
~OUTPUT <= ~SYM[5]~ ~TYPEL[~TYPO]~
~INPUT <= ~SYM[3]~ ~TYP[2]~
~INPUT <= ~SYM[4]~ ~TYPEL[~TYP[1]]~
~INST
assign ~RESULT[~SYM[1]*~SIZE[~TYPEL[~TYPO]]+:~SIZE[~TYPEL[~TYPO]]] = ~SYM[5];
assign ~RESULT[~SYM[6]*~SIZE[~TYPEL[~TYPO]]+:~SIZE[~TYPEL[~TYPO]]] = ~SYM[5];
end
~ENDGENERATE
// imap end
// imap_go end
workInfo: Never
- BlackBox:
name: Clash.Sized.Vector.zipWith
Expand All @@ -157,18 +160,19 @@
// zipWith start
genvar ~GENSYM[i][2];
~GENERATE
for (~SYM[2] = 0; ~SYM[2] < ~LENGTH[~TYPO]; ~SYM[2] = ~SYM[2] + 1) begin : ~GENSYM[zipWith][6]~IF~SIZE[~TYP[1]]~THEN
for (~SYM[2] = 0; ~SYM[2] < ~LENGTH[~TYPO]; ~SYM[2] = ~SYM[2] + 1) begin : ~GENSYM[zipWith][6]
localparam ~GENSYM[vec_index][7] = ~MAXINDEX[~TYPO] - ~SYM[2];~IF~SIZE[~TYP[1]]~THEN
wire ~TYPEL[~TYP[1]] ~GENSYM[zipWith_in1][3];
assign ~SYM[3] = ~VAR[vec1][1][~SYM[2]*~SIZE[~TYPEL[~TYP[1]]]+:~SIZE[~TYPEL[~TYP[1]]]];~ELSE ~FI~IF~SIZE[~TYP[2]]~THEN
assign ~SYM[3] = ~VAR[vec1][1][~SYM[7]*~SIZE[~TYPEL[~TYP[1]]]+:~SIZE[~TYPEL[~TYP[1]]]];~ELSE ~FI~IF~SIZE[~TYP[2]]~THEN
wire ~TYPEL[~TYP[2]] ~GENSYM[zipWith_in2][4];
assign ~SYM[4] = ~VAR[vec2][2][~SYM[2]*~SIZE[~TYPEL[~TYP[2]]]+:~SIZE[~TYPEL[~TYP[2]]]];~ELSE ~FI
~OUTPUTUSAGE[0] ~TYPEL[~TYPO] ~SYM[5];
assign ~SYM[4] = ~VAR[vec2][2][~SYM[7]*~SIZE[~TYPEL[~TYP[2]]]+:~SIZE[~TYPEL[~TYP[2]]]];~ELSE ~FI
~OUTPUTUSAGE[0] ~TYPEL[~TYPO] ~GENSYM[zip_out][5];
~INST 0
~OUTPUT <= ~SYM[5]~ ~TYPEL[~TYPO]~
~INPUT <= ~SYM[3]~ ~TYPEL[~TYP[1]]~
~INPUT <= ~SYM[4]~ ~TYPEL[~TYP[2]]~
~INST
assign ~RESULT[~SYM[2]*~SIZE[~TYPEL[~TYPO]]+:~SIZE[~TYPEL[~TYPO]]] = ~SYM[5];
assign ~RESULT[~SYM[7]*~SIZE[~TYPEL[~TYPO]]+:~SIZE[~TYPEL[~TYPO]]] = ~SYM[5];
end
~ENDGENERATE
// zipWith end
Expand All @@ -185,8 +189,9 @@
genvar ~GENSYM[i][3];
~GENERATE
for (~SYM[3]=0; ~SYM[3] < ~LENGTH[~TYP[2]]; ~SYM[3]=~SYM[3]+1) begin : ~GENSYM[foldr][4]~IF~SIZE[~TYP[2]]~THEN
localparam ~GENSYM[vec_index][8] = ~MAXINDEX[~TYP[2]] - ~SYM[3];
wire ~TYPEL[~TYP[2]] ~GENSYM[foldr_in1][5];
assign ~SYM[5] = ~VAR[xs][2][(~LENGTH[~TYP[2]]-1-~SYM[3])*~SIZE[~TYPEL[~TYP[2]]]+:~SIZE[~TYPEL[~TYP[2]]]];~ELSE ~FI
assign ~SYM[5] = ~VAR[xs][2][(~SYM[8])*~SIZE[~TYPEL[~TYP[2]]]+:~SIZE[~TYPEL[~TYP[2]]]];~ELSE ~FI
wire ~TYPO ~GENSYM[foldr_in2][6];
~OUTPUTUSAGE[0] ~TYPO ~GENSYM[foldr_out][7];

Expand Down
2 changes: 2 additions & 0 deletions clash-lib/src/Clash/Netlist/BlackBox/Util.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -897,7 +897,9 @@ renderTag b (Depth e) = return . Text.pack . show . treeDepth $ lineToType b [e]
renderTag b (MaxIndex e) = return . Text.pack . show . vecLen $ lineToType b [e]
where
vecLen (Vector n _) = n-1
vecLen (Void (Just (Vector n _))) = n-1
vecLen (MemBlob n _) = n-1
vecLen (Void (Just (MemBlob n _))) = n-1
vecLen thing =
error $ $(curLoc) ++ "vecLen of a non-vector-like type: " ++ show thing

Expand Down