llvm.org GIT mirror llvm / be07815
Merging r332342: ------------------------------------------------------------------------ r332342 | whitequark | 2018-05-15 04:31:07 -0700 (Tue, 15 May 2018) | 23 lines [MergeFunctions] Fix merging of small weak functions When two interposable functions are merged, we cannot replace uses and have to emit calls to a common internal function. However, writeThunk() will not actually emit a thunk if the function is too small. This leaves us in a broken state where mergeTwoFunctions already rewired the functions, but writeThunk doesn't do anything. This patch changes the implementation so that: * writeThunk() does just that. * The direct replacement of calls is moved into mergeTwoFunctions() into the non-interposable case only. * isThunkProfitable() is extracted and will be called for the non-iterposable case always, and in the interposable case only if uses are still left after replacement. This issue has been introduced in https://reviews.llvm.org/D34806, where the code for checking thunk profitability has been moved. Differential Revision: https://reviews.llvm.org/D46804 Reviewed By: whitequark ------------------------------------------------------------------------ git-svn-id: https://llvm.org/svn/llvm-project/llvm/branches/release_60@332662 91177308-0d34-0410-b5e6-96231b3b80d8 Tom Stellard 1 year, 3 months ago
2 changed file(s) with 66 addition(s) and 36 deletion(s). Raw diff Collapse all Expand all
637637 DEBUG(dbgs() << " }\n");
638638 }
639639
640 // Don't merge tiny functions using a thunk, since it can just end up
641 // making the function larger.
642 static bool isThunkProfitable(Function * F) {
643 if (F->size() == 1) {
644 if (F->front().size() <= 2) {
645 DEBUG(dbgs() << "isThunkProfitable: " << F->getName()
646 << " is too small to bother creating a thunk for\n");
647 return false;
648 }
649 }
650 return true;
651 }
652
640653 // Replace G with a simple tail call to bitcast(F). Also (unless
641654 // MergeFunctionsPDI holds) replace direct uses of G with bitcast(F),
642655 // delete G. Under MergeFunctionsPDI, we use G itself for creating
646659 // For better debugability, under MergeFunctionsPDI, we do not modify G's
647660 // call sites to point to F even when within the same translation unit.
648661 void MergeFunctions::writeThunk(Function *F, Function *G) {
649 if (!G->isInterposable() && !MergeFunctionsPDI) {
650 if (G->hasGlobalUnnamedAddr()) {
651 // G might have been a key in our GlobalNumberState, and it's illegal
652 // to replace a key in ValueMap with a non-global.
653 GlobalNumbers.erase(G);
654 // If G's address is not significant, replace it entirely.
655 Constant *BitcastF = ConstantExpr::getBitCast(F, G->getType());
656 G->replaceAllUsesWith(BitcastF);
657 } else {
658 // Redirect direct callers of G to F. (See note on MergeFunctionsPDI
659 // above).
660 replaceDirectCallers(G, F);
661 }
662 }
663
664 // If G was internal then we may have replaced all uses of G with F. If so,
665 // stop here and delete G. There's no need for a thunk. (See note on
666 // MergeFunctionsPDI above).
667 if (G->hasLocalLinkage() && G->use_empty() && !MergeFunctionsPDI) {
668 G->eraseFromParent();
669 return;
670 }
671
672 // Don't merge tiny functions using a thunk, since it can just end up
673 // making the function larger.
674 if (F->size() == 1) {
675 if (F->front().size() <= 2) {
676 DEBUG(dbgs() << "writeThunk: " << F->getName()
677 << " is too small to bother creating a thunk for\n");
678 return;
679 }
680 }
681
682662 BasicBlock *GEntryBlock = nullptr;
683663 std::vector PDIUnrelatedWL;
684664 BasicBlock *BB = nullptr;
753733 if (F->isInterposable()) {
754734 assert(G->isInterposable());
755735
736 if (!isThunkProfitable(F)) {
737 return;
738 }
739
756740 // Make them both thunks to the same internal function.
757741 Function *H = Function::Create(F->getFunctionType(), F->getLinkage(), "",
758742 F->getParent());
769753 F->setAlignment(MaxAlignment);
770754 F->setLinkage(GlobalValue::PrivateLinkage);
771755 ++NumDoubleWeak;
756 ++NumFunctionsMerged;
772757 } else {
758 // For better debugability, under MergeFunctionsPDI, we do not modify G's
759 // call sites to point to F even when within the same translation unit.
760 if (!G->isInterposable() && !MergeFunctionsPDI) {
761 if (G->hasGlobalUnnamedAddr()) {
762 // G might have been a key in our GlobalNumberState, and it's illegal
763 // to replace a key in ValueMap with a non-global.
764 GlobalNumbers.erase(G);
765 // If G's address is not significant, replace it entirely.
766 Constant *BitcastF = ConstantExpr::getBitCast(F, G->getType());
767 G->replaceAllUsesWith(BitcastF);
768 } else {
769 // Redirect direct callers of G to F. (See note on MergeFunctionsPDI
770 // above).
771 replaceDirectCallers(G, F);
772 }
773 }
774
775 // If G was internal then we may have replaced all uses of G with F. If so,
776 // stop here and delete G. There's no need for a thunk. (See note on
777 // MergeFunctionsPDI above).
778 if (G->hasLocalLinkage() && G->use_empty() && !MergeFunctionsPDI) {
779 G->eraseFromParent();
780 ++NumFunctionsMerged;
781 return;
782 }
783
784 if (!isThunkProfitable(F)) {
785 return;
786 }
787
773788 writeThunk(F, G);
774 }
775
776 ++NumFunctionsMerged;
789 ++NumFunctionsMerged;
790 }
777791 }
778792
779793 /// Replace function F by function G.
0 ; RUN: opt -mergefunc -S < %s | FileCheck %s
1
2 ; Weak functions too small for merging to be profitable
3
4 ; CHECK: define weak i32 @foo(i8*, i32)
5 ; CHECK-NEXT: ret i32 %1
6 ; CHECK: define weak i32 @bar(i8*, i32)
7 ; CHECK-NEXT: ret i32 %1
8
9 define weak i32 @foo(i8*, i32) #0 {
10 ret i32 %1
11 }
12
13 define weak i32 @bar(i8*, i32) #0 {
14 ret i32 %1
15 }