[llvm] r315852 - [MergeFunctions] Replace all uses of unnamed_addr functions.

Wed Oct 18 03:46:38 PDT 2017

On 2017-10-17 23:53, Bob Wilson wrote:
> This change is causing an assertion failure for the Swift compiler
> (https://ci.swift.org/view/swift-master-next/job/oss-swift-incremental-RA-osx-master-next/).
> 
> Assertion failed: (isa<KeySansPointerT>(new_key) && "Invalid RAUW on
> key of ValueMap<>"), function allUsesReplacedWith, file
> /Users/buildnode/jenkins/workspace/oss-swift-incremental-RA-osx-master-next/llvm/include/llvm/IR/ValueMap.h,
> line 275.
> 
> The FunctionComparator utility used in MergeFunctions defines a
> GlobalNumberState that uses a ValueMap to map GlobalValues to
> integers. The ConstantExpr bitcast created here is not a GlobalValue.
> I investigated this and found that the assertion fails when the value
> is one of those new bitcast instructions. As an experiment, I changed
> the ValueMap in the FunctionComparator to operate on Values instead of
> GlobalValues and that fixed the assertion. I’m not sure that is the
> right solution, though.

I don't think this is the right solution. Could you please try one
of the attached patches? Either should work but the second is more
efficient.

> 
> Unfortunately, I don’t have a small testcase for this.
> 
>> On Oct 15, 2017, at 5:29 AM, whitequark via llvm-commits 
>> <llvm-commits at lists.llvm.org> wrote:
>> 
>> Author: whitequark
>> Date: Sun Oct 15 05:29:01 2017
>> New Revision: 315852
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=315852&view=rev
>> Log:
>> [MergeFunctions] Replace all uses of unnamed_addr functions.
>> 
>> This reduces code size for constructs like vtables or interrupt
>> tables that refer to functions in global initializers.
>> 
>> Differential Revision: https://reviews.llvm.org/D34805
>> 
>> Added:
>>    llvm/trunk/test/Transforms/MergeFunc/merge-unnamed-addr-bitcast.ll
>>    llvm/trunk/test/Transforms/MergeFunc/merge-unnamed-addr.ll
>> Modified:
>>    llvm/trunk/lib/Transforms/IPO/MergeFunctions.cpp
>> 
>> Modified: llvm/trunk/lib/Transforms/IPO/MergeFunctions.cpp
>> URL: 
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/MergeFunctions.cpp?rev=315852&r1=315851&r2=315852&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Transforms/IPO/MergeFunctions.cpp (original)
>> +++ llvm/trunk/lib/Transforms/IPO/MergeFunctions.cpp Sun Oct 15 
>> 05:29:01 2017
>> @@ -628,9 +628,15 @@ void MergeFunctions::filterInstsUnrelate
>> // call sites to point to F even when within the same translation 
>> unit.
>> void MergeFunctions::writeThunk(Function *F, Function *G) {
>>   if (!G->isInterposable() && !MergeFunctionsPDI) {
>> -    // Redirect direct callers of G to F. (See note on 
>> MergeFunctionsPDI
>> -    // above).
>> -    replaceDirectCallers(G, F);
>> +    if (G->hasGlobalUnnamedAddr()) {
>> +      // If G's address is not significant, replace it entirely.
>> +      Constant *BitcastF = ConstantExpr::getBitCast(F, G->getType());
>> +      G->replaceAllUsesWith(BitcastF);
>> +    } else {
>> +      // Redirect direct callers of G to F. (See note on 
>> MergeFunctionsPDI
>> +      // above).
>> +      replaceDirectCallers(G, F);
>> +    }
>>   }
>> 
>>   // If G was internal then we may have replaced all uses of G with F. 
>> If so,
>> 
>> Added: 
>> llvm/trunk/test/Transforms/MergeFunc/merge-unnamed-addr-bitcast.ll
>> URL: 
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/MergeFunc/merge-unnamed-addr-bitcast.ll?rev=315852&view=auto
>> ==============================================================================
>> --- llvm/trunk/test/Transforms/MergeFunc/merge-unnamed-addr-bitcast.ll 
>> (added)
>> +++ llvm/trunk/test/Transforms/MergeFunc/merge-unnamed-addr-bitcast.ll 
>> Sun Oct 15 05:29:01 2017
>> @@ -0,0 +1,30 @@
>> +; RUN: opt -S -mergefunc < %s | FileCheck %s
>> +
>> +%A = type { i32 }
>> +%B = type { i32 }
>> +
>> +; CHECK-NOT: @b
>> +
>> + at x = constant { i32 (i32)*, i32 (i32)* }
>> +  { i32 (i32)* bitcast (i32 (%A)* @a to i32 (i32)*),
>> +    i32 (i32)* bitcast (i32 (%B)* @b to i32 (i32)*) }
>> +; CHECK: { i32 (i32)* bitcast (i32 (%A)* @a to i32 (i32)*), i32 
>> (i32)* bitcast (i32 (%A)* @a to i32 (i32)*) }
>> +
>> +define internal i32 @a(%A) unnamed_addr {
>> +  extractvalue %A %0, 0
>> +  xor i32 %2, 0
>> +  ret i32 %3
>> +}
>> +
>> +define internal i32 @b(%B) unnamed_addr {
>> +  extractvalue %B %0, 0
>> +  xor i32 %2, 0
>> +  ret i32 %3
>> +}
>> +
>> +define i32 @c(i32) {
>> +  insertvalue %B undef, i32 %0, 0
>> +  call i32 @b(%B %2)
>> +; CHECK: call i32 bitcast (i32 (%A)* @a to i32 (%B)*)(%B %2)
>> +  ret i32 %3
>> +}
>> 
>> Added: llvm/trunk/test/Transforms/MergeFunc/merge-unnamed-addr.ll
>> URL: 
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/MergeFunc/merge-unnamed-addr.ll?rev=315852&view=auto
>> ==============================================================================
>> --- llvm/trunk/test/Transforms/MergeFunc/merge-unnamed-addr.ll (added)
>> +++ llvm/trunk/test/Transforms/MergeFunc/merge-unnamed-addr.ll Sun Oct 
>> 15 05:29:01 2017
>> @@ -0,0 +1,18 @@
>> +; RUN: opt -S -mergefunc < %s | FileCheck %s
>> +
>> +; CHECK-NOT: @b
>> +
>> + at x = constant { i32 (i32)*, i32 (i32)* } { i32 (i32)* @a, i32 (i32)* 
>> @b }
>> +; CHECK: { i32 (i32)* @a, i32 (i32)* @a }
>> +
>> +define internal i32 @a(i32 %a) unnamed_addr {
>> +  %b = xor i32 %a, 0
>> +  %c = xor i32 %b, 0
>> +  ret i32 %c
>> +}
>> +
>> +define internal i32 @b(i32 %a) unnamed_addr {
>> +  %b = xor i32 %a, 0
>> +  %c = xor i32 %b, 0
>> +  ret i32 %c
>> +}
>> 
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits

-- 
whitequark
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fix-mergefunc-2.patch
Type: text/x-diff
Size: 1256 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171018/153527ee/attachment.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fix-mergefunc-1.patch
Type: text/x-diff
Size: 794 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171018/153527ee/attachment-0001.patch>