<div dir="ltr"><div>We can't answer whether the reassociation pass is good/bad in isolation. Clearly, on this example it's doing harm...but it's probably positive overall? You could disable it and run some benchmarks.<br></div><div><br></div><div>But there are more basic questions raised here. <br></div><div><br></div><div>1. The reassociate pass in combination with instcombine linearizes add/sub integer/FP sequences - forms that have parallelism end up with none. So we will transform as follows:<br></div><div><br></div><div>Name: reassociate minimizes parallelism<br><div> %wx = sub i8 %w, %x<br> %yz = sub i8 %y, %z<br> %r = add i8 %wx, %yz</div><div> =><br></div> %wx = sub i8 %w, %x<br> %wxy = add i8 %wx, %y<br> %r = sub i8 %wxy, %z</div><div><br></div><div><br></div><div>There's a proposal to limit that:</div><div><a href="https://reviews.llvm.org/D65614">https://reviews.llvm.org/D65614</a></div><div><br></div><div>There's also a proposal for a new pass that reassociates to reduce tree-height (increase parallelism):</div><div><a href="https://reviews.llvm.org/D67383">https://reviews.llvm.org/D67383</a></div><div><br></div><div>Neither of those has been updated in a while, so we could reverse-ping them to check status.<br></div><div><br></div><div>2. There's a potential missed canonicalization within instcombine that might serve as a work-around for the motivating problem:</div><div><br></div><div>Name: canonicalize sub before add<br> %a = add i8 %x, %y<br> %r = sub i8 %a, %z<br> =><br> %s = sub i8 %y, %z<br> %r = add i8 %s, %x</div><div><br></div><div><a href="https://rise4fun.com/Alive/VAm">https://rise4fun.com/Alive/VAm</a></div><div><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Nov 11, 2019 at 11:33 AM kamlesh kumar via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">Please ignore above the mail.<div><br></div><div>Hi Devs,<br>I am looking at the bug <br><a href="https://bugs.llvm.org/show_bug.cgi?id=43953" target="_blank">https://bugs.llvm.org/show_bug.cgi?id=43953</a> <br> <br>and found that following piece of ir <br><br> %arrayidx = getelementptr inbounds float, float* %Vec0, i64 %idxprom<br> %0 = load float, float* %arrayidx, align 4, !tbaa !2<br> %arrayidx2 = getelementptr inbounds float, float* %Vec1, i64 %idxprom<br> %1 = load float, float* %arrayidx2, align 4, !tbaa !2<br> %sub = fsub fast float %0, %1<br> %add = fadd fast float %sum.0, %sub<br><br>is transformed into <br><br>%arrayidx = getelementptr inbounds float, float* %Vec0, i64 %idxprom<br> %0 = load float, float* %arrayidx, align 4, !tbaa !2<br> %arrayidx2 = getelementptr inbounds float, float* %Vec1, i64 %idxprom<br> %1 = load float, float* %arrayidx2, align 4, !tbaa !2<br> %.neg = fsub fast float -0.000000e+00, %1<br> %sub = fadd fast float %0, %sum.0<br> %add = fadd fast float %sub, %.neg<br><br>which again by instcombiner transformed into<br> <br> %arrayidx = getelementptr inbounds float, float* %Vec0, i64 %idxprom<br> %0 = load float, float* %arrayidx, align 4, !tbaa !2<br> %arrayidx2 = getelementptr inbounds float, float* %Vec1, i64 %idxprom<br> %1 = load float, float* %arrayidx2, align 4, !tbaa !2<br> %sub = fadd fast float %0, %sum.012<br> %add = fsub fast float %sub, %1<br><br> <br> Now at the time of SLP Vectorization for reported testcase<br> <br> SLP Vectorization visits instructions in bottom up fashion<br> so this instruction is first one to be analyzed<br> %add = fsub fast float %sub, %1<br> <br> When this piece of code is called for the above instruction by HorizontalReduction::matchAssociativeReduction(SLPVectorizer.cpp)<br> /// Checks if the reduction operation can be vectorized.<br> bool isVectorizable(Instruction *I) const {<br> return isVectorizable() && isAssociative(I);<br> }<br> isAssociative returns false for the above instruction ans vectorization fails.<br> If we disable the reassociation the reported testcase is vectorized.<br> Like to community thought on this whether reassociation is doing any good here?<br> <br> ./Kamlesh<br><div><br></div><div><br></div></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Sun, Nov 10, 2019 at 8:03 PM kamlesh kumar <<a href="mailto:kamleshbhalui@gmail.com" target="_blank">kamleshbhalui@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">Hi Devs,<br><div>I am looking at the bug </div><div><a href="https://bugs.llvm.org/show_bug.cgi?id=43953" target="_blank">https://bugs.llvm.org/show_bug.cgi?id=43953</a> </div><div> <br></div><div>and found that following piece of ir </div><div><br></div><div> %arrayidx = getelementptr inbounds float, float* %Vec0, i64 %idxprom<br> %0 = load float, float* %arrayidx, align 4, !tbaa !2<br> %arrayidx2 = getelementptr inbounds float, float* %Vec1, i64 %idxprom<br> %1 = load float, float* %arrayidx2, align 4, !tbaa !2<br> %sub = fsub fast float %0, %1<br> %add = fadd fast float %sum.0, %sub<br></div><div><br></div><div>is transformed into </div><div><br></div><div>%arrayidx = getelementptr inbounds float, float* %Vec0, i64 %idxprom<br> %0 = load float, float* %arrayidx, align 4, !tbaa !2<br> %arrayidx2 = getelementptr inbounds float, float* %Vec1, i64 %idxprom<br> %1 = load float, float* %arrayidx2, align 4, !tbaa !2<br> %.neg = fsub fast float -0.000000e+00, %1<br> %sub = fadd fast float %0, %sum.0<br> %add = fadd fast float %sub, %.neg<br></div><div><br></div><div>which again by instcombiner transformed into</div><div><br></div></div>
</blockquote></div>
_______________________________________________<br>
LLVM Developers mailing list<br>
<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br>
</blockquote></div></div>