Page MenuHomePhabricator

Please use GitHub pull requests for new patches. Avoid migrating existing patches. Phabricator shutdown timeline

[VPlan] Clone original VPlan and specialize for each VF (WIP).
Changes PlannedPublic

Authored by fhahn on Mar 1 2021, 11:26 AM.



Diff Detail

Event Timeline

fhahn created this revision.Mar 1 2021, 11:26 AM
fhahn requested review of this revision.Mar 1 2021, 11:26 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 1 2021, 11:26 AM
Herald added a subscriber: vkmr. · View Herald Transcript

I didn't recognize that it's still WIP until some point, so some nitpicking comments aren't very useful probably...


Are we somehow limited that the argument must be an OriginalPlan or can it be some Plan? Specifically, what if I want to try some optimization on already cloned VPlan such that I'm not sure if it's profitable or not, so I want to create a clone of the Plan instead of transforming in place. Would I be able to use that clone method?


Why don't we use cast similar to line 8721? It's also not clear why we have early continue above, but it might actually have the same answer.


nit: I'd move it out of the loop.


That's mostly copy of the lambda above. Can we unify them somehow?


I think it deserves a dedicated comment, especially because it's surprising to see new recipes created inside the block of code starting with the comment "Remove dead instrutions."

I think that it would be beneficial to address this even in the WIP stage of the patch and not during the final polishing.


Missed in the copy-paste?

Kazhuu added a subscriber: Kazhuu.Mar 3 2021, 8:42 PM
fhahn updated this revision to Diff 335340.Apr 5 2021, 3:05 PM

Rebased and simplified.

fhahn planned changes to this revision.May 16 2021, 10:49 AM

Putting this on hold for now, to first focus on sinking scalar operands as VPlan-to-VPlan transform: D100258