<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="">Hi Chandler,<div class=""><br class=""></div><div class="">Thanks for letting me know, it’s been reverted.</div><div class=""><br class=""></div><div class="">Thanks,</div><div class="">Roman<br class=""><div><br class=""><blockquote type="cite" class=""><div class="">On Jan 30, 2019, at 7:24 PM, Chandler Carruth <<a href="mailto:chandlerc@gmail.com" class="">chandlerc@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" class=""><div class="">We're seeing numerous compile timeouts due to this commit. We don't have a reduced test case here, but I think I see the bug and you can just write a test case that triggers the behavior:</div><div dir="ltr" class=""><br class=""></div><div dir="ltr" class="">On Fri, Jan 18, 2019 at 7:41 PM Roman Tereshin via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" class="">llvm-commits@lists.llvm.org</a>> wrote:<br class=""></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">==============================================================================<br class="">
--- llvm/trunk/lib/CodeGen/CodeGenPrepare.cpp (original)<br class="">
+++ llvm/trunk/lib/CodeGen/CodeGenPrepare.cpp Fri Jan 18 19:37:25 2019<br class="">
@@ -4664,13 +4664,27 @@ bool CodeGenPrepare::optimizeMemoryInst(<br class="">
// will look through it and provide only the integer value. In that case,<br class="">
// use it here.<br class="">
if (!DL->isNonIntegralPointerType(Addr->getType())) {<br class="">
+ const auto getResultPtr = [MemoryInst, Addr,<br class="">
+ &Builder](Value *Reg) -> Value * {<br class="">
+ BasicBlock *BB = MemoryInst->getParent();<br class="">
+ for (User *U : Reg->users())<br class=""></blockquote><div class=""><br class=""></div><div class="">This doesn't seem at all scalable.</div><div class=""><br class=""></div><div class="">For each register we choose to rewrite we scan *all* of the users looking for the cast. If we try to rewrite the same parameter used in many GEPs, we'll walk the user list of the same instruction a quadratic number of times.</div><div class=""><br class=""></div><div class="">Even if we don't hit that case, walking the entire user list every time seems really expensive. The use-def edge graph (what you're walking) is much larger than the number of instructions in the block.</div><div class=""><br class=""></div><div class="">I think this should get reverted for now as I'm pretty sure it can go quadratic, and I'm 100% sure it is causing lots of compile timeouts. We can get a test case if needed, but I think just a better algorithm is really what is needed for this code.</div><div class=""> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+ if (auto *I2P = dyn_cast<IntToPtrInst>(U))<br class="">
+ if (I2P->getType() == Addr->getType() && I2P->getParent() == BB) {<br class="">
+ auto *RegInst = dyn_cast<Instruction>(Reg);<br class="">
+ if (RegInst && RegInst->getParent() == BB &&<br class="">
+ !isa<PHINode>(RegInst) && !RegInst->isEHPad())<br class="">
+ I2P->moveAfter(RegInst);<br class="">
+ else<br class="">
+ I2P->moveBefore(*BB, BB->getFirstInsertionPt());<br class="">
+ return I2P;<br class="">
+ }<br class="">
+ return Builder.CreateIntToPtr(Reg, Addr->getType(), "sunkaddr");<br class="">
+ };<br class="">
if (!ResultPtr && AddrMode.BaseReg) {<br class="">
- ResultPtr = Builder.CreateIntToPtr(AddrMode.BaseReg, Addr->getType(),<br class="">
- "sunkaddr");<br class="">
+ ResultPtr = getResultPtr(AddrMode.BaseReg);<br class="">
AddrMode.BaseReg = nullptr;<br class="">
} else if (!ResultPtr && AddrMode.Scale == 1) {<br class="">
- ResultPtr = Builder.CreateIntToPtr(AddrMode.ScaledReg, Addr->getType(),<br class="">
- "sunkaddr");<br class="">
+ ResultPtr = getResultPtr(AddrMode.ScaledReg);<br class="">
AddrMode.Scale = 0;<br class="">
}<br class="">
}<br class="">
<br class="">
Added: llvm/trunk/test/Transforms/CodeGenPrepare/X86/sink-addrmode-cse-inttoptrs.ll<br class="">
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/CodeGenPrepare/X86/sink-addrmode-cse-inttoptrs.ll?rev=351626&view=auto" rel="noreferrer" target="_blank" class="">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/CodeGenPrepare/X86/sink-addrmode-cse-inttoptrs.ll?rev=351626&view=auto</a><br class="">
==============================================================================<br class="">
--- llvm/trunk/test/Transforms/CodeGenPrepare/X86/sink-addrmode-cse-inttoptrs.ll (added)<br class="">
+++ llvm/trunk/test/Transforms/CodeGenPrepare/X86/sink-addrmode-cse-inttoptrs.ll Fri Jan 18 19:37:25 2019<br class="">
@@ -0,0 +1,140 @@<br class="">
+; RUN: opt -mtriple=x86_64-- -codegenprepare %s -S -o - | FileCheck %s --check-prefixes=CGP,COMMON<br class="">
+; RUN: opt -mtriple=x86_64-- -codegenprepare -load-store-vectorizer %s -S -o - | FileCheck %s --check-prefixes=LSV,COMMON<br class="">
+<br class="">
+; Make sure CodeGenPrepare doesn't emit multiple inttoptr instructions<br class="">
+; of the same integer value while sinking address computations, but<br class="">
+; rather CSEs them on the fly: excessive inttoptr's confuse SCEV<br class="">
+; into thinking that related pointers have nothing to do with each other.<br class="">
+;<br class="">
+; Triggering this problem involves having just right addressing modes,<br class="">
+; and verifying that the motivating pass (LoadStoreVectorizer) is able<br class="">
+; to benefit from it - just right LSV-policies. Hence the atypical combination<br class="">
+; of the target and datalayout / address spaces in this test.<br class="">
+<br class="">
+target datalayout = "p1:32:32:32"<br class="">
+<br class="">
+@int_typeinfo = global i8 0<br class="">
+<br class="">
+define void @test1(i32 %tmp, i32 %off) {<br class="">
+; COMMON-LABEL: @test1<br class="">
+; CGP: = inttoptr<br class="">
+; CGP-NOT: = inttoptr<br class="">
+; LSV: = load <2 x float><br class="">
+; LSV: = load <2 x float><br class="">
+entry:<br class="">
+ %tmp1 = inttoptr i32 %tmp to float addrspace(1)*<br class="">
+ %arrayidx.i.7 = getelementptr inbounds float, float addrspace(1)* %tmp1, i32 %off<br class="">
+ %add20.i.7 = add i32 %off, 1<br class="">
+ %arrayidx22.i.7 = getelementptr inbounds float, float addrspace(1)* %tmp1, i32 %add20.i.7<br class="">
+ br label %for.body<br class="">
+<br class="">
+for.body:<br class="">
+ %tmp8 = phi float [ undef, %entry ], [ %tmp62, %for.body ]<br class="">
+ %tmp28 = load float, float addrspace(1)* %arrayidx.i.7<br class="">
+ %tmp29 = load float, float addrspace(1)* %arrayidx22.i.7<br class="">
+ %arrayidx.i321.7 = getelementptr inbounds float, float addrspace(1)* %tmp1, i32 0<br class="">
+ %tmp43 = load float, float addrspace(1)* %arrayidx.i321.7<br class="">
+ %arrayidx22.i327.7 = getelementptr inbounds float, float addrspace(1)* %tmp1, i32 1<br class="">
+ %tmp44 = load float, float addrspace(1)* %arrayidx22.i327.7<br class="">
+ %tmp62 = tail call fast float @foo(float %tmp8, float %tmp44, float %tmp43, float %tmp29, float %tmp28)<br class="">
+ br label %for.body<br class="">
+}<br class="">
+<br class="">
+define void @test2(i64 %a, i64 %b, i64 %c) {<br class="">
+; COMMON-LABEL: @test2<br class="">
+; CGP: loop:<br class="">
+; CGP-NEXT: %mul =<br class="">
+; CGP-NEXT: = inttoptr i64 %mul<br class="">
+; CGP-NOT: = inttoptr<br class="">
+; LSV: store <2 x i64><br class="">
+entry:<br class="">
+ %mul.neg.i630 = add nsw i64 %a, -16<br class="">
+ br label %loop<br class="">
+<br class="">
+loop:<br class="">
+ %mul = mul nsw i64 %b, -16<br class="">
+ %sub.i631 = add nsw i64 %mul.neg.i630, %mul<br class="">
+ %tmp = inttoptr i64 %sub.i631 to i8*<br class="">
+ %tmp1 = inttoptr i64 %sub.i631 to i64*<br class="">
+ store i64 %c, i64* %tmp1, align 16<br class="">
+ %arrayidx172 = getelementptr inbounds i8, i8* %tmp, i64 8<br class="">
+ %tmp2 = bitcast i8* %arrayidx172 to i64*<br class="">
+ store i64 42, i64* %tmp2, align 8<br class="">
+ br label %loop<br class="">
+}<br class="">
+<br class="">
+define i32 @test3(i64 %a, i64 %b, i64 %c) personality i32 (...)* @__gxx_personality_v0 {<br class="">
+; COMMON-LABEL: @test3<br class="">
+; CGP: entry:<br class="">
+; CGP-NEXT: %mul =<br class="">
+; CGP: lpad:<br class="">
+; CGP-NEXT: landingpad<br class="">
+; CGP-NEXT: cleanup<br class="">
+; CGP-NEXT: catch<br class="">
+; CGP-NEXT: = inttoptr i64 %mul<br class="">
+; CGP-NOT: = inttoptr<br class="">
+; LSV: store <2 x i64><br class="">
+entry:<br class="">
+ %mul = mul nsw i64 %b, -16<br class="">
+ %mul.neg.i630 = add nsw i64 %a, -16<br class="">
+ invoke void @might_throw()<br class="">
+ to label %cont unwind label %lpad<br class="">
+<br class="">
+cont:<br class="">
+ ret i32 0<br class="">
+<br class="">
+eh.resume:<br class="">
+ ret i32 1<br class="">
+<br class="">
+catch_int:<br class="">
+ ret i32 2<br class="">
+<br class="">
+lpad:<br class="">
+ %ehvals = landingpad { i8*, i32 }<br class="">
+ cleanup<br class="">
+ catch i8* @int_typeinfo<br class="">
+ %sub.i631 = add nsw i64 %mul.neg.i630, %mul<br class="">
+ %tmp = inttoptr i64 %sub.i631 to i8*<br class="">
+ %tmp1 = inttoptr i64 %sub.i631 to i64*<br class="">
+ store i64 %c, i64* %tmp1, align 16<br class="">
+ %arrayidx172 = getelementptr inbounds i8, i8* %tmp, i64 8<br class="">
+ %tmp2 = bitcast i8* %arrayidx172 to i64*<br class="">
+ store i64 42, i64* %tmp2, align 8<br class="">
+ %ehptr = extractvalue { i8*, i32 } %ehvals, 0<br class="">
+ %ehsel = extractvalue { i8*, i32 } %ehvals, 1<br class="">
+ call void @cleanup()<br class="">
+ %int_sel = call i32 @llvm.eh.typeid.for(i8* @int_typeinfo)<br class="">
+ %int_match = icmp eq i32 %ehsel, %int_sel<br class="">
+ br i1 %int_match, label %catch_int, label %eh.resume<br class="">
+}<br class="">
+<br class="">
+define void @test4(i64 %a, i64 %b, i64 %c, i64 %d) {<br class="">
+; COMMON-LABEL: @test4<br class="">
+; CGP: loop:<br class="">
+; CGP-NEXT: %ptrval =<br class="">
+; CGP-NEXT: %val =<br class="">
+; CGP-NEXT: = inttoptr i64 %ptrval<br class="">
+; CGP-NOT: = inttoptr<br class="">
+; LSV: store <2 x i64><br class="">
+entry:<br class="">
+ %mul.neg.i630 = add nsw i64 %a, -16<br class="">
+ br label %loop<br class="">
+<br class="">
+loop:<br class="">
+ %ptrval = phi i64 [ %b, %entry ], [ %d, %loop ]<br class="">
+ %val = phi i64 [ 22, %entry ], [ 42, %loop ]<br class="">
+ %sub.i631 = add nsw i64 %mul.neg.i630, %ptrval<br class="">
+ %tmp = inttoptr i64 %sub.i631 to i8*<br class="">
+ %tmp1 = inttoptr i64 %sub.i631 to i64*<br class="">
+ store i64 %c, i64* %tmp1, align 16<br class="">
+ %arrayidx172 = getelementptr inbounds i8, i8* %tmp, i64 8<br class="">
+ %tmp2 = bitcast i8* %arrayidx172 to i64*<br class="">
+ store i64 %val, i64* %tmp2, align 8<br class="">
+ br label %loop<br class="">
+}<br class="">
+<br class="">
+declare float @foo(float, float, float, float, float)<br class="">
+declare i32 @__gxx_personality_v0(...)<br class="">
+declare i32 @llvm.eh.typeid.for(i8*)<br class="">
+declare void @might_throw()<br class="">
+declare void @cleanup()<br class="">
<br class="">
<br class="">
_______________________________________________<br class="">
llvm-commits mailing list<br class="">
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank" class="">llvm-commits@lists.llvm.org</a><br class="">
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank" class="">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br class="">
</blockquote></div></div>
</div></blockquote></div><br class=""></div></body></html>