Page MenuHomePhabricator

[Dominators] Teach LoopUnswitch to use the incremental API

Authored by kuhar on Jul 17 2017, 6:53 PM.



This patch makes LoopUnswitch use new incremental API for updating dominators.
It also updates SplitCriticalEdge, as it is called in LoopUnswitch.

There doesn't seem to be any noticeable performance difference when bootstrapping clang with this patch.

Diff Detail


Event Timeline

kuhar created this revision.Jul 17 2017, 6:53 PM
kuhar removed a reviewer: dgross.
grosser accepted this revision.Jul 17 2017, 8:31 PM

This looks good to me! Thank you.

1260 ↗(On Diff #107009)

It IS no longer needed, so delete it.

227 ↗(On Diff #107009)

Very nice cleanup, indeed!

This revision is now accepted and ready to land.Jul 17 2017, 8:31 PM
davide edited edge metadata.Jul 17 2017, 9:16 PM

I'm a little fried at the moment, so just some drive-by comments. I wonder if we can make the memory management automatic instead of calling naked delete.

938 ↗(On Diff #107009)

message in the assertion?

939 ↗(On Diff #107009)

instead of using isa<> + cast<> can you use dyn_cast<> maybe ?

224–227 ↗(On Diff #107009)

Can you add a more descriptive comment here?

kuhar updated this revision to Diff 107124.Jul 18 2017, 9:55 AM

Improve comments

939 ↗(On Diff #107009)

Wouldn't that be slower in release builds without assertions?

kuhar updated this revision to Diff 107128.Jul 18 2017, 10:01 AM

Improve LoopUnswitch assertion.

kuhar marked 3 inline comments as done.Jul 18 2017, 10:01 AM
davide accepted this revision.Jul 18 2017, 11:27 AM

The logic is fine. Thanks!

kuhar updated this revision to Diff 109181.Aug 1 2017, 12:29 PM

Use the new batch update API.

This revision was automatically updated to reflect the committed changes.