<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class="">I did <span style="color: rgb(245, 245, 245); font-family: Monaco; background-color: rgb(0, 0, 0);" class="">$ cat nary-add.ll</span> and looked at the last four cases in the files, which are OK. Out of luck you took example on the bad cases…<div class=""><br class=""></div><div class="">— </div><div class="">Mehdi</div><div class=""><br class=""></div><div class=""><br class=""><div class=""><div class=""><div><blockquote type="cite" class=""><div class="">On Sep 15, 2015, at 11:13 AM, Marcello Maggioni <<a href="mailto:mmaggioni@apple.com" class="">mmaggioni@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><meta http-equiv="Content-Type" content="text/html charset=utf-8" class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class="">Isn’t this test in “nary-add.ll” doing exactly the same ?<div class=""><br class=""></div><div class=""><div class="">define void @left_reassociate(i32 %a, i32 %b, i32 %c) {</div><div class="">; CHECK-LABEL: @left_reassociate(</div><div class=""> %1 = add i32 %a, %c</div><div class="">; CHECK: [[BASE:%[a-zA-Z0-9]+]] = add i32 %a, %c</div><div class=""> call void @foo(i32 %1)</div><div class=""> %2 = add i32 %b, %c</div><div class=""> %3 = add i32 %a, %2</div><div class="">; CHECK: add i32 [[BASE]], %b</div><div class=""> call void @foo(i32 %3)</div><div class=""> ret void</div><div class="">}</div><div class=""><br class=""></div><div class="">Or maybe I missed the point of the issue you were reporting?</div><div class=""><br class=""></div><div class="">Marcello</div><div class=""><blockquote type="cite" class=""><div class="">On 15 Sep 2015, at 11:07, Mehdi Amini <<a href="mailto:mehdi.amini@apple.com" class="">mehdi.amini@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><meta http-equiv="Content-Type" content="text/html charset=utf-8" class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><br class=""><div class=""><blockquote type="cite" class=""><div class="">On Sep 15, 2015, at 11:03 AM, Marcello Maggioni <<a href="mailto:mmaggioni@apple.com" class="">mmaggioni@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><meta http-equiv="Content-Type" content="text/html charset=utf-8" class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class="">I think Volkan just used the same template of the tests in the sibling “nary-add.ll” test file.<div class=""><br class=""></div><div class="">If you think the test is not adequate probably all of them should be fixed to a more consistent format?</div></div></div></blockquote><div class=""><br class=""></div><div class="">There is only one other test in this directory (as far as I can see), and it does not suffer from what I’m reporting.</div><div class=""><br class=""></div><div class="">— </div><div class="">Mehdi</div><div class=""><br class=""></div><br class=""><blockquote type="cite" class=""><div class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class="">Volkan, do you have a comment on that?</div><div class=""><br class=""></div><div class="">Marcello<br class=""><div class=""><blockquote type="cite" class=""><div class="">On 15 Sep 2015, at 10:29, Mehdi Amini via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" class="">llvm-commits@lists.llvm.org</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><br class="Apple-interchange-newline">On Sep 15, 2015, at 10:22 AM, Marcello Maggioni via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" class="">llvm-commits@lists.llvm.org</a>> wrote:<br class=""><br class="">Author: mggm<br class="">Date: Tue Sep 15 12:22:52 2015<br class="">New Revision: 247705<br class=""><br class="">URL: <a href="http://llvm.org/viewvc/llvm-project?rev=247705&view=rev" class="">http://llvm.org/viewvc/llvm-project?rev=247705&view=rev</a><br class="">Log:<br class="">[NaryReassociate] Add support for Mul instructions<br class=""><br class="">This patch extends the current pass by handling<br class="">Mul instructions as well.<br class=""><br class="">Patch by: Volkan Keles (<a href="mailto:vkeles@apple.com" class="">vkeles@apple.com</a>)<br class=""><br class="">Added:<br class=""> llvm/trunk/test/Transforms/NaryReassociate/nary-mul.ll<br class="">Modified:<br class=""> llvm/trunk/lib/Transforms/Scalar/NaryReassociate.cpp<br class=""><br class="">Modified: llvm/trunk/lib/Transforms/Scalar/NaryReassociate.cpp<br class="">URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/NaryReassociate.cpp?rev=247705&r1=247704&r2=247705&view=diff" class="">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/NaryReassociate.cpp?rev=247705&r1=247704&r2=247705&view=diff</a><br class="">==============================================================================<br class="">--- llvm/trunk/lib/Transforms/Scalar/NaryReassociate.cpp (original)<br class="">+++ llvm/trunk/lib/Transforms/Scalar/NaryReassociate.cpp Tue Sep 15 12:22:52 2015<br class="">@@ -71,8 +71,8 @@<br class="">//<br class="">// Limitations and TODO items:<br class="">//<br class="">-// 1) We only considers n-ary adds for now. This should be extended and<br class="">-// generalized.<br class="">+// 1) We only considers n-ary adds and muls for now. This should be extended<br class="">+// and generalized.<br class="">//<br class="">//===----------------------------------------------------------------------===//<br class=""><br class="">@@ -145,12 +145,23 @@ private:<br class=""> unsigned I, Value *LHS,<br class=""> Value *RHS, Type *IndexedType);<br class=""><br class="">- // Reassociate Add for better CSE.<br class="">- Instruction *tryReassociateAdd(BinaryOperator *I);<br class="">- // A helper function for tryReassociateAdd. LHS and RHS are explicitly passed.<br class="">- Instruction *tryReassociateAdd(Value *LHS, Value *RHS, Instruction *I);<br class="">- // Rewrites I to LHS + RHS if LHS is computed already.<br class="">- Instruction *tryReassociatedAdd(const SCEV *LHS, Value *RHS, Instruction *I);<br class="">+ // Reassociate binary operators for better CSE.<br class="">+ Instruction *tryReassociateBinaryOp(BinaryOperator *I);<br class="">+<br class="">+ // A helper function for tryReassociateBinaryOp. LHS and RHS are explicitly<br class="">+ // passed.<br class="">+ Instruction *tryReassociateBinaryOp(Value *LHS, Value *RHS,<br class="">+ BinaryOperator *I);<br class="">+ // Rewrites I to (LHS op RHS) if LHS is computed already.<br class="">+ Instruction *tryReassociatedBinaryOp(const SCEV *LHS, Value *RHS,<br class="">+ BinaryOperator *I);<br class="">+<br class="">+ // Tries to match Op1 and Op2 by using V.<br class="">+ bool matchTernaryOp(BinaryOperator *I, Value *V, Value *&Op1, Value *&Op2);<br class="">+<br class="">+ // Gets SCEV for (LHS op RHS).<br class="">+ const SCEV *getBinarySCEV(BinaryOperator *I, const SCEV *LHS,<br class="">+ const SCEV *RHS);<br class=""><br class=""> // Returns the closest dominator of \c Dominatee that computes<br class=""> // \c CandidateExpr. Returns null if not found.<br class="">@@ -219,6 +230,7 @@ static bool isPotentiallyNaryReassociabl<br class=""> switch (I->getOpcode()) {<br class=""> case Instruction::Add:<br class=""> case Instruction::GetElementPtr:<br class="">+ case Instruction::Mul:<br class=""> return true;<br class=""> default:<br class=""> return false;<br class="">@@ -276,7 +288,8 @@ bool NaryReassociate::doOneIteration(Fun<br class="">Instruction *NaryReassociate::tryReassociate(Instruction *I) {<br class=""> switch (I->getOpcode()) {<br class=""> case Instruction::Add:<br class="">- return tryReassociateAdd(cast<BinaryOperator>(I));<br class="">+ case Instruction::Mul:<br class="">+ return tryReassociateBinaryOp(cast<BinaryOperator>(I));<br class=""> case Instruction::GetElementPtr:<br class=""> return tryReassociateGEP(cast<GetElementPtrInst>(I));<br class=""> default:<br class="">@@ -453,49 +466,89 @@ GetElementPtrInst *NaryReassociate::tryR<br class=""> return NewGEP;<br class="">}<br class=""><br class="">-Instruction *NaryReassociate::tryReassociateAdd(BinaryOperator *I) {<br class="">+Instruction *NaryReassociate::tryReassociateBinaryOp(BinaryOperator *I) {<br class=""> Value *LHS = I->getOperand(0), *RHS = I->getOperand(1);<br class="">- if (auto *NewI = tryReassociateAdd(LHS, RHS, I))<br class="">+ if (auto *NewI = tryReassociateBinaryOp(LHS, RHS, I))<br class=""> return NewI;<br class="">- if (auto *NewI = tryReassociateAdd(RHS, LHS, I))<br class="">+ if (auto *NewI = tryReassociateBinaryOp(RHS, LHS, I))<br class=""> return NewI;<br class=""> return nullptr;<br class="">}<br class=""><br class="">-Instruction *NaryReassociate::tryReassociateAdd(Value *LHS, Value *RHS,<br class="">- Instruction *I) {<br class="">+Instruction *NaryReassociate::tryReassociateBinaryOp(Value *LHS, Value *RHS,<br class="">+ BinaryOperator *I) {<br class=""> Value *A = nullptr, *B = nullptr;<br class="">- // To be conservative, we reassociate I only when it is the only user of A+B.<br class="">- if (LHS->hasOneUse() && match(LHS, m_Add(m_Value(A), m_Value(B)))) {<br class="">- // I = (A + B) + RHS<br class="">- // = (A + RHS) + B or (B + RHS) + A<br class="">+ // To be conservative, we reassociate I only when it is the only user of (A op<br class="">+ // B).<br class="">+ if (LHS->hasOneUse() && matchTernaryOp(I, LHS, A, B)) {<br class="">+ // I = (A op B) op RHS<br class="">+ // = (A op RHS) op B or (B op RHS) op A<br class=""> const SCEV *AExpr = SE->getSCEV(A), *BExpr = SE->getSCEV(B);<br class=""> const SCEV *RHSExpr = SE->getSCEV(RHS);<br class=""> if (BExpr != RHSExpr) {<br class="">- if (auto *NewI = tryReassociatedAdd(SE->getAddExpr(AExpr, RHSExpr), B, I))<br class="">+ if (auto *NewI =<br class="">+ tryReassociatedBinaryOp(getBinarySCEV(I, AExpr, RHSExpr), B, I))<br class=""> return NewI;<br class=""> }<br class=""> if (AExpr != RHSExpr) {<br class="">- if (auto *NewI = tryReassociatedAdd(SE->getAddExpr(BExpr, RHSExpr), A, I))<br class="">+ if (auto *NewI =<br class="">+ tryReassociatedBinaryOp(getBinarySCEV(I, BExpr, RHSExpr), A, I))<br class=""> return NewI;<br class=""> }<br class=""> }<br class=""> return nullptr;<br class="">}<br class=""><br class="">-Instruction *NaryReassociate::tryReassociatedAdd(const SCEV *LHSExpr,<br class="">- Value *RHS, Instruction *I) {<br class="">+Instruction *NaryReassociate::tryReassociatedBinaryOp(const SCEV *LHSExpr,<br class="">+ Value *RHS,<br class="">+ BinaryOperator *I) {<br class=""> // Look for the closest dominator LHS of I that computes LHSExpr, and replace<br class="">- // I with LHS + RHS.<br class="">+ // I with LHS op RHS.<br class=""> auto *LHS = findClosestMatchingDominator(LHSExpr, I);<br class=""> if (LHS == nullptr)<br class=""> return nullptr;<br class=""><br class="">- Instruction *NewI = BinaryOperator::CreateAdd(LHS, RHS, "", I);<br class="">+ Instruction *NewI = nullptr;<br class="">+ switch (I->getOpcode()) {<br class="">+ case Instruction::Add:<br class="">+ NewI = BinaryOperator::CreateAdd(LHS, RHS, "", I);<br class="">+ break;<br class="">+ case Instruction::Mul:<br class="">+ NewI = BinaryOperator::CreateMul(LHS, RHS, "", I);<br class="">+ break;<br class="">+ default:<br class="">+ llvm_unreachable("Unexpected instruction.");<br class="">+ }<br class=""> NewI->takeName(I);<br class=""> return NewI;<br class="">}<br class=""><br class="">+bool NaryReassociate::matchTernaryOp(BinaryOperator *I, Value *V, Value *&Op1,<br class="">+ Value *&Op2) {<br class="">+ switch (I->getOpcode()) {<br class="">+ case Instruction::Add:<br class="">+ return match(V, m_Add(m_Value(Op1), m_Value(Op2)));<br class="">+ case Instruction::Mul:<br class="">+ return match(V, m_Mul(m_Value(Op1), m_Value(Op2)));<br class="">+ default:<br class="">+ llvm_unreachable("Unexpected instruction.");<br class="">+ }<br class="">+ return false;<br class="">+}<br class="">+<br class="">+const SCEV *NaryReassociate::getBinarySCEV(BinaryOperator *I, const SCEV *LHS,<br class="">+ const SCEV *RHS) {<br class="">+ switch (I->getOpcode()) {<br class="">+ case Instruction::Add:<br class="">+ return SE->getAddExpr(LHS, RHS);<br class="">+ case Instruction::Mul:<br class="">+ return SE->getMulExpr(LHS, RHS);<br class="">+ default:<br class="">+ llvm_unreachable("Unexpected instruction.");<br class="">+ }<br class="">+ return nullptr;<br class="">+}<br class="">+<br class="">Instruction *<br class="">NaryReassociate::findClosestMatchingDominator(const SCEV *CandidateExpr,<br class=""> Instruction *Dominatee) {<br class=""><br class="">Added: llvm/trunk/test/Transforms/NaryReassociate/nary-mul.ll<br class="">URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/NaryReassociate/nary-mul.ll?rev=247705&view=auto" class="">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/NaryReassociate/nary-mul.ll?rev=247705&view=auto</a><br class="">==============================================================================<br class="">--- llvm/trunk/test/Transforms/NaryReassociate/nary-mul.ll (added)<br class="">+++ llvm/trunk/test/Transforms/NaryReassociate/nary-mul.ll Tue Sep 15 12:22:52 2015<br class="">@@ -0,0 +1,18 @@<br class="">+; RUN: opt < %s -nary-reassociate -S | FileCheck %s<br class="">+<br class="">+target datalayout = "e-i64:64-v16:16-v32:32-n16:32:64"<br class="">+<br class="">+declare void @foo(i32)<br class="">+<br class="">+; CHECK-LABEL: @bar(<br class="">+define void @bar(i32 %a, i32 %b, i32 %c) {<br class="">+ %1 = mul i32 %a, %c<br class="">+; CHECK: [[BASE:%[a-zA-Z0-9]+]] = mul i32 %a, %c<br class="">+ call void @foo(i32 %1)<br class="">+ %2 = mul i32 %a, %b<br class="">+ %3 = mul i32 %2, %c<br class="">+; CHECK: mul i32 [[BASE]], %b<br class="">+ call void @foo(i32 %3)<br class=""></blockquote><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">The CHECK seems incomplete to me, it is not clear what sequence is fed to the call in the end.</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; 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: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">Mehdi</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">_______________________________________________</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">llvm-commits mailing list</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><a href="mailto:llvm-commits@lists.llvm.org" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class="">llvm-commits@lists.llvm.org</a><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class="">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a></div></blockquote></div><br class=""></div></div></div></blockquote></div><br class=""></div></div></blockquote></div><br class=""></div></div></div></blockquote></div><br class=""></div></div></div></body></html>