<html><head><meta http-equiv="Content-Type" content="text/html; charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class="">I tried the second patch (since you said it is more efficient). That fixes the problem. Can you get it committed soon?<br class=""><div><br class=""><blockquote type="cite" class=""><div class="">On Oct 18, 2017, at 3:46 AM, whitequark <<a href="mailto:whitequark@whitequark.org" class="">whitequark@whitequark.org</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">On 2017-10-17 23:53, Bob Wilson wrote:</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px;" class="">This change is causing an assertion failure for the Swift compiler<br class="">(<a href="https://ci.swift.org/view/swift-master-next/job/oss-swift-incremental-RA-osx-master-next/" class="">https://ci.swift.org/view/swift-master-next/job/oss-swift-incremental-RA-osx-master-next/</a>).<br class="">Assertion failed: (isa<KeySansPointerT>(new_key) && "Invalid RAUW on<br class="">key of ValueMap<>"), function allUsesReplacedWith, file<br class="">/Users/buildnode/jenkins/workspace/oss-swift-incremental-RA-osx-master-next/llvm/include/llvm/IR/ValueMap.h,<br class="">line 275.<br class="">The FunctionComparator utility used in MergeFunctions defines a<br class="">GlobalNumberState that uses a ValueMap to map GlobalValues to<br class="">integers. The ConstantExpr bitcast created here is not a GlobalValue.<br class="">I investigated this and found that the assertion fails when the value<br class="">is one of those new bitcast instructions. As an experiment, I changed<br class="">the ValueMap in the FunctionComparator to operate on Values instead of<br class="">GlobalValues and that fixed the assertion. I’m not sure that is the<br class="">right solution, though.<br class=""></blockquote><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">I don't think this is the right solution. Could you please try one</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">of the attached patches? Either should work but the second is more</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">efficient.</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px;" class="">Unfortunately, I don’t have a small testcase for this.<br class=""><blockquote type="cite" class="">On Oct 15, 2017, at 5:29 AM, whitequark via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" class="">llvm-commits@lists.llvm.org</a>> wrote:<br class="">Author: whitequark<br class="">Date: Sun Oct 15 05:29:01 2017<br class="">New Revision: 315852<br class="">URL: <a href="http://llvm.org/viewvc/llvm-project?rev=315852&view=rev" class="">http://llvm.org/viewvc/llvm-project?rev=315852&view=rev</a><br class="">Log:<br class="">[MergeFunctions] Replace all uses of unnamed_addr functions.<br class="">This reduces code size for constructs like vtables or interrupt<br class="">tables that refer to functions in global initializers.<br class="">Differential Revision: <a href="https://reviews.llvm.org/D34805" class="">https://reviews.llvm.org/D34805</a><br class="">Added:<br class=""> llvm/trunk/test/Transforms/MergeFunc/merge-unnamed-addr-bitcast.ll<br class=""> llvm/trunk/test/Transforms/MergeFunc/merge-unnamed-addr.ll<br class="">Modified:<br class=""> llvm/trunk/lib/Transforms/IPO/MergeFunctions.cpp<br class="">Modified: llvm/trunk/lib/Transforms/IPO/MergeFunctions.cpp<br class="">URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/MergeFunctions.cpp?rev=315852&r1=315851&r2=315852&view=diff" class="">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/MergeFunctions.cpp?rev=315852&r1=315851&r2=315852&view=diff</a><br class="">==============================================================================<br class="">--- llvm/trunk/lib/Transforms/IPO/MergeFunctions.cpp (original)<br class="">+++ llvm/trunk/lib/Transforms/IPO/MergeFunctions.cpp Sun Oct 15 05:29:01 2017<br class="">@@ -628,9 +628,15 @@ void MergeFunctions::filterInstsUnrelate<br class="">// call sites to point to F even when within the same translation unit.<br class="">void MergeFunctions::writeThunk(Function *F, Function *G) {<br class=""> if (!G->isInterposable() && !MergeFunctionsPDI) {<br class="">- // Redirect direct callers of G to F. (See note on MergeFunctionsPDI<br class="">- // above).<br class="">- replaceDirectCallers(G, F);<br class="">+ if (G->hasGlobalUnnamedAddr()) {<br class="">+ // If G's address is not significant, replace it entirely.<br class="">+ Constant *BitcastF = ConstantExpr::getBitCast(F, G->getType());<br class="">+ G->replaceAllUsesWith(BitcastF);<br class="">+ } else {<br class="">+ // Redirect direct callers of G to F. (See note on MergeFunctionsPDI<br class="">+ // above).<br class="">+ replaceDirectCallers(G, F);<br class="">+ }<br class=""> }<br class=""> // If G was internal then we may have replaced all uses of G with F. If so,<br class="">Added: llvm/trunk/test/Transforms/MergeFunc/merge-unnamed-addr-bitcast.ll<br class="">URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/MergeFunc/merge-unnamed-addr-bitcast.ll?rev=315852&view=auto" class="">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/MergeFunc/merge-unnamed-addr-bitcast.ll?rev=315852&view=auto</a><br class="">==============================================================================<br class="">--- llvm/trunk/test/Transforms/MergeFunc/merge-unnamed-addr-bitcast.ll (added)<br class="">+++ llvm/trunk/test/Transforms/MergeFunc/merge-unnamed-addr-bitcast.ll Sun Oct 15 05:29:01 2017<br class="">@@ -0,0 +1,30 @@<br class="">+; RUN: opt -S -mergefunc < %s | FileCheck %s<br class="">+<br class="">+%A = type { i32 }<br class="">+%B = type { i32 }<br class="">+<br class="">+; CHECK-NOT: @b<br class="">+<br class="">+@x = constant { i32 (i32)*, i32 (i32)* }<br class="">+ { i32 (i32)* bitcast (i32 (%A)* @a to i32 (i32)*),<br class="">+ i32 (i32)* bitcast (i32 (%B)* @b to i32 (i32)*) }<br class="">+; CHECK: { i32 (i32)* bitcast (i32 (%A)* @a to i32 (i32)*), i32 (i32)* bitcast (i32 (%A)* @a to i32 (i32)*) }<br class="">+<br class="">+define internal i32 @a(%A) unnamed_addr {<br class="">+ extractvalue %A %0, 0<br class="">+ xor i32 %2, 0<br class="">+ ret i32 %3<br class="">+}<br class="">+<br class="">+define internal i32 @b(%B) unnamed_addr {<br class="">+ extractvalue %B %0, 0<br class="">+ xor i32 %2, 0<br class="">+ ret i32 %3<br class="">+}<br class="">+<br class="">+define i32 @c(i32) {<br class="">+ insertvalue %B undef, i32 %0, 0<br class="">+ call i32 @b(%B %2)<br class="">+; CHECK: call i32 bitcast (i32 (%A)* @a to i32 (%B)*)(%B %2)<br class="">+ ret i32 %3<br class="">+}<br class="">Added: llvm/trunk/test/Transforms/MergeFunc/merge-unnamed-addr.ll<br class="">URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/MergeFunc/merge-unnamed-addr.ll?rev=315852&view=auto" class="">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/MergeFunc/merge-unnamed-addr.ll?rev=315852&view=auto</a><br class="">==============================================================================<br class="">--- llvm/trunk/test/Transforms/MergeFunc/merge-unnamed-addr.ll (added)<br class="">+++ llvm/trunk/test/Transforms/MergeFunc/merge-unnamed-addr.ll Sun Oct 15 05:29:01 2017<br class="">@@ -0,0 +1,18 @@<br class="">+; RUN: opt -S -mergefunc < %s | FileCheck %s<br class="">+<br class="">+; CHECK-NOT: @b<br class="">+<br class="">+@x = constant { i32 (i32)*, i32 (i32)* } { i32 (i32)* @a, i32 (i32)* @b }<br class="">+; CHECK: { i32 (i32)* @a, i32 (i32)* @a }<br class="">+<br class="">+define internal i32 @a(i32 %a) unnamed_addr {<br class="">+ %b = xor i32 %a, 0<br class="">+ %c = xor i32 %b, 0<br class="">+ ret i32 %c<br class="">+}<br class="">+<br class="">+define internal i32 @b(i32 %a) unnamed_addr {<br class="">+ %b = xor i32 %a, 0<br class="">+ %c = xor i32 %b, 0<br class="">+ ret i32 %c<br class="">+}<br class="">_______________________________________________<br class="">llvm-commits mailing list<br class=""><a href="mailto:llvm-commits@lists.llvm.org" class="">llvm-commits@lists.llvm.org</a><br class="">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits<br class=""></blockquote></blockquote><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">--<span class="Apple-converted-space"> </span></span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">whitequark</span><span id="cid:5E0F449F-0BCC-49D4-B748-2F651E96B0B6"><fix-mergefunc-2.patch></span><span id="cid:A15FC665-54CF-4DB8-91C4-82B46440C869"><fix-mergefunc-1.patch></span></div></blockquote></div><br class=""></body></html>