[PATCH] D12168: Improve the determinism of MergeFunctions

[Jason Koenig via llvm-commits]

jrkoenig created this revision.
jrkoenig added reviewers: jfb, dschuff, nlewycky.
jrkoenig added subscribers: chapuni, llvm-commits.

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.

http://reviews.llvm.org/D12168

Files:
  lib/Transforms/IPO/MergeFunctions.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D12168.32622.patch
Type: text/x-patch
Size: 15404 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150819/39ee310a/attachment.bin>