<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Tue, Nov 19, 2013 at 8:25 PM, David Blaikie <span dir="ltr"><<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra">Just moving this branch of the thread out of the review because I don't want to derail the review thread...<br>
<br>Kostya - why are these two cases not optimization bugs in general? (why do they only affect sanitizers?)</div></div></blockquote><div><br></div><div>The recent case from mozilla (<a href="https://code.google.com/p/thread-sanitizer/issues/detail?id=40#c2" target="_blank">https://code.google.com/p/thread-sanitizer/issues/detail?id=40#c2</a>) is a legal </div>
<div>optimization -- it hoists a safe load (i.e. a load which is known not to fail) out of conditional branch.</div><div>It reduces the number of basic blocks and branches, and so I think it's good in general. </div><div>
I can't imagine a case where this optimization will break a valid program. </div><div>Which is the second one you are referring to? </div><div><br></div><div>--kcc </div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div dir="ltr"><div class="gmail_extra"> </div></div></blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div dir="ltr"><div class="gmail_extra">
<br><div class="gmail_quote">On Mon, Nov 18, 2013 at 8:37 PM, Kostya Serebryany <span dir="ltr"><<a href="mailto:kcc@google.com" target="_blank">kcc@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div dir="ltr">And we've been just informed by the mozilla folks about yet another case of optimization being hostile to sanitizers: <div>hoisting a safe load out of conditional branch introduces a race which tsan happily reports. <br>
<div><a href="https://code.google.com/p/thread-sanitizer/issues/detail?id=40#c2" target="_blank">https://code.google.com/p/thread-sanitizer/issues/detail?id=40#c2</a><br></div></div><div><br></div><div>--kcc </div></div>
<div><div><div class="gmail_extra">
<br><br><div class="gmail_quote">On Tue, Nov 19, 2013 at 8:06 AM, Kostya Serebryany <span dir="ltr"><<a href="mailto:kcc@google.com" target="_blank">kcc@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote"><div>On Tue, Nov 19, 2013 at 1:27 AM, David Blaikie <span dir="ltr"><<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr">Do we have precedence for this kind of change (where sanitizers affect optimizations in arbitrary internal ways - not simply by enabling/disabling certain passes)? </div>
</blockquote><div><br></div></div><div>Yes. AddressSanitizer and ThreadSanitizer disable load widening that would create a partially out-of-bounds or a racy access. </div>See lib/Analysis/MemoryDependenceAnalysis.cpp (search for Attribute::SanitizeAddress and Attribute::SanitizeThread).<br>
</div><div class="gmail_quote">This case with MemorySanitizer is slightly different because we are not fighting a false positive, but rather a debug-info-damaging optimization. </div><div class="gmail_quote"><br></div><div class="gmail_quote">
--kcc </div><div><div><div class="gmail_quote"><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div dir="ltr">
If not, does this need some deeper discussion about alternatives (is it important that we be able to produce equivalent code without the sanitizers enabled?)?<br>
<div class="gmail_extra"><br><br><div class="gmail_quote"><div><div>On Mon, Nov 18, 2013 at 7:02 AM, Evgeniy Stepanov <span dir="ltr"><<a href="mailto:eugenis@google.com" target="_blank">eugenis@google.com</a>></span> wrote:<br>
</div></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div><div>Branch folding optimization often leads to confusing MSan reports due to lost debug info.<br>
For example,<br>
1: if (x < 0)<br>
2: if (y < 0)<br>
3: do_something();<br>
is transformed into something like<br>
%0 = and i32 %y, %x<br>
%1 = icmp slt i32 %0, 0<br>
br i1 %1, label %if.then2, label %if.end3<br>
where all 3 instructions are associated with line 1.<br>
<br>
This patch disables folding of conditional branches in functions with sanitize_memory attribute.<br>
<br>
<a href="http://llvm-reviews.chandlerc.com/D2214" target="_blank">http://llvm-reviews.chandlerc.com/D2214</a><br>
<br>
Files:<br>
lib/Transforms/Utils/SimplifyCFG.cpp<br>
test/Transforms/SimplifyCFG/branch-fold-msan.ll<br>
<br>
Index: lib/Transforms/Utils/SimplifyCFG.cpp<br>
===================================================================<br>
--- lib/Transforms/Utils/SimplifyCFG.cpp<br>
+++ lib/Transforms/Utils/SimplifyCFG.cpp<br>
@@ -1967,6 +1967,13 @@<br>
bool llvm::FoldBranchToCommonDest(BranchInst *BI) {<br>
BasicBlock *BB = BI->getParent();<br>
<br>
+ // This optimization results in confusing MemorySanitizer reports. Use of<br>
+ // uninitialized value in this branch instruction is reported with the<br>
+ // predecessor's debug location.<br>
+ if (BB->getParent()->hasFnAttribute(Attribute::SanitizeMemory) &&<br>
+ BI->isConditional())<br>
+ return false;<br>
+<br>
Instruction *Cond = 0;<br>
if (BI->isConditional())<br>
Cond = dyn_cast<Instruction>(BI->getCondition());<br>
Index: test/Transforms/SimplifyCFG/branch-fold-msan.ll<br>
===================================================================<br>
--- test/Transforms/SimplifyCFG/branch-fold-msan.ll<br>
+++ test/Transforms/SimplifyCFG/branch-fold-msan.ll<br>
@@ -0,0 +1,29 @@<br>
+; RUN: opt < %s -simplifycfg -S | FileCheck %s<br>
+<br>
+declare void @callee()<br>
+<br>
+; Test that conditional branches are not folded with sanitize_memory.<br>
+define void @caller(i32 %x, i32 %y) sanitize_memory {<br>
+; CHECK: define void @caller(i32 [[X:%.*]], i32 [[Y:%.*]])<br>
+; CHECK: icmp slt i32 {{.*}}[[X]]<br>
+; CHECK: icmp slt i32 {{.*}}[[Y]]<br>
+; CHECK: ret void<br>
+<br>
+entry:<br>
+ %cmp = icmp slt i32 %x, 0<br>
+ br i1 %cmp, label %if.then, label %if.end3<br>
+<br>
+if.then: ; preds = %entry<br>
+ %cmp1 = icmp slt i32 %y, 0<br>
+ br i1 %cmp1, label %if.then2, label %if.end<br>
+<br>
+if.then2: ; preds = %if.then<br>
+ call void @callee()<br>
+ br label %if.end<br>
+<br>
+if.end: ; preds = %if.then2, %if.then<br>
+ br label %if.end3<br>
+<br>
+if.end3: ; preds = %if.end, %entry<br>
+ ret void<br>
+}<br>
<br></div></div>_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
<br></blockquote></div><br></div></div>
<br>_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
<br></blockquote></div><br></div></div></div></div>
</blockquote></div><br></div>
</div></div></blockquote></div><br></div></div>
</blockquote></div><br></div></div>