[FIRRTL] Canonicalize away CVT and adjust all patterns which matched cvt #5527
Conversation
Co-authored-by: Fabian Schuiki <fabian@schuiki.ch>
…dev/darthscsi/castcanon
| (AndPrimOp $x, (NativeCodeCall<"$_builder.create<ConstantOp>($0.getLoc(), cast<IntType>($0.getType()), $1.getValue().trunc($1.getValue().getBitWidth()-1).zext(*cast<IntType>($0.getType()).getWidth()))"> $x, $cst)) | ||
| (AndPrimOp $x, (NativeCodeCall<"$_builder.create<ConstantOp>($0.getLoc(), " | ||
| "cast<IntType>($0.getType()), " | ||
| "$1.getValue().sextOrTrunc(cast<IntType>($0.getType()).getWidthOrSentinel()))"> $x, $cst)) |
There was a problem hiding this comment.
🤔 could this pattern work for non-constants (of known width) equally well, and let a different folder create the sextortrunc thing for us?
There was a problem hiding this comment.
(This can be done separately, partly just curious, it's of course more important to fix the issue, thanks!)
There was a problem hiding this comment.
I was playing with a conversion via asSInt and then a width-narrowing pattern.
There was a problem hiding this comment.
We have more robust width-narrowing in comb, and I'm inclined to do more of this stuff in comb where it's less error prone.
darthscsi
changed the title
| def CVTUnSigned : Pat< | ||
| (CvtPrimOp:$old $x), | ||
| (MoveNameHint $old, | ||
| (AsSIntPrimOp | ||
| (CatPrimOp | ||
| (NativeCodeCall<"$_builder.create<ConstantOp>($0.getLoc(), APSInt(APInt()))"> $old), | ||
| $x))), | ||
| [(UIntType $x)]>; |
There was a problem hiding this comment.
This canonicalization effectively removes the need for cvt. Maybe we just boot this from the spec or remove this during parsing and drop the operation.
There was a problem hiding this comment.
CVT primarily exists for inferred widths. It may be convenient in that context (e.g. at chisel emission), but I'm not sure.
closes #5524