[PATCH] D53271: [MergeFuncs] Improve ordering of equal functions

[Nikita Popov via Phabricator via llvm-commits]

nikic created this revision.
nikic added reviewers: jfb, whitequark.
Herald added a subscriber: llvm-commits.

MergeFunctions currently tries to process strong functions before
weak functions, because weak functions can simply call strong
functions, while a strong/weak function cannot call a weak function
(a backing strong function is needed).

This patch additionally tries to process external functions before
local functions, because we definitely have to keep the external
function, but may be able to drop the local one (and definitely
can if it is also unnamed_addr).

Unfortunately, this exposes an existing bug in the implementation:
The FnTree and FNodesInTree structures can currently go out of
sync in the case where two weak functions are merged, because the
function in FnTree/FNodesInTree is RAUWed. This leaves it behind in
FnTree (this is intended, as it is the strong backing function which
should be used for further merges), while it is replaced in
FNodesInTree (this is not intended).

This is fixed by switching FNodesInTree from using a ValueMap to
using a DenseMap of AssertingVH.

This exposes another minor issue: Currently FNodesInTree is not
cleared after MergeFunctions finishes running. Currently, this is
potentially dangerous (e.g. if something else wants to RAUW a function
with a non-function), but at the very least it is unnecessary/inefficient.
After the change to use AssertingVH it becomes more problematic,
because there are certainly passes that remove functions.

This issue is fixed by clearing FNodesInTree at the end of the pass.


Repository:
  rL LLVM

https://reviews.llvm.org/D53271

Files:
  lib/Transforms/IPO/MergeFunctions.cpp
  test/Transforms/MergeFunc/external-before-local.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D53271.169658.patch
Type: text/x-patch
Size: 4076 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20181015/d797aed0/attachment.bin>