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

[Bob Wilson via llvm-commits]

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.

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