<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
<meta name="Generator" content="Microsoft Word 15 (filtered medium)">
<style><!--
/* Font Definitions */
@font-face
{font-family:"Cambria Math";
panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0in;
margin-bottom:.0001pt;
font-size:12.0pt;
font-family:"Times New Roman",serif;}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:blue;
text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
{mso-style-priority:99;
color:purple;
text-decoration:underline;}
p.MsoListParagraph, li.MsoListParagraph, div.MsoListParagraph
{mso-style-priority:34;
margin-top:0in;
margin-right:0in;
margin-bottom:0in;
margin-left:.5in;
margin-bottom:.0001pt;
font-size:12.0pt;
font-family:"Times New Roman",serif;}
p.msonormal0, li.msonormal0, div.msonormal0
{mso-style-name:msonormal;
mso-margin-top-alt:auto;
margin-right:0in;
mso-margin-bottom-alt:auto;
margin-left:0in;
font-size:12.0pt;
font-family:"Times New Roman",serif;}
span.EmailStyle18
{mso-style-type:personal-reply;
font-family:"Calibri",sans-serif;
color:#1F497D;}
.MsoChpDefault
{mso-style-type:export-only;}
@page WordSection1
{size:8.5in 11.0in;
margin:1.0in 1.0in 1.0in 1.0in;}
div.WordSection1
{page:WordSection1;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]-->
</head>
<body lang="EN-US" link="blue" vlink="purple">
<div class="WordSection1">
<p class="MsoNormal">> OK, in that case I will keep working on a reduced test case. Once I have something I can share, I will forward to you.<o:p></o:p></p>
<p class="MsoNormal">><o:p> </o:p></p>
<p class="MsoNormal">> Sorry about the revert, and thanks for your help.<o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D">Sorry for the trouble in the first place. Thanks for looking further! It’s very much appreciated.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D">-Warren</span><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D"><o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><b><span style="font-size:11.0pt;font-family:"Calibri",sans-serif">From:</span></b><span style="font-size:11.0pt;font-family:"Calibri",sans-serif"> David Jones <dlj@google.com>
<br>
<b>Sent:</b> Wednesday, December 5, 2018 3:17 PM<br>
<b>To:</b> Ristow, Warren <warren.ristow@sony.com><br>
<b>Cc:</b> llvm-commits@lists.llvm.org; Reid Kleckner <rnk@google.com><br>
<b>Subject:</b> Re: [llvm] r347934 - [SCEV] Guard movement of insertion point for loop-invariants<o:p></o:p></span></p>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<div>
<div>
<div>
<p class="MsoNormal">On Wed, Dec 5, 2018 at 2:56 PM <<a href="mailto:warren.ristow@sony.com">warren.ristow@sony.com</a>> wrote:<o:p></o:p></p>
</div>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-right:0in">
<div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D">Hi David,</span><o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D"> </span><o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D">[+ Reid]</span><o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D"> </span><o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D">> ...</span><o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D">> I suspect your change is exposing a pre-existing bug; but unfortunately, it does very</span><o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D">> cleanly resolve to exactly r347934, and it occurs with multiple targets.</span><o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D">> </span><o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D">> Could you advise on a fix? Unfortunately, I'm not familiar with this code, so the best</span><o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D">> option I can offer is a revert. (If you won't able to author a timely fix, please let</span><o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D">> me know so I can submit the revert for you.)</span><o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D"> </span><o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D">Sorry to hear about that. I definitely won't be able to author a timely fix, so please do go ahead
and revert it.</span><o:p></o:p></p>
</div>
</div>
</blockquote>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">OK... sorry for bringing out the big hammer. :-/<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<div>
<p class="MsoNormal">FYI: Committed revision 348426.<o:p></o:p></p>
</div>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal"> <o:p></o:p></p>
</div>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-right:0in">
<div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D"> </span><o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D">As it happens, while looking at the history of this SCEV div-by-0 area, in prep for a follow-up patch:</span><o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D"><a href="https://reviews.llvm.org/D55232" target="_blank">https://reviews.llvm.org/D55232</a></span><o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D"> </span><o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D">I noticed that an equivalent change was made about a year ago (r289412). Reid found it caused a problem
in instcombine when ASan was enabled. This issue you reported sounds very similar. Reid reverted that year-old commit at r289453:</span><o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D"> </span><o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D"> ‘’’</span><o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D"> Reverts r289412. It caused an OOB PHI operand access in instcombine when</span><o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D"> ASan is enabled. Reduction in progress.</span><o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D"> ‘’’</span><o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D"> </span><o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D">> I don't feel I have a useful reproducer to share... between creduce and bugpoint, I've</span><o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D">> gotten one target down to 68k of .bc (which disassembles to about 180k of .ll). (And it</span><o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D">> would take a lot of effort to redact before I can share it.)</span><o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D"> </span><o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D">Possibly Reid has access to a reduced test-case from back then (or can create one now)?</span><o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D"> </span><o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D">I'm not familiar with the code where the instcombine failure is happening. Without a test-case, I
feel my hands are tied. So a reduced test-case from either you or Reid would be greatly appreciated!</span><o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D"> </span><o:p></o:p></p>
</div>
</div>
</blockquote>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">OK, in that case I will keep working on a reduced test case. Once I have something I can share, I will forward to you.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">Sorry about the revert, and thanks for your help.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">--dlj<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal"> <o:p></o:p></p>
</div>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-right:0in">
<div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D">Thanks,</span><o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D">-Warren</span><o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D"> </span><o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"><b><span style="font-size:11.0pt;font-family:"Calibri",sans-serif">From:</span></b><span style="font-size:11.0pt;font-family:"Calibri",sans-serif"> David Jones <<a href="mailto:dlj@google.com" target="_blank">dlj@google.com</a>>
<br>
<b>Sent:</b> Wednesday, December 5, 2018 2:25 PM<br>
<b>To:</b> Ristow, Warren <<a href="mailto:warren.ristow@sony.com" target="_blank">warren.ristow@sony.com</a>><br>
<b>Cc:</b> <a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<b>Subject:</b> Re: [llvm] r347934 - [SCEV] Guard movement of insertion point for loop-invariants</span><o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"> <o:p></o:p></p>
<div>
<div>
<div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">Hi Warren --<o:p></o:p></p>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"> <o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">I've observed several Clang crashes on SEGV, which bisect to this change.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"> <o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">The crashes generally look something like this:<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"> <o:p></o:p></p>
</div>
<div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">$ opt -O2 -o /dev/null bugpoint-reduced-simplified.ll<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">Stack dump:<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">0. Program arguments: opt -O2 -o /dev/null bugpoint-reduced-simplified.ll <o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">1. Running pass 'CallGraph Pass Manager' on module 'bugpoint-reduced-simplified.ll'.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">2. Running pass 'Combine redundant instructions' on function '@_Z[redacted]'<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">#0 0x0000558aa2221628 llvm::sys::RunSignalHandlers() (opt+0x1f3b628)<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">#1 0x0000558aa2223746 SignalHandler(int) (opt+0x1f3d746)<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">#2 0x00007f5def60a9a0 __restore_rt (/usr/grte/v4/lib64/libpthread.so.0+0xf9a0)<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">#3 0x0000558aa1bbec7f llvm::InstCombiner::visitPHINode(llvm::PHINode&) (opt+0x18d8c7f)<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">#4 0x0000558aa1b8db24 llvm::InstCombiner::run() (opt+0x18a7b24)<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">#5 0x0000558aa1b8f504 combineInstructionsOverFunction(llvm::Function&, llvm::InstCombineWorklist&, llvm::AAResults*, llvm::AssumptionCache&, llvm::TargetLibraryInfo&, llvm::DominatorTree&,
llvm::OptimizationRemarkEmitter&, bool, llvm::LoopInfo*) (opt+0x18a9504)<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">#6 0x0000558aa1b8f8af llvm::InstructionCombiningPass::runOnFunction(llvm::Function&) (opt+0x18a98af)<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">#7 0x0000558aa207d2fc llvm::FPPassManager::runOnFunction(llvm::Function&) (opt+0x1d972fc)<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">#8 0x0000558aa1f062e0 (anonymous namespace)::CGPassManager::runOnModule(llvm::Module&) (opt+0x1c202e0)<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">#9 0x0000558aa207da85 llvm::legacy::PassManagerImpl::run(llvm::Module&) (opt+0x1d97a85)<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">#10 0x0000558aa0a8900b main (opt+0x7a300b)<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">#11 0x00007f5def378bbd __libc_start_main (/usr/grte/v4/lib64/libc.so.6+0x38bbd)<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">#12 0x0000558aa0a740e9 _start /usr/grte/v4/debug-src/src/csu/../sysdeps/x86_64/start.S:121:0<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">[1] 17713 segmentation fault opt -O2 -o /dev/null bugpoint-reduced-simplified.ll<o:p></o:p></p>
</div>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"> <o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"> <o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">gdb's backtrace is a bit more useful:<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"> <o:p></o:p></p>
</div>
<div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">#0 0x000055555958b1d4 in llvm::Use::get (this=<optimized out>) at third_party/llvm/llvm/include/llvm/IR/Use.h:108<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">#1 llvm::PHINode::getOperand (this=<optimized out>, i_nocapture=4294967295) at third_party/llvm/llvm/include/llvm/IR/Instructions.h:2714<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">#2 llvm::PHINode::getIncomingValue (this=<optimized out>, i=4294967295) at third_party/llvm/llvm/include/llvm/IR/Instructions.h:2603<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">#3 0x000055555c41abba in llvm::InstCombiner::visitPHINode (this=0x7ffff4e3b970, PN=...) at third_party/llvm/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp:1238<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">#4 0x000055555c378489 in llvm::InstCombiner::run (this=<optimized out>) at third_party/llvm/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp:3192<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">#5 0x000055555c37b6d2 in combineInstructionsOverFunction (F=..., Worklist=..., AA=0x60600002b7c0, AC=..., TLI=..., DT=..., ORE=..., ExpensiveCombines=<optimized out>, LI=<optimized
out>)<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"> at third_party/llvm/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp:3424<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">#6 0x000055555c37be37 in llvm::InstructionCombiningPass::runOnFunction (this=<optimized out>, F=...) at third_party/llvm/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp:3483<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">#7 0x000055555d39b609 in llvm::FPPassManager::runOnFunction (this=<optimized out>, F=...) at third_party/llvm/llvm/lib/IR/LegacyPassManager.cpp:1644<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">#8 0x000055555ce8f11b in (anonymous namespace)::CGPassManager::RunPassOnSCC (this=<optimized out>, P=0x614000009a40, CurSCC=..., CG=..., CallGraphUpToDate=<optimized out>, DevirtualizedCall=<optimized
out>)<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"> at third_party/llvm/llvm/lib/Analysis/CallGraphSCCPass.cpp:178<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">#9 (anonymous namespace)::CGPassManager::RunAllPassesOnSCC (this=<optimized out>, CurSCC=..., CG=..., DevirtualizedCall=<optimized out>) at third_party/llvm/llvm/lib/Analysis/CallGraphSCCPass.cpp:442<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">#10 (anonymous namespace)::CGPassManager::runOnModule (this=0x614000009840, M=...) at third_party/llvm/llvm/lib/Analysis/CallGraphSCCPass.cpp:498<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">#11 0x000055555d39ca7a in (anonymous namespace)::MPPassManager::runOnModule (this=<optimized out>, M=...) at third_party/llvm/llvm/lib/IR/LegacyPassManager.cpp:1744<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">#12 llvm::legacy::PassManagerImpl::run (this=0x619000020f50, M=...) at third_party/llvm/llvm/lib/IR/LegacyPassManager.cpp:1857<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">#13 0x00005555588bfb58 in main (argc=<optimized out>, argv=<optimized out>) at third_party/llvm/llvm/tools/opt/opt.cpp:828<o:p></o:p></p>
</div>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"> <o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"> <o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"> <o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">I've spent the last 48 hours attempting to reduce a test case (both mechanically and manually). Unfortunately, I don't feel I have a useful reproducer to share... between creduce
and bugpoint, I've gotten one target down to 68k of .bc (which disassembles to about 180k of .ll). (And it would take a lot of effort to redact before I can share it.)<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"> <o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">The stack trace pretty clearly points to an unchecked out-of-range return value here:<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"> <o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"><a href="https://git.llvm.org/klaus/llvm/blob/master/lib/Transforms/InstCombine/InstCombinePHI.cpp#L-1238" target="_blank">https://git.llvm.org/klaus/llvm/blob/master/lib/Transforms/InstCombine/InstCombinePHI.cpp#L-1238</a><o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"> <o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">I suspect your change is exposing a pre-existing bug; but unfortunately, it does very cleanly resolve to exactly r347934, and it occurs with multiple targets.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"> <o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">Could you advise on a fix? Unfortunately, I'm not familiar with this code, so the best option I can offer is a revert. (If you won't able to author a timely fix, please let me know
so I can submit the revert for you.)<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"> <o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">--David Jones<o:p></o:p></p>
</div>
</div>
</div>
</div>
</div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"> <o:p></o:p></p>
<div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">On Thu, Nov 29, 2018 at 4:05 PM Warren Ristow via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>> wrote:<o:p></o:p></p>
</div>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-top:5.0pt;margin-right:0in;margin-bottom:5.0pt">
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">Author: wristow<br>
Date: Thu Nov 29 16:02:54 2018<br>
New Revision: 347934<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=347934&view=rev" target="_blank">
http://llvm.org/viewvc/llvm-project?rev=347934&view=rev</a><br>
Log:<br>
[SCEV] Guard movement of insertion point for loop-invariants<br>
<br>
r320789 suppressed moving the insertion point of SCEV expressions with<br>
dev/rem operations to the loop header in non-loop-invariant situations.<br>
This, and similar, hoisting is also unsafe in the loop-invariant case,<br>
since there may be a guard against a zero denominator. This is an<br>
adjustment to the fix of r320789 to suppress the movement even in the<br>
loop-invariant case.<br>
<br>
This fixes PR30806.<br>
<br>
Differential Revision: <a href="https://reviews.llvm.org/D54713" target="_blank">
https://reviews.llvm.org/D54713</a><br>
<br>
Added:<br>
llvm/trunk/test/Transforms/LoopVectorize/pr30806.ll<br>
Modified:<br>
llvm/trunk/lib/Analysis/ScalarEvolutionExpander.cpp<br>
<br>
Modified: llvm/trunk/lib/Analysis/ScalarEvolutionExpander.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/ScalarEvolutionExpander.cpp?rev=347934&r1=347933&r2=347934&view=diff" target="_blank">
http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/ScalarEvolutionExpander.cpp?rev=347934&r1=347933&r2=347934&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Analysis/ScalarEvolutionExpander.cpp (original)<br>
+++ llvm/trunk/lib/Analysis/ScalarEvolutionExpander.cpp Thu Nov 29 16:02:54 2018<br>
@@ -1732,49 +1732,50 @@ Value *SCEVExpander::expand(const SCEV *<br>
// Compute an insertion point for this SCEV object. Hoist the instructions<br>
// as far out in the loop nest as possible.<br>
Instruction *InsertPt = &*Builder.GetInsertPoint();<br>
- for (Loop *L = SE.LI.getLoopFor(Builder.GetInsertBlock());;<br>
- L = L->getParentLoop())<br>
- if (SE.isLoopInvariant(S, L)) {<br>
- if (!L) break;<br>
- if (BasicBlock *Preheader = L->getLoopPreheader())<br>
- InsertPt = Preheader->getTerminator();<br>
- else {<br>
- // LSR sets the insertion point for AddRec start/step values to the<br>
- // block start to simplify value reuse, even though it's an invalid<br>
- // position. SCEVExpander must correct for this in all cases.<br>
- InsertPt = &*L->getHeader()->getFirstInsertionPt();<br>
- }<br>
- } else {<br>
- // We can move insertion point only if there is no div or rem operations<br>
- // otherwise we are risky to move it over the check for zero denominator.<br>
- auto SafeToHoist = [](const SCEV *S) {<br>
- return !SCEVExprContains(S, [](const SCEV *S) {<br>
- if (const auto *D = dyn_cast<SCEVUDivExpr>(S)) {<br>
- if (const auto *SC = dyn_cast<SCEVConstant>(D->getRHS()))<br>
- // Division by non-zero constants can be hoisted.<br>
- return SC->getValue()->isZero();<br>
- // All other divisions should not be moved as they may be<br>
- // divisions by zero and should be kept within the<br>
- // conditions of the surrounding loops that guard their<br>
- // execution (see PR35406).<br>
- return true;<br>
- }<br>
- return false;<br>
- });<br>
- };<br>
- // If the SCEV is computable at this level, insert it into the header<br>
- // after the PHIs (and after any other instructions that we've inserted<br>
- // there) so that it is guaranteed to dominate any user inside the loop.<br>
- if (L && SE.hasComputableLoopEvolution(S, L) && !PostIncLoops.count(L) &&<br>
- SafeToHoist(S))<br>
- InsertPt = &*L->getHeader()->getFirstInsertionPt();<br>
- while (InsertPt->getIterator() != Builder.GetInsertPoint() &&<br>
- (isInsertedInstruction(InsertPt) ||<br>
- isa<DbgInfoIntrinsic>(InsertPt))) {<br>
- InsertPt = &*std::next(InsertPt->getIterator());<br>
+<br>
+ // We can move insertion point only if there is no div or rem operations<br>
+ // otherwise we are risky to move it over the check for zero denominator.<br>
+ auto SafeToHoist = [](const SCEV *S) {<br>
+ return !SCEVExprContains(S, [](const SCEV *S) {<br>
+ if (const auto *D = dyn_cast<SCEVUDivExpr>(S)) {<br>
+ if (const auto *SC = dyn_cast<SCEVConstant>(D->getRHS()))<br>
+ // Division by non-zero constants can be hoisted.<br>
+ return SC->getValue()->isZero();<br>
+ // All other divisions should not be moved as they may be<br>
+ // divisions by zero and should be kept within the<br>
+ // conditions of the surrounding loops that guard their<br>
+ // execution (see PR35406).<br>
+ return true;<br>
+ }<br>
+ return false;<br>
+ });<br>
+ };<br>
+ if (SafeToHoist(S)) {<br>
+ for (Loop *L = SE.LI.getLoopFor(Builder.GetInsertBlock());;<br>
+ L = L->getParentLoop()) {<br>
+ if (SE.isLoopInvariant(S, L)) {<br>
+ if (!L) break;<br>
+ if (BasicBlock *Preheader = L->getLoopPreheader())<br>
+ InsertPt = Preheader->getTerminator();<br>
+ else<br>
+ // LSR sets the insertion point for AddRec start/step values to the<br>
+ // block start to simplify value reuse, even though it's an invalid<br>
+ // position. SCEVExpander must correct for this in all cases.<br>
+ InsertPt = &*L->getHeader()->getFirstInsertionPt();<br>
+ } else {<br>
+ // If the SCEV is computable at this level, insert it into the header<br>
+ // after the PHIs (and after any other instructions that we've inserted<br>
+ // there) so that it is guaranteed to dominate any user inside the loop.<br>
+ if (L && SE.hasComputableLoopEvolution(S, L) && !PostIncLoops.count(L))<br>
+ InsertPt = &*L->getHeader()->getFirstInsertionPt();<br>
+ while (InsertPt->getIterator() != Builder.GetInsertPoint() &&<br>
+ (isInsertedInstruction(InsertPt) ||<br>
+ isa<DbgInfoIntrinsic>(InsertPt)))<br>
+ InsertPt = &*std::next(InsertPt->getIterator());<br>
+ break;<br>
}<br>
- break;<br>
}<br>
+ }<br>
<br>
// Check to see if we already expanded this here.<br>
auto I = InsertedExpressions.find(std::make_pair(S, InsertPt));<br>
<br>
Added: llvm/trunk/test/Transforms/LoopVectorize/pr30806.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/LoopVectorize/pr30806.ll?rev=347934&view=auto" target="_blank">
http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/LoopVectorize/pr30806.ll?rev=347934&view=auto</a><br>
==============================================================================<br>
--- llvm/trunk/test/Transforms/LoopVectorize/pr30806.ll (added)<br>
+++ llvm/trunk/test/Transforms/LoopVectorize/pr30806.ll Thu Nov 29 16:02:54 2018<br>
@@ -0,0 +1,65 @@<br>
+; RUN: opt -loop-vectorize -S < %s 2>&1 | FileCheck %s<br>
+<br>
+; Produced from test-case:<br>
+;<br>
+; void testGuardedInnerLoop(uint32_t *ptr, uint32_t denom, uint32_t numer, uint32_t outer_lim)<br>
+; {<br>
+; for(uint32_t outer_i = 0; outer_i < outer_lim; ++outer_i) {<br>
+; if (denom > 0) {<br>
+; const uint32_t lim = numer / denom;<br>
+;<br>
+; for (uint32_t i = 0; i < lim; ++i)<br>
+; ptr[i] = 1;<br>
+; }<br>
+; }<br>
+; }<br>
+<br>
+<br>
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128-ni:1"<br>
+target triple = "x86_64-unknown-linux-gnu"<br>
+<br>
+define void @testGuardedInnerLoop(i32* %ptr, i32 %denom, i32 %numer, i32 %outer_lim) {<br>
+entry:<br>
+ %cmp1 = icmp eq i32 %outer_lim, 0<br>
+ br i1 %cmp1, label %exit, label %loop1.preheader<br>
+<br>
+; Verify that a 'udiv' does not appear between the 'loop1.preheader' label, and<br>
+; whatever label comes next.<br>
+loop1.preheader:<br>
+; CHECK-LABEL: loop1.preheader:<br>
+; CHECK-NOT: udiv<br>
+; CHECK-LABEL: :<br>
+ br label %loop1<br>
+<br>
+loop1:<br>
+ %outer_i = phi i32 [ %inc1, %loop2.exit ], [ 0, %loop1.preheader ]<br>
+ %0 = add i32 %denom, -1<br>
+ %1 = icmp ult i32 %0, %numer<br>
+ br i1 %1, label %loop2.preheader, label %loop2.exit<br>
+<br>
+; Verify that a 'udiv' does appear between the 'loop2.preheader' label, and<br>
+; whatever label comes next.<br>
+loop2.preheader:<br>
+; CHECK-LABEL: loop2.preheader:<br>
+; CHECK: udiv<br>
+; CHECK-LABEL: :<br>
+ %lim = udiv i32 %numer, %denom<br>
+ %2 = zext i32 %lim to i64<br>
+ br label %loop2<br>
+<br>
+loop2:<br>
+ %indvar.loop2 = phi i64 [ 0, %loop2.preheader ], [ %indvar.loop2.next, %loop2 ]<br>
+ %arrayidx = getelementptr inbounds i32, i32* %ptr, i64 %indvar.loop2<br>
+ store i32 1, i32* %arrayidx, align 4<br>
+ %indvar.loop2.next = add nuw nsw i64 %indvar.loop2, 1<br>
+ %cmp2 = icmp ult i64 %indvar.loop2.next, %2<br>
+ br i1 %cmp2, label %loop2, label %loop2.exit<br>
+<br>
+loop2.exit:<br>
+ %inc1 = add nuw i32 %outer_i, 1<br>
+ %exitcond = icmp eq i32 %inc1, %outer_lim<br>
+ br i1 %exitcond, label %exit, label %loop1<br>
+<br>
+exit:<br>
+ ret void<br>
+}<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" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><o:p></o:p></p>
</blockquote>
</div>
</div>
</div>
</blockquote>
</div>
</div>
</div>
</div>
</body>
</html>