Page MenuHomePhabricator

[DFAJumpThreading] make update order deterministic
ClosedPublic

Authored by ollef on Jan 31 2022, 2:42 AM.

Details

Summary

We tracked down some non-determinism in compilation output to the
DFAJumpThreading pass. These changes fixed our issue:

  • Make the DefMap type a MapVector to make its iteration order depend on insertion order.
  • Sort the values to be inserted into NewDefs by instruction order to make the insertion order deterministic. Since these values come from iterating over a ValueMap, which doesn't have deterministic iteration order, I couldn't fix this at its source.

Diff Detail

Event Timeline

ollef created this revision.Jan 31 2022, 2:42 AM
ollef requested review of this revision.Jan 31 2022, 2:42 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 31 2022, 2:42 AM
llvm/lib/Transforms/Scalar/DFAJumpThreading.cpp
1129

Please use SmallVector instead of the STL vector.

1150

Structured binding is a C++17 feature and as far as I know LLVM is still C++14

https://llvm.org/docs/CodingStandards.html#c-standard-versions

ollef updated this revision to Diff 404730.Jan 31 2022, 2:17 PM

Address review comments

  • Use SmallVector instead of std::vector.
  • Stick to C++14 features (no destructuring bindings).
ollef marked 2 inline comments as done.Jan 31 2022, 2:23 PM

Thank you @ollef this LGTM

ollef added a comment.Feb 1 2022, 12:17 AM

There are some seemingly unrelated failures in the Debian build. How do we handle that?

alexey.zhikhar accepted this revision.Feb 1 2022, 6:07 AM
This revision is now accepted and ready to land.Feb 1 2022, 6:07 AM

There are some seemingly unrelated failures in the Debian build. How do we handle that?

I've seen it before, it's unrelated, please ignore

ollef added a comment.Feb 1 2022, 6:16 AM

Thanks!

I don't have commit access. Can you help me get this landed? :)

This revision was landed with ongoing or failed builds.Feb 1 2022, 8:04 AM
This revision was automatically updated to reflect the committed changes.