[Sim] Add cascade erase for print/proc.print format/get_file producer chains#10204
[Sim] Add cascade erase for print/proc.print format/get_file producer chains#10204nanjo712 wants to merge 1 commit into
Conversation
b4784a8 to
e328515
Compare
| void cascadeErasePrint(PrintFormattedOp op, mlir::RewriterBase &rewriter); | ||
| void cascadeErasePrint(PrintFormattedProcOp op, mlir::RewriterBase &rewriter); | ||
|
|
||
| /// TODO: Add explicit cycle detection helper if callers need local validation. |
There was a problem hiding this comment.
I think we need a helper to detect cycle. But it's not quite related to the topic of this PR. So I think I would implement it in a follow-up PR.
| static bool isDeleteCascadable(Operation *op) { | ||
| return isa<FormatLiteralOp, FormatHexOp, FormatOctOp, FormatBinOp, | ||
| FormatScientificOp, FormatFloatOp, FormatGeneralOp, FormatDecOp, | ||
| FormatCharOp, FormatHierPathOp, FormatStringConcatOp, GetFileOp>( | ||
| op); | ||
| } |
There was a problem hiding this comment.
It might be more appropriate to check this using a trait, as it would likely offer better maintainability. I'm wondering whether to add a trait to explicitly express this, or simply use Pure.
|
Could you elaborate why you think applying RegionDCE to the parent operation is a problem? After The cleanup process in
|
My hesitation with RegionDCE is mostly about granularity. The dead ops here are really the formatting / file-helper producer chain rooted at the removed I also liked that the explicit cleanup keeps the transform narrowly scoped: it only removes the formatting / So it is not that I think RegionDCE would be wrong here; I just found the explicit cleanup to be a better fit for the specific subgraph we are invalidating. |
This PR introduces a utility to cascade-delete sim.print and sim.proc.print along with their dependent sim.fmt (and sim.get_file) DAG nodes when they become dead.
This is motivated by lowering sim.print/sim.proc.print without relying on RegionDCE. Since RegionDCE is region-local, but the format dependency chain of a print may cross region boundaries (e.g., through if regions), using it would require walking up to a top-level procedural region (such as sv.always or hw.triggered) to perform cleanup.
I think it will be helpful to #10172.