<div dir="ltr"><div>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"><br></div><div dir="ltr">On Fri, Jan 18, 2019 at 7:41 PM Roman Tereshin via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>> wrote:<br></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>
--- llvm/trunk/lib/CodeGen/CodeGenPrepare.cpp (original)<br>
+++ llvm/trunk/lib/CodeGen/CodeGenPrepare.cpp Fri Jan 18 19:37:25 2019<br>
@@ -4664,13 +4664,27 @@ bool CodeGenPrepare::optimizeMemoryInst(<br>
// will look through it and provide only the integer value. In that case,<br>
// use it here.<br>
if (!DL->isNonIntegralPointerType(Addr->getType())) {<br>
+ const auto getResultPtr = [MemoryInst, Addr,<br>
+ &Builder](Value *Reg) -> Value * {<br>
+ BasicBlock *BB = MemoryInst->getParent();<br>
+ for (User *U : Reg->users())<br></blockquote><div><br></div><div>This doesn't seem at all scalable.</div><div><br></div><div>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><br></div><div>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><br></div><div>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> </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>
+ if (I2P->getType() == Addr->getType() && I2P->getParent() == BB) {<br>
+ auto *RegInst = dyn_cast<Instruction>(Reg);<br>
+ if (RegInst && RegInst->getParent() == BB &&<br>
+ !isa<PHINode>(RegInst) && !RegInst->isEHPad())<br>
+ I2P->moveAfter(RegInst);<br>
+ else<br>
+ I2P->moveBefore(*BB, BB->getFirstInsertionPt());<br>
+ return I2P;<br>
+ }<br>
+ return Builder.CreateIntToPtr(Reg, Addr->getType(), "sunkaddr");<br>
+ };<br>
if (!ResultPtr && AddrMode.BaseReg) {<br>
- ResultPtr = Builder.CreateIntToPtr(AddrMode.BaseReg, Addr->getType(),<br>
- "sunkaddr");<br>
+ ResultPtr = getResultPtr(AddrMode.BaseReg);<br>
AddrMode.BaseReg = nullptr;<br>
} else if (!ResultPtr && AddrMode.Scale == 1) {<br>
- ResultPtr = Builder.CreateIntToPtr(AddrMode.ScaledReg, Addr->getType(),<br>
- "sunkaddr");<br>
+ ResultPtr = getResultPtr(AddrMode.ScaledReg);<br>
AddrMode.Scale = 0;<br>
}<br>
}<br>
<br>
Added: llvm/trunk/test/Transforms/CodeGenPrepare/X86/sink-addrmode-cse-inttoptrs.ll<br>
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">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/CodeGenPrepare/X86/sink-addrmode-cse-inttoptrs.ll?rev=351626&view=auto</a><br>
==============================================================================<br>
--- llvm/trunk/test/Transforms/CodeGenPrepare/X86/sink-addrmode-cse-inttoptrs.ll (added)<br>
+++ llvm/trunk/test/Transforms/CodeGenPrepare/X86/sink-addrmode-cse-inttoptrs.ll Fri Jan 18 19:37:25 2019<br>
@@ -0,0 +1,140 @@<br>
+; RUN: opt -mtriple=x86_64-- -codegenprepare %s -S -o - | FileCheck %s --check-prefixes=CGP,COMMON<br>
+; RUN: opt -mtriple=x86_64-- -codegenprepare -load-store-vectorizer %s -S -o - | FileCheck %s --check-prefixes=LSV,COMMON<br>
+<br>
+; Make sure CodeGenPrepare doesn't emit multiple inttoptr instructions<br>
+; of the same integer value while sinking address computations, but<br>
+; rather CSEs them on the fly: excessive inttoptr's confuse SCEV<br>
+; into thinking that related pointers have nothing to do with each other.<br>
+;<br>
+; Triggering this problem involves having just right addressing modes,<br>
+; and verifying that the motivating pass (LoadStoreVectorizer) is able<br>
+; to benefit from it - just right LSV-policies. Hence the atypical combination<br>
+; of the target and datalayout / address spaces in this test.<br>
+<br>
+target datalayout = "p1:32:32:32"<br>
+<br>
+@int_typeinfo = global i8 0<br>
+<br>
+define void @test1(i32 %tmp, i32 %off) {<br>
+; COMMON-LABEL: @test1<br>
+; CGP: = inttoptr<br>
+; CGP-NOT: = inttoptr<br>
+; LSV: = load <2 x float><br>
+; LSV: = load <2 x float><br>
+entry:<br>
+ %tmp1 = inttoptr i32 %tmp to float addrspace(1)*<br>
+ %arrayidx.i.7 = getelementptr inbounds float, float addrspace(1)* %tmp1, i32 %off<br>
+ %add20.i.7 = add i32 %off, 1<br>
+ %arrayidx22.i.7 = getelementptr inbounds float, float addrspace(1)* %tmp1, i32 %add20.i.7<br>
+ br label %for.body<br>
+<br>
+for.body:<br>
+ %tmp8 = phi float [ undef, %entry ], [ %tmp62, %for.body ]<br>
+ %tmp28 = load float, float addrspace(1)* %arrayidx.i.7<br>
+ %tmp29 = load float, float addrspace(1)* %arrayidx22.i.7<br>
+ %arrayidx.i321.7 = getelementptr inbounds float, float addrspace(1)* %tmp1, i32 0<br>
+ %tmp43 = load float, float addrspace(1)* %arrayidx.i321.7<br>
+ %arrayidx22.i327.7 = getelementptr inbounds float, float addrspace(1)* %tmp1, i32 1<br>
+ %tmp44 = load float, float addrspace(1)* %arrayidx22.i327.7<br>
+ %tmp62 = tail call fast float @foo(float %tmp8, float %tmp44, float %tmp43, float %tmp29, float %tmp28)<br>
+ br label %for.body<br>
+}<br>
+<br>
+define void @test2(i64 %a, i64 %b, i64 %c) {<br>
+; COMMON-LABEL: @test2<br>
+; CGP: loop:<br>
+; CGP-NEXT: %mul =<br>
+; CGP-NEXT: = inttoptr i64 %mul<br>
+; CGP-NOT: = inttoptr<br>
+; LSV: store <2 x i64><br>
+entry:<br>
+ %mul.neg.i630 = add nsw i64 %a, -16<br>
+ br label %loop<br>
+<br>
+loop:<br>
+ %mul = mul nsw i64 %b, -16<br>
+ %sub.i631 = add nsw i64 %mul.neg.i630, %mul<br>
+ %tmp = inttoptr i64 %sub.i631 to i8*<br>
+ %tmp1 = inttoptr i64 %sub.i631 to i64*<br>
+ store i64 %c, i64* %tmp1, align 16<br>
+ %arrayidx172 = getelementptr inbounds i8, i8* %tmp, i64 8<br>
+ %tmp2 = bitcast i8* %arrayidx172 to i64*<br>
+ store i64 42, i64* %tmp2, align 8<br>
+ br label %loop<br>
+}<br>
+<br>
+define i32 @test3(i64 %a, i64 %b, i64 %c) personality i32 (...)* @__gxx_personality_v0 {<br>
+; COMMON-LABEL: @test3<br>
+; CGP: entry:<br>
+; CGP-NEXT: %mul =<br>
+; CGP: lpad:<br>
+; CGP-NEXT: landingpad<br>
+; CGP-NEXT: cleanup<br>
+; CGP-NEXT: catch<br>
+; CGP-NEXT: = inttoptr i64 %mul<br>
+; CGP-NOT: = inttoptr<br>
+; LSV: store <2 x i64><br>
+entry:<br>
+ %mul = mul nsw i64 %b, -16<br>
+ %mul.neg.i630 = add nsw i64 %a, -16<br>
+ invoke void @might_throw()<br>
+ to label %cont unwind label %lpad<br>
+<br>
+cont:<br>
+ ret i32 0<br>
+<br>
+eh.resume:<br>
+ ret i32 1<br>
+<br>
+catch_int:<br>
+ ret i32 2<br>
+<br>
+lpad:<br>
+ %ehvals = landingpad { i8*, i32 }<br>
+ cleanup<br>
+ catch i8* @int_typeinfo<br>
+ %sub.i631 = add nsw i64 %mul.neg.i630, %mul<br>
+ %tmp = inttoptr i64 %sub.i631 to i8*<br>
+ %tmp1 = inttoptr i64 %sub.i631 to i64*<br>
+ store i64 %c, i64* %tmp1, align 16<br>
+ %arrayidx172 = getelementptr inbounds i8, i8* %tmp, i64 8<br>
+ %tmp2 = bitcast i8* %arrayidx172 to i64*<br>
+ store i64 42, i64* %tmp2, align 8<br>
+ %ehptr = extractvalue { i8*, i32 } %ehvals, 0<br>
+ %ehsel = extractvalue { i8*, i32 } %ehvals, 1<br>
+ call void @cleanup()<br>
+ %int_sel = call i32 @llvm.eh.typeid.for(i8* @int_typeinfo)<br>
+ %int_match = icmp eq i32 %ehsel, %int_sel<br>
+ br i1 %int_match, label %catch_int, label %eh.resume<br>
+}<br>
+<br>
+define void @test4(i64 %a, i64 %b, i64 %c, i64 %d) {<br>
+; COMMON-LABEL: @test4<br>
+; CGP: loop:<br>
+; CGP-NEXT: %ptrval =<br>
+; CGP-NEXT: %val =<br>
+; CGP-NEXT: = inttoptr i64 %ptrval<br>
+; CGP-NOT: = inttoptr<br>
+; LSV: store <2 x i64><br>
+entry:<br>
+ %mul.neg.i630 = add nsw i64 %a, -16<br>
+ br label %loop<br>
+<br>
+loop:<br>
+ %ptrval = phi i64 [ %b, %entry ], [ %d, %loop ]<br>
+ %val = phi i64 [ 22, %entry ], [ 42, %loop ]<br>
+ %sub.i631 = add nsw i64 %mul.neg.i630, %ptrval<br>
+ %tmp = inttoptr i64 %sub.i631 to i8*<br>
+ %tmp1 = inttoptr i64 %sub.i631 to i64*<br>
+ store i64 %c, i64* %tmp1, align 16<br>
+ %arrayidx172 = getelementptr inbounds i8, i8* %tmp, i64 8<br>
+ %tmp2 = bitcast i8* %arrayidx172 to i64*<br>
+ store i64 %val, i64* %tmp2, align 8<br>
+ br label %loop<br>
+}<br>
+<br>
+declare float @foo(float, float, float, float, float)<br>
+declare i32 @__gxx_personality_v0(...)<br>
+declare i32 @llvm.eh.typeid.for(i8*)<br>
+declare void @might_throw()<br>
+declare void @cleanup()<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div></div>