Page MenuHomePhabricator

Run VerifyDAGDiverence in debug only
ClosedPublic

Authored by mikael on Sep 25 2018, 2:57 AM.

Details

Summary

VerifyDAGDiverence cost compilation time, avoid running it in non-debug
builds.

Diff Detail

Event Timeline

mikael created this revision.Sep 25 2018, 2:57 AM
svenvh added a subscriber: svenvh.Sep 25 2018, 2:59 AM

TargetTransformInfo::hasBranchDivergence() only returns true only if the divergence makes sense for the given target.
So, the compile time should be only affected for such targets: AMDGPU, NVPTX etc.

On which target have you observed compile time increase? How much (% of the whole selection pass) was it affected?

I also initially planned to put the DAG verification under #ifdef NDEBUG.
My change was reviewed by Eli Friedman and he insisted on DAG verification for non-debug builds.
Maybe we should make him know that we changed this?

The compile time increase showed up in our internal GPU target.

efriedma accepted this revision.Sep 25 2018, 12:35 PM
efriedma added a subscriber: efriedma.

It's okay not to run the verifier in Release mode if it's hurting performance. I'm a bit surprised it's actually measurable, but I guess adding a bunch of extra walks over the DAG could add up to a few percent.

LGTM

This revision is now accepted and ready to land.Sep 25 2018, 12:35 PM
alex-t accepted this revision.Sep 25 2018, 12:41 PM
vsk added a subscriber: vsk.Sep 25 2018, 1:24 PM
This revision was automatically updated to reflect the committed changes.