Page MenuHomePhabricator

[Utils] Clean another duplicated util method.
ClosedPublic

Authored by asbirlea on May 31 2019, 2:21 PM.

Details

Summary

Following the cleanup in D48202, method foldBlockIntoPredecessor has the
same behavior. Replace its uses with MergeBlockIntoPredecessor.
Remove foldBlockIntoPredecessor.

Diff Detail

Repository
rL LLVM

Event Timeline

asbirlea created this revision.May 31 2019, 2:21 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 31 2019, 2:21 PM

Looks like a nice cleanup to me.

Why not remove foldBlockIntoPredecessor here?

asbirlea updated this revision to Diff 202797.Jun 3 2019, 2:03 PM

No reason for not removing the unused method.
Squashed a local NFC commit I had into this one.

asbirlea edited the summary of this revision. (Show Details)Jun 3 2019, 2:03 PM
dmgreen accepted this revision.Jun 3 2019, 2:34 PM

LGTM

This revision is now accepted and ready to land.Jun 3 2019, 2:34 PM
This revision was automatically updated to reflect the committed changes.

This caused a serious compile time regression. https://martin.st/temp/glew-preproc.c used to take 75 s to compile, now it takes 220 s. Repro with clang -target x86_64-w64-mingw32 -c -O3 glew-preproc.c.

It appears the difference is in the way the DT updates are done.
+@kuhar, @NutshellySima: Could you please take a look at the way the updates are done in MergeBlockIntoPredecessor vs (the removed) foldBlockIntoPredecessor? I tried to use a Lazy strategy and flush after doing all the potential merges, but the regression is still there. Can we get the same performance with the DomTreeUpdater here?

Following up: I can match the previous performance if all update inserts are done first. I'll send a patch for this. But perhaps this should be done inside DTU (i.e. sort updates)?