-
Notifications
You must be signed in to change notification settings - Fork 299
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
[LowerFirReg] Reimplement the mux reachability analysis #6709
Conversation
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.
I left some minor comments. In general, this iteration seems to be heading in a great direction. Thanks for taking the quick n dirty solution I put together and thinking hard about how to do it both correctly and efficiently.
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 for tackling this! Left some feedback 👍 .
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.
I think the core logic here is good, and certainly better than what we had. Please look through Will's open comments, but I think we can merge this.
1643010
to
950b338
Compare
WIP wip wip Fix lit test Fixup
950b338
to
12a9689
Compare
Thanks for all the feedback. Finally merged this PR and sorry for the delay, I just realized it was still open. |
This implements a new heuristic to determine if a Mux is reachable from a FirReg. In general an operation is reachable from a register if its in the fanout of the register. For FirReg lowering, an if/else structure is required for proper enable inference, if a mux is within the fanout from the register. The fanout path can only consist of MuxOp, ArrayGetOp or ArrayCreateOp. Thus any ops other than MuxOp, ArrayGetOp or ArrayCreateOp block the reachability. The analysis is built lazily when its queried and the result is cached to avoid redundant traversal of the IR. This fixes an issue that caused the merged commit for #6709 to be reverted. The was to explicitly specify the default size of `SmallVector`.
The PR implements a new heuristic to determine if a Mux is reachable from a FirReg. In general an operation is reachable from a register if its in the fanout of the register. For FirReg lowering, an if/else structure is required for proper enable inference, if a mux is within the fanout from the register. The fanout path can only consist of MuxOp, ArrayGetOp or ArrayCreateOp. Thus any ops other than MuxOp, ArrayGetOp or ArrayCreateOp block the reachability. The analysis is built lazily when its queried and the result is cached to avoid redundant traversal of the IR.
This implements a new heuristic to determine if a Mux is reachable from a FirReg. In general an operation is reachable from a register if its in the fanout of the register. For FirReg lowering, an if/else structure is required for proper enable inference, if a mux is within the fanout from the register. The fanout path can only consist of MuxOp, ArrayGetOp or ArrayCreateOp. Thus any ops other than MuxOp, ArrayGetOp or ArrayCreateOp block the reachability. The analysis is built lazily when its queried and the result is cached to avoid redundant traversal of the IR. This fixes an issue that caused the merged commit for llvm#6709 to be reverted. The was to explicitly specify the default size of `SmallVector`.
The PR implements a new heuristic to determine if a Mux is reachable from a FirReg.
In general an operation is reachable from a register if its in the fanout of the register.
For FirReg lowering, an if/else structure is required for proper enable inference, if a mux is within the fanout from the register.
The fanout path can only consist of
MuxOp
,ArrayGetOp
orArrayCreateOp
.Thus any ops other than
MuxOp
,ArrayGetOp
orArrayCreateOp
block the reachability.The analysis is built lazily when its queried and the result is cached to avoid redundant traversal of the IR.