-
Notifications
You must be signed in to change notification settings - Fork 162
prettyBlackBox: Correctly escape [
and ]
#2989
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.
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
base: master
Are you sure you want to change the base?
Conversation
Sadly it appears that we now emit // map begin
genvar i;
generate
for (i=0; i < 3; i = i + 1) begin : map
wire [43:0] map_in;
assign map_in = a1[i*44+:44];
wire [31:0] map_out;
assign map_out = map_in[\0+:32\];
assign c$ds2_app_arg_0[i*32+:32] = map_out;
end
endgenerate
// map end Unfortunately this doesn't manifest in the minimal reproducer above. |
This program is sufficient to reproduce the bad behavior: {-# LANGUAGE DataKinds #-}
module Hi where
import Clash.Prelude
import Prelude ()
topEntity
:: Signal System (Vec 4 (BitVector 40))
-> Signal System (Vec 4 (BitVector 32))
topEntity = f
f
:: Signal System (Vec n (BitVector 40))
-> Signal System (Vec n (BitVector 32))
f x = fmap (fmap truncateB) x
{-# OPAQUE f #-}
{-# ANN topEntity
(Synthesize
{ t_name = "repro"
, t_inputs = [ PortName "inp" ]
, t_output = PortName "outp"
}) #-} Resulting in: module Hi_topEntity_f
( // Inputs
input wire [159:0] x
// Outputs
, output wire [127:0] result
);
// map begin
genvar i;
generate
for (i=0; i < 4; i = i + 1) begin : map
wire [39:0] map_in;
assign map_in = x[i*40+:40];
wire [31:0] map_out;
assign map_out = map_in[\0+:32\];
assign result[i*32+:32] = map_out;
end
endgenerate
// map end
endmodule |
@christiaanb it would be great to hear your thoughts on the intended behavior here. I'm struggling to see how |
81933a0
to
88e6bb7
Compare
I have shamelessly stolen (with a bit of cleanup) the approach taken in https://github.com/clash-lang/clash-compiler/tree/fix-map-head. This indeed appears to solve the issue, as demonstrated by the added tests. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @bgamari, this LGTM. I'll add a changelog entry later and merge.
88e6bb7
to
7e9fc9d
Compare
The way that way that blackboxes of function arguments to higher order blackboxes are handled is somewhat weird. For historical reasons, they are parsed, pretty-printed, later re-parsed again. However, the pretty-printer failed to correctly re-escape previously escaped square brackets, resulting in the generation of invalid HDL, as seen in clash-lang#2809. Fixes clash-lang#2809. Fixes clash-lang#2988.
7e9fc9d
to
f2b20fd
Compare
The pretty-printer previously failed to round-trip, resulting in invalid primitives due to the logic in
Clash.Netlist.BlackBox.mkFunInput
.I have also included some clean-ups and error message improvements that I found useful while debugging this.
Fixes #2988.
Fixes #2809.
TODO