Improve the determinism of MergeFunctions
authorJF Bastien <jfb@google.com>
Fri, 21 Aug 2015 23:27:24 +0000 (23:27 +0000)
committerJF Bastien <jfb@google.com>
Fri, 21 Aug 2015 23:27:24 +0000 (23:27 +0000)
commit51be368e3fb558e6d678db7c6e9f6840c1cf7aa5
treec537725674e936e74680908ee8a79ac66a8c9008
parentd4fd5e5000048b033182d070c1ebcc1c29a57fa2
Improve the determinism of MergeFunctions

Summary:

Merge functions previously relied on unsigned comparisons of pointer values to
order functions. This caused observable non-determinism in the compiler for
large bitcode programs. Basically, opt -mergefuncs program.bc | md5sum produces
different hashes when run repeatedly on the same machine. Differing output was
observed on three large bitcodes, but it was less frequent on the smallest file.
It is possible that this only manifests on the large inputs, hence remaining
undetected until now.

This patch fixes this by removing (almost, see below) all places where
comparisons between pointers are used to order functions. Most of these changes
are local, but the comparison of global values requires assigning an identifier
to each local in the order it is visited. This is very similar to the way the
comparison function identifies Value*'s defined within a function. Because the
order of visiting the functions and their subparts is deterministic, the
identifiers assigned to the globals will be as well, and the order of functions
will be deterministic.

With these changes, there is no more observed non-determinism. There is also
only minor slowdowns (negligible to 4%) compared to the baseline, which is
likely a result of the fact that global comparisons involve hash lookups and not
just pointer comparisons.

The one caveat so far is that programs containing BlockAddress constants can
still be non-deterministic. It is not clear what the right solution is here. In
particular, even if the global numbers are used to order by function, we still
need a way to order the BasicBlock*'s. Unfortunately, we cannot just bail out
and fail to order the functions or consider them equal, because we require a
total order over functions. Note that programs with BlockAddress constants are
relatively rare, so the impact of leaving this in is minor as long as this pass
is opt-in.

Author: jrkoenig

Reviewers: nlewycky, jfb, dschuff

Subscribers: jevinskie, llvm-commits, chapuni

Differential revision: http://reviews.llvm.org/D12168

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@245762 91177308-0d34-0410-b5e6-96231b3b80d8
lib/Transforms/IPO/MergeFunctions.cpp
test/Transforms/MergeFunc/constant-entire-value.ll [new file with mode: 0644]