This removes some duplication from splitCallSite and makes it easier to
add additional code dealing with each predecessor. It also allows us to
split for more than 2 predecessors, although that is not enabled for
now.
Details
Diff Detail
- Repository
- rL LLVM
Event Timeline
Sorry for the late response. Just minor comments.
lib/Transforms/Scalar/CallSiteSplitting.cpp | ||
---|---|---|
112 ↗ | (On Diff #129065) | Why don't we use 2 instead of 3? |
152 ↗ | (On Diff #129065) | Why not use ConditionsTy here ? |
282–283 ↗ | (On Diff #129065) | It appears that this loop can be merged into the above loop (line 251), and then we can remove SplitInfo. |
lib/Transforms/Scalar/CallSiteSplitting.cpp | ||
---|---|---|
112 ↗ | (On Diff #129065) | We potentially record more than 2 conditions now I think and I thought 3 might be better suited to avoid dynamic allocations. I am happy to change it to 2 if you prefer that. |
152 ↗ | (On Diff #129065) | This function does not require knowing the size of the SmallVector, so I thought there is no need to make the type more restrictive. I am happy to change it if you prefer that |
LGTM !
lib/Transforms/Scalar/CallSiteSplitting.cpp | ||
---|---|---|
112 ↗ | (On Diff #129065) | I'm okay with 3, but I don't think this change increase the possibility of recording more than 2 conditions through single predecessors. |
152 ↗ | (On Diff #129065) | As it's used only with ConditionsTy tightly in our current implementation, I may prefer to be consistent with ConditionsTy for now. |
282–283 ↗ | (On Diff #129065) | Thanks for handling this. |