<html><head><meta http-equiv="Content-Type" content="text/html charset=windows-1252"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class="">Hi,</div><div class=""><br class=""></div><div class="">The patch looks good but I’d like to see some benchmarking data before fully signing off on this. Most importantly, do we vectorize more now? My concern is that we may have tuned the threshold with the incorrect formula.</div><div class=""><br class=""></div><div class="">More comments below. Also next time, please use Phab, thanks!</div><div class=""><br class=""></div>On Jun 2, 2015, at 9:44 AM, Silviu Baranga <<a href="mailto:silviu.baranga@arm.com" class="">silviu.baranga@arm.com</a>> wrote:<br class=""><div><blockquote type="cite" class=""><br class="Apple-interchange-newline"><div class=""><div class="WordSection1" style="page: WordSection1; orphans: auto; text-align: start; text-indent: 0px; widows: auto; -webkit-text-stroke-width: 0px; word-spacing: 0px; white-space: normal; text-transform: none; line-height: normal; letter-spacing: normal; font-weight: normal; font-variant: normal; font-style: normal; font-size: 11px; font-family: Helvetica;"><div style="margin: 0cm 0cm 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">Hi,<o:p class=""></o:p></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class=""><o:p class=""> </o:p></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">The attached patch fixes the estimation of the number of memchecks.<o:p class=""></o:p></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class=""><o:p class=""> </o:p></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">We need to add a runtime memcheck for pair of accesses (x,y) where at least one of x and y<o:p class=""></o:p></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">are writes.<o:p class=""></o:p></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class=""><o:p class=""> </o:p></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">Assuming we have w writes and r reads, currently this number is estimated as being<o:p class=""></o:p></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">w* (w+r-1). This estimation will count (write,write) pairs twice and will overestimate<o:p class=""></o:p></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">the number of checks required.<o:p class=""></o:p></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class=""><o:p class=""> </o:p></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">The actual number of memchecks is (r + w - 1) + (r + w - 2) + .. + r which is equal to<o:p class=""></o:p></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">(w^2 + 2wr - w)/2.<o:p class=""></o:p></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class=""><o:p class=""> </o:p></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">The patch changes the formula and adds a small test.<o:p class=""></o:p></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class=""><o:p class=""> </o:p></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">Please review!<o:p class=""></o:p></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class=""><o:p class=""> </o:p></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">Thanks,<o:p class=""></o:p></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">Silviu<o:p class=""></o:p></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class=""><o:p class=""> </o:p></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class=""><o:p class=""> </o:p></div></div></div></blockquote><div><blockquote type="cite" class=""><div>diff --git a/lib/Analysis/LoopAccessAnalysis.cpp b/lib/Analysis/LoopAccessAnalysis.cpp</div><div>index 782bf70..5f97392 100644</div><div>--- a/lib/Analysis/LoopAccessAnalysis.cpp</div><div>+++ b/lib/Analysis/LoopAccessAnalysis.cpp</div><div>@@ -357,8 +357,12 @@ bool AccessAnalysis::canCheckPtrAtRT(</div><div> if (IsDepCheckNeeded && CanDoRT && RunningDepId == 2)</div><div> NumComparisons += 0; // Only one dependence set.</div><div> else {</div><div>- NumComparisons += (NumWritePtrChecks * (NumReadPtrChecks +</div><div>- NumWritePtrChecks - 1));</div><div>+ // For r reads and w writes we will do</div><div>+ // (r + w - 1) + (r + w - 2) + .. + r checks,</div><div>+ // which evaluates to (w^2 + 2wr - w) / 2 checks.</div><div>+ NumComparisons += (NumWritePtrChecks * NumWritePtrChecks +</div><div>+ 2 * NumWritePtrChecks * NumReadPtrChecks -</div><div>+ NumWritePtrChecks) / 2;</div></blockquote><div><br class=""></div><div>Please add more comment. Clearly this is not obvious because whoever wrote the original code didn’t get it right. Something like this: for each write we have to compare against all reads and write that we haven’t previously compared against.</div><div><br class=""></div><div>You may also want to add the alternative way of how for example I worked out your formula: it’s a combination of two writes plus all writes against all reads, i.e. C(w, 2) + w*r = w*(w-1)/2 + wr = (w^2 - w + 2wr)/2</div><br class=""><blockquote type="cite" class=""><div> }</div><div> </div><div> ++ASId;</div><div>diff --git a/test/Analysis/LoopAccessAnalysis/number-of-memchecks.ll b/test/Analysis/LoopAccessAnalysis/number-of-memchecks.ll</div><div>new file mode 100644</div><div>index 0000000..4ef3667</div><div>--- /dev/null</div><div>+++ b/test/Analysis/LoopAccessAnalysis/number-of-memchecks.ll</div><div>@@ -0,0 +1,52 @@</div><div>+; RUN: opt -loop-accesses -analyze < %s | FileCheck %s</div><div>+</div><div>+; 3 reads and 3 writes should need 12 memchecks</div><div>+</div><div>+target datalayout = "e-m:e-i64:64-i128:128-n32:64-S128"</div><div>+target triple = "aarch64--linux-gnueabi"</div><div>+</div><div>+; CHECK: Memory dependences are safe with run-time checks</div><div>+; CHECK: 11:</div><div>+; CHECK-NOT: 12:</div></blockquote><div><br class=""></div><div>Your comment above and what you’re checking disagree. I think the correct number is 12 and we used estimate 15 here.</div><div><br class=""></div><div>Please also match the line "Run-time memory checks:” and then the number after it with CHECK-NEXT.</div><div><br class=""></div><div>Thanks,</div><div>Adam</div><br class=""><blockquote type="cite" class=""><div>+</div><div>+define void @testf(i16* %a,</div><div>+ i16* %b,</div><div>+ i16* %c,</div><div>+ i16* %d,</div><div>+ i16* %e,</div><div>+ i16* %f) {</div><div>+entry:</div><div>+ br label %for.body</div><div>+</div><div>+for.body: ; preds = %for.body, %entry</div><div>+ %ind = phi i64 [ 0, %entry ], [ %add, %for.body ]</div><div>+</div><div>+ %add = add nuw nsw i64 %ind, 1</div><div>+</div><div>+ %arrayidxA = getelementptr inbounds i16, i16* %a, i64 %ind</div><div>+ %loadA = load i16, i16* %arrayidxA, align 2</div><div>+</div><div>+ %arrayidxB = getelementptr inbounds i16, i16* %b, i64 %ind</div><div>+ %loadB = load i16, i16* %arrayidxB, align 2</div><div>+</div><div>+ %arrayidxC = getelementptr inbounds i16, i16* %c, i64 %ind</div><div>+ %loadC = load i16, i16* %arrayidxC, align 2</div><div>+</div><div>+ %mul = mul i16 %loadB, %loadA</div><div>+ %mul1 = mul i16 %mul, %loadC</div><div>+</div><div>+ %arrayidxD = getelementptr inbounds i16, i16* %d, i64 %ind</div><div>+ store i16 %mul1, i16* %arrayidxD, align 2</div><div>+</div><div>+ %arrayidxE = getelementptr inbounds i16, i16* %e, i64 %ind</div><div>+ store i16 %mul, i16* %arrayidxE, align 2</div><div>+</div><div>+ %arrayidxF = getelementptr inbounds i16, i16* %f, i64 %ind</div><div>+ store i16 %mul1, i16* %arrayidxF, align 2</div><div>+</div><div>+ %exitcond = icmp eq i64 %add, 20</div><div>+ br i1 %exitcond, label %for.end, label %for.body</div><div>+</div><div>+for.end: ; preds = %for.body</div><div>+ ret void</div><div>+}</div></blockquote><div class=""><div><br class=""></div></div></div><div><br class=""></div><br class=""><blockquote type="cite" class=""><div class=""><div class="WordSection1" style="page: WordSection1; orphans: auto; text-align: start; text-indent: 0px; widows: auto; -webkit-text-stroke-width: 0px; word-spacing: 0px; white-space: normal; text-transform: none; line-height: normal; letter-spacing: normal; font-weight: normal; font-variant: normal; font-style: normal; font-size: 11px; font-family: Helvetica;"><div style="margin: 0cm 0cm 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class=""><o:p class=""> </o:p></div></div><span id="cid:15AA98ED-0D71-4A28-8A80-CABB100E7D09@apple.com"><runtime.diff></span><span style="font-family: Helvetica; font-size: 11px; 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: 11px; 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: 11px; 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: 11px; 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@cs.uiuc.edu" style="color: purple; text-decoration: underline; font-family: Helvetica; font-size: 11px; 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@cs.uiuc.edu</a><br style="font-family: Helvetica; font-size: 11px; 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.cs.uiuc.edu/mailman/listinfo/llvm-commits" style="color: purple; text-decoration: underline; font-family: Helvetica; font-size: 11px; 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.cs.uiuc.edu/mailman/listinfo/llvm-commits</a></div></blockquote></div><br class=""></body></html>