<div dir="ltr">Philip,<div><br></div><div>FYI, I was able to crash a stage 1 build with this:</div><div><br></div><div><div><br></div><div>```</div><div>static void loops(int lim)</div><div>{</div><div>  for (int i = 0; i < 1;)</div><div>    for (int j = 0; j < lim; ++j)</div><div>      ++i;</div><div>}</div><div><br></div><div>typedef void (*handler)(int);</div><div>handler x = loops;</div></div><div>```</div><div><br></div><div>Here is the stack trace (edited for clarity):</div><div><br></div><div><div>#0  SignalHandler(int) </div><div>#1  __restore_rt </div><div>#2  llvm::simplifyUsersOfIV(llvm::PHINode*, llvm::ScalarEvolution*, llvm::DominatorTree*, llvm::LoopInfo*, llvm::SmallVectorImpl<llvm::WeakTrackingVH>&, llvm::SCEVExpander&, llvm::IVVisitor*)</div><div>#3  (anonymous namespace)::IndVarSimplify::run(llvm::Loop*)</div><div>#4  (anonymous namespace)::IndVarSimplifyLegacyPass::runOnLoop(llvm::Loop*, llvm::LPPassManager&) </div><div>#5  llvm::LPPassManager::runOnFunction(llvm::Function&) </div><div>#6  llvm::FPPassManager::runOnFunction(llvm::Function&)</div><div>#7  (anonymous namespace)::CGPassManager::runOnModule(llvm::Module&) </div><div>#8  llvm::legacy::PassManagerImpl::run(llvm::Module&)</div><div>#9  clang::EmitBackendOutput(clang::DiagnosticsEngine&, clang::HeaderSearchOptions const&, clang::CodeGenOptions const&, clang::TargetOptions const&, clang::LangOptions const&, llvm::DataLayout const&, llvm::Module*, clang::BackendAction, std::unique_ptr<llvm::raw_pwrite_stream, std::default_delete<llvm::raw_pwrite_stream> >) </div><div>#10  clang::BackendConsumer::HandleTranslationUnit(clang::ASTContext&)</div><div>#11  clang::ParseAST(clang::Sema&, bool, bool) </div><div>#12  clang::FrontendAction::Execute()</div><div>#13  clang::CompilerInstance::ExecuteAction(clang::FrontendAction&)</div><div>#14  clang::ExecuteCompilerInvocation(clang::CompilerInstance*) </div><div>#15  cc1_main(llvm::ArrayRef<char const*>, char const*, void*)</div><div>#16  main </div><div>#17  __libc_start_main</div><div>#18  _start</div><div>Stack dump:</div><div><div>0.<span style="white-space:pre"> </span>Program arguments: bin/clang -cc1 -triple x86_64-grtev4-linux-gnu -emit-obj -O2 -x c c02475.c <br></div><div>1.<span style="white-space:pre">   </span><eof> parser at end of file</div><div>2.<span style="white-space:pre">   </span>Per-module optimization passes</div><div>3.<span style="white-space:pre">      </span>Running pass 'CallGraph Pass Manager' on module 'c02475.c'.</div><div>4.<span style="white-space:pre"> </span>Running pass 'Loop Pass Manager' on function '@loops'</div><div>5.<span style="white-space:pre">       </span>Running pass 'Induction Variable Simplification' on basic block '%2'</div><div><br></div></div></div></div><br><br><div class="gmail_quote"><div dir="ltr">On Wed, Nov 1, 2017 at 12:35 PM Philip Reames via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">FYI - I've gotten a report that this change has introduced a miscompile<br>
in a stage2 clang build.  I'm about to revert this and the following<br>
patch due to lack of time to investigate fully.<br>
<br>
<br>
On 10/31/2017 11:04 AM, Philip Reames via llvm-commits wrote:<br>
> Author: reames<br>
> Date: Tue Oct 31 11:04:57 2017<br>
> New Revision: 317016<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=317016&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=317016&view=rev</a><br>
> Log:<br>
> [IndVarSimplify] Extract wrapper around SE-.isLoopInvariantPredicate [NFC]<br>
><br>
> This an intermediate state, the next patch will re-inline the markLoopInvariantPredicate function to reduce code duplication.<br>
><br>
><br>
> Modified:<br>
>      llvm/trunk/lib/Transforms/Utils/SimplifyIndVar.cpp<br>
><br>
> Modified: llvm/trunk/lib/Transforms/Utils/SimplifyIndVar.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/SimplifyIndVar.cpp?rev=317016&r1=317015&r2=317016&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/SimplifyIndVar.cpp?rev=317016&r1=317015&r2=317016&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/lib/Transforms/Utils/SimplifyIndVar.cpp (original)<br>
> +++ llvm/trunk/lib/Transforms/Utils/SimplifyIndVar.cpp Tue Oct 31 11:04:57 2017<br>
> @@ -83,6 +83,11 @@ namespace {<br>
><br>
>       bool eliminateOverflowIntrinsic(CallInst *CI);<br>
>       bool eliminateIVUser(Instruction *UseInst, Instruction *IVOperand);<br>
> +    bool isCheapLoopInvariantPredicate(ICmpInst::Predicate Pred,<br>
> +           const SCEV *LHS, const SCEV *RHS, const Loop *L,<br>
> +           const SmallDenseMap<const SCEV*, Value*> &FreeExpansions,<br>
> +           ICmpInst::Predicate &InvariantPred,<br>
> +           Value *&LHSV, Value *& RHSV);<br>
>       bool makeIVComparisonInvariant(ICmpInst *ICmp, Value *IVOperand);<br>
>       void eliminateIVComparison(ICmpInst *ICmp, Value *IVOperand);<br>
>       void simplifyIVRemainder(BinaryOperator *Rem, Value *IVOperand,<br>
> @@ -162,6 +167,26 @@ Value *SimplifyIndvar::foldIVUser(Instru<br>
>     return IVSrc;<br>
>   }<br>
><br>
> +bool SimplifyIndvar::isCheapLoopInvariantPredicate(ICmpInst::Predicate Pred,<br>
> +           const SCEV *LHS, const SCEV *RHS, const Loop *L,<br>
> +           const SmallDenseMap<const SCEV*, Value*> &FreeExpansions,<br>
> +           ICmpInst::Predicate &InvariantPred,<br>
> +           Value *&LHSV, Value *& RHSV) {<br>
> +<br>
> +  const SCEV *InvariantLHS, *InvariantRHS;<br>
> +  if (!SE->isLoopInvariantPredicate(Pred, LHS, RHS, L, InvariantPred,<br>
> +                                    InvariantLHS, InvariantRHS))<br>
> +    return false;<br>
> +<br>
> +  // Rewrite the comparison to a loop invariant comparison if it can be done<br>
> +  // cheaply, where cheaply means "we don't need to emit any new<br>
> +  // instructions".<br>
> +  LHSV = FreeExpansions.lookup(InvariantLHS);<br>
> +  RHSV = FreeExpansions.lookup(InvariantRHS);<br>
> +<br>
> +  return (LHSV && RHSV);<br>
> +}<br>
> +<br>
>   bool SimplifyIndvar::makeIVComparisonInvariant(ICmpInst *ICmp,<br>
>                                                  Value *IVOperand) {<br>
>     unsigned IVOperIdx = 0;<br>
> @@ -179,19 +204,9 @@ bool SimplifyIndvar::makeIVComparisonInv<br>
>     const SCEV *S = SE->getSCEVAtScope(ICmp->getOperand(IVOperIdx), ICmpLoop);<br>
>     const SCEV *X = SE->getSCEVAtScope(ICmp->getOperand(1 - IVOperIdx), ICmpLoop);<br>
><br>
> -  ICmpInst::Predicate InvariantPredicate;<br>
> -  const SCEV *InvariantLHS, *InvariantRHS;<br>
> -<br>
>     auto *PN = dyn_cast<PHINode>(IVOperand);<br>
>     if (!PN)<br>
>       return false;<br>
> -  if (!SE->isLoopInvariantPredicate(Pred, S, X, L, InvariantPredicate,<br>
> -                                    InvariantLHS, InvariantRHS))<br>
> -    return false;<br>
> -<br>
> -  // Rewrite the comparison to a loop invariant comparison if it can be done<br>
> -  // cheaply, where cheaply means "we don't need to emit any new<br>
> -  // instructions".<br>
><br>
>     SmallDenseMap<const SCEV*, Value*> CheapExpansions;<br>
>     CheapExpansions[S] = ICmp->getOperand(IVOperIdx);<br>
> @@ -204,17 +219,18 @@ bool SimplifyIndvar::makeIVComparisonInv<br>
>       const SCEV *IncomingS = SE->getSCEV(Incoming);<br>
>       CheapExpansions[IncomingS] = Incoming;<br>
>     }<br>
> -  Value *NewLHS = CheapExpansions[InvariantLHS];<br>
> -  Value *NewRHS = CheapExpansions[InvariantRHS];<br>
><br>
> -  if (!NewLHS || !NewRHS)<br>
> -    // We could not find an existing value to replace either LHS or RHS.<br>
> -    // Generating new instructions has subtler tradeoffs, so avoid doing that<br>
> -    // for now.<br>
> +  ICmpInst::Predicate NewPred;<br>
> +  Value *NewLHS = nullptr, *NewRHS = nullptr;<br>
> +<br>
> +  if (!isCheapLoopInvariantPredicate(Pred, S, X, L, CheapExpansions,<br>
> +                                     NewPred, NewLHS, NewRHS))<br>
>       return false;<br>
> +<br>
> +  assert(NewLHS && NewRHS);<br>
><br>
>     DEBUG(dbgs() << "INDVARS: Simplified comparison: " << *ICmp << '\n');<br>
> -  ICmp->setPredicate(InvariantPredicate);<br>
> +  ICmp->setPredicate(NewPred);<br>
>     ICmp->setOperand(0, NewLHS);<br>
>     ICmp->setOperand(1, NewRHS);<br>
>     return true;<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" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><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" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div>