fabianschuiki / moore

A hardware compiler based on LLHD and CIRCT
http://www.llhd.io
Apache License 2.0
243 stars 31 forks source link

Surround moore.mir.concat with conversion casts to pass values of the type the ops expect #246

Closed maerhart closed 2 years ago

maerhart commented 2 years ago

This should fix (most of) CI.

Once we add even more operations, we should do some kind of lazy conversion cast insertion instead if eagerly surrounding every op.

fabianschuiki commented 2 years ago

That one failing thing in CI is interesting and looks like it is a genuine bug in codegen that wasn't caught with the Rust LLHD codegen before :see_no_evil:. Probably some implicit cast missing that makes the mux types line up properly.

maerhart commented 2 years ago

The problem with the remaining testcase is that there is a replication with a multiple of 0 here. I don't know the precise code location where this goes wrong, but it adds a i1 value to the concatenation instead of a i0. Likely one of the frequent std::cmp::max(size, 1) calls when creating a hw::ConstantOp. It'll be hard to fix that because we don't have support for i0 in CIRCT. Did Rust-LLHD have i0 values, I'm not sure anymore?

fabianschuiki commented 2 years ago

Huh good question. I think the i0 should actually be fixed in CIRCT by now, and it should be possible to construct such constants. It would be good to track this in a separate issue and not block this PR on it :-)

maerhart commented 2 years ago

I think all the support necessary was added to APInt, but CIRCT doesn't allow it yet. See here.