[FIRRTL] fix missing type conversion in FoldRegMems

[unlsycn]

Fix chipsalliance/chisel#4536

[rwy7]

Hi, the underlying issue seems to be in FoldUnusedBits (introduced in #4652), a different memory canonicalizer, which wasn't correctly handling signed data in a memory. The bitcast op added in this PR hides the problem. We found a different way to produce the error which does not involve the 1-element-memory -> register fold. I have a different fix over here #7913, please take a look!

Thanks for all your help!

[unlsycn]

Hi, the underlying issue seems to be in FoldUnusedBits (introduced in #4652), a different memory canonicalizer, which wasn't correctly handling signed data in a memory. The bitcast op added in this PR hides the problem. We found a different way to produce the error which does not involve the 1-element-memory -> register fold. I have a different fix over here #7913, please take a look!

Thanks for all your help!

Hi, thanks for your explanation! #7913 won't fix chipsalliance/chisel#4536 since the actual lack of type cast in the original issue is that dataPart and nextPart produced by firrtl.bits are unsigned but connect to the signed mem. While #7913 is obviously a nice patch, it doesn't seems to be related to this issue.

[rwy7]

Ugh, thanks, looks like I managed to golf the example to a different issue, sorry. I think you're right. Do you think it makes more sense to cast when applying the mask, right here: https://github.com/llvm/circt/blob/main/lib/Dialect/FIRRTL/FIRRTLFolds.cpp#L3036

Either way, LGTM.