<div dir="ltr">Thank you for the revert and test. Sorry about the bug.<br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, May 3, 2019 at 11:36 AM Evgenii Stepanov <<a href="mailto:eugeni.stepanov@gmail.com">eugeni.stepanov@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">$ bin/llc 2.bc<br>
<br>
attempting to delete unknown value kind<br>
UNREACHABLE executed at ../llvm-project/llvm/lib/IR/Value.cpp:120!<br>
Stack dump:<br>
0. Program arguments: bin/llc 2.bc<br>
1. Running pass 'Function Pass Manager' on module '2.bc'.<br>
2. Running pass 'CodeGen Prepare' on function<br>
'@_ZN12_GLOBAL__N_117CommandLineParser23ParseCommandLineOptionsEiPKPKcN4llvm9StringRefEPNS5_11raw_ostreamE'<br>
 #0 0x0000000002218c9d PrintStackTraceSignalHandler(void*) (bin/llc+0x2218c9d)<br>
 #1 0x00000000022169ce llvm::sys::RunSignalHandlers() (bin/llc+0x22169ce)<br>
 #2 0x0000000002218e58 SignalHandler(int) (bin/llc+0x2218e58)<br>
 #3 0x00007f7b98c1b0c0 __restore_rt<br>
(/lib/x86_64-linux-gnu/libpthread.so.0+0x110c0)<br>
 #4 0x00007f7b97ddefcf raise (/lib/x86_64-linux-gnu/libc.so.6+0x32fcf)<br>
 #5 0x00007f7b97de03fa abort (/lib/x86_64-linux-gnu/libc.so.6+0x343fa)<br>
 #6 0x00000000021b31c7 (bin/llc+0x21b31c7)<br>
 #7 0x00000000020df85a (bin/llc+0x20df85a)<br>
 #8 0x00000000018f75bc (anonymous<br>
namespace)::CodeGenPrepare::runOnFunction(llvm::Function&)<br>
(bin/llc+0x18f75bc)<br>
 #9 0x0000000002097496<br>
llvm::FPPassManager::runOnFunction(llvm::Function&)<br>
(bin/llc+0x2097496)<br>
#10 0x00000000020977a3 llvm::FPPassManager::runOnModule(llvm::Module&)<br>
(bin/llc+0x20977a3)<br>
#11 0x0000000002097dec<br>
llvm::legacy::PassManagerImpl::run(llvm::Module&) (bin/llc+0x2097dec)<br>
#12 0x000000000174a9a6 compileModule(char**, llvm::LLVMContext&)<br>
(bin/llc+0x174a9a6)<br>
#13 0x000000000174845d main (bin/llc+0x174845d)<br>
#14 0x00007f7b97dcc2b1 __libc_start_main<br>
(/lib/x86_64-linux-gnu/libc.so.6+0x202b1)<br>
#15 0x000000000174802a _start (bin/llc+0x174802a)<br>
Aborted<br>
<br>
On Fri, May 3, 2019 at 10:28 AM Evgenii Stepanov<br>
<<a href="mailto:eugeni.stepanov@gmail.com" target="_blank">eugeni.stepanov@gmail.com</a>> wrote:<br>
><br>
> Confirmed in a local build. Reverting. I'll prepare a test case later.<br>
><br>
> On Fri, May 3, 2019 at 10:13 AM Evgenii Stepanov<br>
> <<a href="mailto:eugeni.stepanov@gmail.com" target="_blank">eugeni.stepanov@gmail.com</a>> wrote:<br>
> ><br>
> > Hi,<br>
> ><br>
> > this crash might be caused by this commit:<br>
> ><br>
> > <a href="http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-bootstrap-ubsan/builds/12660/steps/build%20clang%2Fubsan/logs/stdio" rel="noreferrer" target="_blank">http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-bootstrap-ubsan/builds/12660/steps/build%20clang%2Fubsan/logs/stdio</a><br>
> ><br>
> > 0. Program arguments:<br>
> > /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build0/bin/clang-9<br>
> > -cc1 -triple x86_64-unknown-linux-gnu -emit-obj -disable-free<br>
> > -main-file-name CommandLine.cpp -mrelocation-model pic -pic-level 2<br>
> > -mthread-model posix -mdisable-fp-elim -fmath-errno -masm-verbose<br>
> > -mconstructor-aliases -munwind-tables -fuse-init-array -target-cpu<br>
> > x86-64 -dwarf-column-info -debug-info-kind=line-tables-only<br>
> > -dwarf-version=4 -debugger-tuning=gdb -momit-leaf-frame-pointer<br>
> > -ffunction-sections -fdata-sections -coverage-notes-file<br>
> > /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build_ubsan/lib/Support/CMakeFiles/LLVMSupport.dir/CommandLine.cpp.gcno<br>
> > -resource-dir /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build0/lib/clang/9.0.0<br>
> > -dependency-file<br>
> > lib/Support/CMakeFiles/LLVMSupport.dir/CommandLine.cpp.o.d<br>
> > -sys-header-deps -MT<br>
> > lib/Support/CMakeFiles/LLVMSupport.dir/CommandLine.cpp.o -D<br>
> > GTEST_HAS_RTTI=0 -D _DEBUG -D _GNU_SOURCE -D __STDC_CONSTANT_MACROS -D<br>
> > __STDC_FORMAT_MACROS -D __STDC_LIMIT_MACROS -I lib/Support -I<br>
> > /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm/lib/Support -I<br>
> > /usr/include/libxml2 -I include -I<br>
> > /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm/include -U NDEBUG<br>
> > -internal-isystem<br>
> > /usr/lib/gcc/x86_64-linux-gnu/6.3.0/../../../../include/c++/6.3.0<br>
> > -internal-isystem<br>
> > /usr/lib/gcc/x86_64-linux-gnu/6.3.0/../../../../include/x86_64-linux-gnu/c++/6.3.0<br>
> > -internal-isystem<br>
> > /usr/lib/gcc/x86_64-linux-gnu/6.3.0/../../../../include/x86_64-linux-gnu/c++/6.3.0<br>
> > -internal-isystem<br>
> > /usr/lib/gcc/x86_64-linux-gnu/6.3.0/../../../../include/c++/6.3.0/backward<br>
> > -internal-isystem /usr/local/include -internal-isystem<br>
> > /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build0/lib/clang/9.0.0/include<br>
> > -internal-externc-isystem /usr/include/x86_64-linux-gnu<br>
> > -internal-externc-isystem /include -internal-externc-isystem<br>
> > /usr/include -O3 -Werror=date-time -Werror=unguarded-availability-new<br>
> > -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual<br>
> > -Wmissing-field-initializers -Wno-long-long -Wimplicit-fallthrough<br>
> > -Wcovered-switch-default -Wno-class-memaccess -Wno-noexcept-type<br>
> > -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wstring-conversion<br>
> > -pedantic -w -std=c++11 -fdeprecated-macro -fdebug-compilation-dir<br>
> > /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build_ubsan<br>
> > -ferror-limit 19 -fmessage-length 0 -fvisibility-inlines-hidden<br>
> > -fsanitize=alignment,array-bounds,bool,builtin,enum,float-cast-overflow,float-divide-by-zero,integer-divide-by-zero,nonnull-attribute,null,object-size,pointer-overflow,return,returns-nonnull-attribute,shift-base,shift-exponent,signed-integer-overflow,unreachable,vla-bound<br>
> > -fsanitize-blacklist=/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm/utils/sanitizers/ubsan_blacklist.txt<br>
> > -fdepfile-entry=/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm/utils/sanitizers/ubsan_blacklist.txt<br>
> > -fno-rtti -fobjc-runtime=gcc -fdiagnostics-show-option<br>
> > -fcolor-diagnostics -vectorize-loops -vectorize-slp -o<br>
> > lib/Support/CMakeFiles/LLVMSupport.dir/CommandLine.cpp.o -x c++<br>
> > /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm/lib/Support/CommandLine.cpp<br>
> > -faddrsig<br>
> > 1. <eof> parser at end of file<br>
> > 2. Code generation<br>
> > 3. Running pass 'Function Pass Manager' on module<br>
> > '/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm/lib/Support/CommandLine.cpp'.<br>
> > 4. Running pass 'CodeGen Prepare' on function<br>
> > '@_ZN12_GLOBAL__N_117CommandLineParser23ParseCommandLineOptionsEiPKPKcN4llvm9StringRefEPNS5_11raw_ostreamE'<br>
> >  #0 0x000055d2f954521a llvm::sys::PrintStackTrace(llvm::raw_ostream&)<br>
> > (/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build0/bin/clang-9+0x298521a)<br>
> >  #1 0x000055d2f9543175 llvm::sys::RunSignalHandlers()<br>
> > (/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build0/bin/clang-9+0x2983175)<br>
> >  #2 0x000055d2f954328c SignalHandler(int)<br>
> > (/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build0/bin/clang-9+0x298328c)<br>
> >  #3 0x00007f0bd09f40e0 __restore_rt<br>
> > (/lib/x86_64-linux-gnu/libpthread.so.0+0x110e0)<br>
> >  #4 0x000055d2f8fb84f0<br>
> > llvm::ReplaceableMetadataImpl::getIfExists(llvm::Metadata&)<br>
> > (/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build0/bin/clang-9+0x23f84f0)<br>
> >  #5 0x000055d2f8fb8569 llvm::MetadataTracking::untrack(void*,<br>
> > llvm::Metadata&)<br>
> > (/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build0/bin/clang-9+0x23f8569)<br>
> >  #6 0x000055d2f8f742b4 llvm::Instruction::~Instruction()<br>
> > (/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build0/bin/clang-9+0x23b42b4)<br>
> >  #7 0x000055d2f8fef140 llvm::Value::deleteValue()<br>
> > (/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build0/bin/clang-9+0x242f140)<br>
> >  #8 0x000055d2f8b53944 (anonymous<br>
> > namespace)::CodeGenPrepare::runOnFunction(llvm::Function&)<br>
> > (.part.1278) (/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build0/bin/clang-9+0x1f93944)<br>
> >  #9 0x000055d2f8fa8710<br>
> > llvm::FPPassManager::runOnFunction(llvm::Function&)<br>
> > (/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build0/bin/clang-9+0x23e8710)<br>
> > #10 0x000055d2f8fa892c llvm::FPPassManager::runOnModule(llvm::Module&)<br>
> > (/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build0/bin/clang-9+0x23e892c)<br>
> > #11 0x000055d2f8fa9454<br>
> > llvm::legacy::PassManagerImpl::run(llvm::Module&)<br>
> > (/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build0/bin/clang-9+0x23e9454)<br>
> > #12 0x000055d2f973ed37 (anonymous<br>
> > namespace)::EmitAssemblyHelper::EmitAssembly(clang::BackendAction,<br>
> > std::unique_ptr<llvm::raw_pwrite_stream,<br>
> > std::default_delete<llvm::raw_pwrite_stream> >)<br>
> > (/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build0/bin/clang-9+0x2b7ed37)<br>
> > #13 0x000055d2f974063c<br>
> > clang::EmitBackendOutput(clang::DiagnosticsEngine&,<br>
> > clang::HeaderSearchOptions const&, clang::CodeGenOptions const&,<br>
> > clang::TargetOptions const&, clang::LangOptions const&,<br>
> > llvm::DataLayout const&, llvm::Module*, clang::BackendAction,<br>
> > std::unique_ptr<llvm::raw_pwrite_stream,<br>
> > std::default_delete<llvm::raw_pwrite_stream> >)<br>
> > (/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build0/bin/clang-9+0x2b8063c)<br>
> > #14 0x000055d2fa11f628<br>
> > clang::BackendConsumer::HandleTranslationUnit(clang::ASTContext&)<br>
> > (/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build0/bin/clang-9+0x355f628)<br>
> > #15 0x000055d2fa982319 clang::ParseAST(clang::Sema&, bool, bool)<br>
> > (/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build0/bin/clang-9+0x3dc2319)<br>
> > #16 0x000055d2fa11e8b9 clang::CodeGenAction::ExecuteAction()<br>
> > (/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build0/bin/clang-9+0x355e8b9)<br>
> > #17 0x000055d2f9bcfbbe clang::FrontendAction::Execute()<br>
> > (/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build0/bin/clang-9+0x300fbbe)<br>
> > #18 0x000055d2f9b97b96<br>
> > clang::CompilerInstance::ExecuteAction(clang::FrontendAction&)<br>
> > (/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build0/bin/clang-9+0x2fd7b96)<br>
> > #19 0x000055d2f9c73271<br>
> > clang::ExecuteCompilerInvocation(clang::CompilerInstance*)<br>
> > (/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build0/bin/clang-9+0x30b3271)<br>
> > #20 0x000055d2f7a5db77 cc1_main(llvm::ArrayRef<char const*>, char<br>
> > const*, void*) (/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build0/bin/clang-9+0xe9db77)<br>
> > #21 0x000055d2f79c812d main<br>
> > (/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build0/bin/clang-9+0xe0812d)<br>
> ><br>
> ><br>
> > On Fri, May 3, 2019 at 6:06 AM Sanjay Patel via llvm-commits<br>
> > <<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>> wrote:<br>
> > ><br>
> > > Author: spatel<br>
> > > Date: Fri May  3 06:09:18 2019<br>
> > > New Revision: 359879<br>
> > ><br>
> > > URL: <a href="http://llvm.org/viewvc/llvm-project?rev=359879&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=359879&view=rev</a><br>
> > > Log:<br>
> > > [CodeGenPrepare] limit overflow intrinsic matching to a single basic block<br>
> > ><br>
> > > Using/updating a dominator tree to match math overflow patterns may be very<br>
> > > expensive in compile-time (because of the way CGP uses a DT), so just handle<br>
> > > the single-block case.<br>
> > ><br>
> > > Also, we were restarting the iterator loops when doing the overflow intrinsic<br>
> > > transforms by marking the dominator tree for update. That was done to prevent<br>
> > > iterating over a removed instruction. But we can postpone the deletion using<br>
> > > the existing "RemovedInsts" structure, and that means we don't need to update<br>
> > > the DT.<br>
> > ><br>
> > > See post-commit thread for rL354298 for more details:<br>
> > > <a href="http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20190422/646276.html" rel="noreferrer" target="_blank">http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20190422/646276.html</a><br>
> > ><br>
> > > Differential Revision: <a href="https://reviews.llvm.org/D61075" rel="noreferrer" target="_blank">https://reviews.llvm.org/D61075</a><br>
> > ><br>
> > > Modified:<br>
> > >     llvm/trunk/lib/CodeGen/CodeGenPrepare.cpp<br>
> > >     llvm/trunk/test/CodeGen/X86/cgp-usubo.ll<br>
> > >     llvm/trunk/test/Transforms/CodeGenPrepare/X86/optimizeSelect-DT.ll<br>
> > >     llvm/trunk/test/Transforms/CodeGenPrepare/X86/overflow-intrinsics.ll<br>
> > ><br>
> > > Modified: llvm/trunk/lib/CodeGen/CodeGenPrepare.cpp<br>
> > > URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/CodeGenPrepare.cpp?rev=359879&r1=359878&r2=359879&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/CodeGenPrepare.cpp?rev=359879&r1=359878&r2=359879&view=diff</a><br>
> > > ==============================================================================<br>
> > > --- llvm/trunk/lib/CodeGen/CodeGenPrepare.cpp (original)<br>
> > > +++ llvm/trunk/lib/CodeGen/CodeGenPrepare.cpp Fri May  3 06:09:18 2019<br>
> > > @@ -388,9 +388,9 @@ class TypePromotionTransaction;<br>
> > >      bool tryToSinkFreeOperands(Instruction *I);<br>
> > >      bool replaceMathCmpWithIntrinsic(BinaryOperator *BO, CmpInst *Cmp,<br>
> > >                                       Intrinsic::ID IID);<br>
> > > -    bool optimizeCmp(CmpInst *Cmp, bool &ModifiedDT);<br>
> > > -    bool combineToUSubWithOverflow(CmpInst *Cmp, bool &ModifiedDT);<br>
> > > -    bool combineToUAddWithOverflow(CmpInst *Cmp, bool &ModifiedDT);<br>
> > > +    bool optimizeCmp(CmpInst *Cmp);<br>
> > > +    bool combineToUSubWithOverflow(CmpInst *Cmp);<br>
> > > +    bool combineToUAddWithOverflow(CmpInst *Cmp);<br>
> > >    };<br>
> > ><br>
> > >  } // end anonymous namespace<br>
> > > @@ -484,9 +484,13 @@ bool CodeGenPrepare::runOnFunction(Funct<br>
> > >      if (!LargeOffsetGEPMap.empty())<br>
> > >        MadeChange |= splitLargeGEPOffsets();<br>
> > ><br>
> > > -    // Really free removed instructions during promotion.<br>
> > > -    for (Instruction *I : RemovedInsts)<br>
> > > +    // Really free instructions removed during promotion or kept around to<br>
> > > +    // improve efficiency (see comments in overflow intrinsic transforms).<br>
> > > +    for (Instruction *I : RemovedInsts) {<br>
> > > +      if (I->getParent())<br>
> > > +        I->removeFromParent();<br>
> > >        I->deleteValue();<br>
> > > +    }<br>
> > ><br>
> > >      EverMadeChange |= MadeChange;<br>
> > >      SeenChainsForSExt.clear();<br>
> > > @@ -1177,6 +1181,20 @@ static bool OptimizeNoopCopyExpression(C<br>
> > >  bool CodeGenPrepare::replaceMathCmpWithIntrinsic(BinaryOperator *BO,<br>
> > >                                                   CmpInst *Cmp,<br>
> > >                                                   Intrinsic::ID IID) {<br>
> > > +  if (BO->getParent() != Cmp->getParent()) {<br>
> > > +    // We used to use a dominator tree here to allow multi-block optimization.<br>
> > > +    // But that was problematic because:<br>
> > > +    // 1. It could cause a perf regression by hoisting the math op into the<br>
> > > +    //    critical path.<br>
> > > +    // 2. It could cause a perf regression by creating a value that was live<br>
> > > +    //    across multiple blocks and increasing register pressure.<br>
> > > +    // 3. Use of a dominator tree could cause large compile-time regression.<br>
> > > +    //    This is because we recompute the DT on every change in the main CGP<br>
> > > +    //    run-loop. The recomputing is probably unnecessary in many cases, so if<br>
> > > +    //    that was fixed, using a DT here would be ok.<br>
> > > +    return false;<br>
> > > +  }<br>
> > > +<br>
> > >    // We allow matching the canonical IR (add X, C) back to (usubo X, -C).<br>
> > >    Value *Arg0 = BO->getOperand(0);<br>
> > >    Value *Arg1 = BO->getOperand(1);<br>
> > > @@ -1186,45 +1204,28 @@ bool CodeGenPrepare::replaceMathCmpWithI<br>
> > >      Arg1 = ConstantExpr::getNeg(cast<Constant>(Arg1));<br>
> > >    }<br>
> > ><br>
> > > -  Instruction *InsertPt;<br>
> > > -  if (BO->hasOneUse() && BO->user_back() == Cmp) {<br>
> > > -    // If the math is only used by the compare, insert at the compare to keep<br>
> > > -    // the condition in the same block as its users. (CGP aggressively sinks<br>
> > > -    // compares to help out SDAG.)<br>
> > > -    InsertPt = Cmp;<br>
> > > -  } else {<br>
> > > -    // The math and compare may be independent instructions. Check dominance to<br>
> > > -    // determine the insertion point for the intrinsic.<br>
> > > -    bool MathDominates = getDT(*BO->getFunction()).dominates(BO, Cmp);<br>
> > > -    if (!MathDominates && !getDT(*BO->getFunction()).dominates(Cmp, BO))<br>
> > > -      return false;<br>
> > > -<br>
> > > -    BasicBlock *MathBB = BO->getParent(), *CmpBB = Cmp->getParent();<br>
> > > -    if (MathBB != CmpBB) {<br>
> > > -      // Avoid hoisting an extra op into a dominating block and creating a<br>
> > > -      // potentially longer critical path.<br>
> > > -      if (!MathDominates)<br>
> > > -        return false;<br>
> > > -      // Check that the insertion doesn't create a value that is live across<br>
> > > -      // more than two blocks, so to minimise the increase in register pressure.<br>
> > > -      BasicBlock *Dominator = MathDominates ? MathBB : CmpBB;<br>
> > > -      BasicBlock *Dominated = MathDominates ? CmpBB : MathBB;<br>
> > > -      auto Successors = successors(Dominator);<br>
> > > -      if (llvm::find(Successors, Dominated) == Successors.end())<br>
> > > -        return false;<br>
> > > +  // Insert at the first instruction of the pair.<br>
> > > +  Instruction *InsertPt = nullptr;<br>
> > > +  for (Instruction &Iter : *Cmp->getParent()) {<br>
> > > +    if (&Iter == BO || &Iter == Cmp) {<br>
> > > +      InsertPt = &Iter;<br>
> > > +      break;<br>
> > >      }<br>
> > > -<br>
> > > -    InsertPt = MathDominates ? cast<Instruction>(BO) : cast<Instruction>(Cmp);<br>
> > >    }<br>
> > > +  assert(InsertPt != nullptr && "Parent block did not contain cmp or binop");<br>
> > ><br>
> > >    IRBuilder<> Builder(InsertPt);<br>
> > >    Value *MathOV = Builder.CreateBinaryIntrinsic(IID, Arg0, Arg1);<br>
> > >    Value *Math = Builder.CreateExtractValue(MathOV, 0, "math");<br>
> > >    Value *OV = Builder.CreateExtractValue(MathOV, 1, "ov");<br>
> > > +<br>
> > > +  // Delay the actual removal/deletion of the binop and compare for efficiency.<br>
> > > +  // If we delete those now, we would invalidate the instruction iterator and<br>
> > > +  // trigger dominator tree updates.<br>
> > >    BO->replaceAllUsesWith(Math);<br>
> > >    Cmp->replaceAllUsesWith(OV);<br>
> > > -  BO->eraseFromParent();<br>
> > > -  Cmp->eraseFromParent();<br>
> > > +  RemovedInsts.insert(BO);<br>
> > > +  RemovedInsts.insert(Cmp);<br>
> > >    return true;<br>
> > >  }<br>
> > ><br>
> > > @@ -1260,8 +1261,7 @@ static bool matchUAddWithOverflowConstan<br>
> > ><br>
> > >  /// Try to combine the compare into a call to the llvm.uadd.with.overflow<br>
> > >  /// intrinsic. Return true if any changes were made.<br>
> > > -bool CodeGenPrepare::combineToUAddWithOverflow(CmpInst *Cmp,<br>
> > > -                                               bool &ModifiedDT) {<br>
> > > +bool CodeGenPrepare::combineToUAddWithOverflow(CmpInst *Cmp) {<br>
> > >    Value *A, *B;<br>
> > >    BinaryOperator *Add;<br>
> > >    if (!match(Cmp, m_UAddWithOverflow(m_Value(A), m_Value(B), m_BinOp(Add))))<br>
> > > @@ -1281,13 +1281,10 @@ bool CodeGenPrepare::combineToUAddWithOv<br>
> > >    if (!replaceMathCmpWithIntrinsic(Add, Cmp, Intrinsic::uadd_with_overflow))<br>
> > >      return false;<br>
> > ><br>
> > > -  // Reset callers - do not crash by iterating over a dead instruction.<br>
> > > -  ModifiedDT = true;<br>
> > >    return true;<br>
> > >  }<br>
> > ><br>
> > > -bool CodeGenPrepare::combineToUSubWithOverflow(CmpInst *Cmp,<br>
> > > -                                               bool &ModifiedDT) {<br>
> > > +bool CodeGenPrepare::combineToUSubWithOverflow(CmpInst *Cmp) {<br>
> > >    // We are not expecting non-canonical/degenerate code. Just bail out.<br>
> > >    Value *A = Cmp->getOperand(0), *B = Cmp->getOperand(1);<br>
> > >    if (isa<Constant>(A) && isa<Constant>(B))<br>
> > > @@ -1342,8 +1339,6 @@ bool CodeGenPrepare::combineToUSubWithOv<br>
> > >    if (!replaceMathCmpWithIntrinsic(Sub, Cmp, Intrinsic::usub_with_overflow))<br>
> > >      return false;<br>
> > ><br>
> > > -  // Reset callers - do not crash by iterating over a dead instruction.<br>
> > > -  ModifiedDT = true;<br>
> > >    return true;<br>
> > >  }<br>
> > ><br>
> > > @@ -1413,14 +1408,14 @@ static bool sinkCmpExpression(CmpInst *C<br>
> > >    return MadeChange;<br>
> > >  }<br>
> > ><br>
> > > -bool CodeGenPrepare::optimizeCmp(CmpInst *Cmp, bool &ModifiedDT) {<br>
> > > +bool CodeGenPrepare::optimizeCmp(CmpInst *Cmp) {<br>
> > >    if (sinkCmpExpression(Cmp, *TLI))<br>
> > >      return true;<br>
> > ><br>
> > > -  if (combineToUAddWithOverflow(Cmp, ModifiedDT))<br>
> > > +  if (combineToUAddWithOverflow(Cmp))<br>
> > >      return true;<br>
> > ><br>
> > > -  if (combineToUSubWithOverflow(Cmp, ModifiedDT))<br>
> > > +  if (combineToUSubWithOverflow(Cmp))<br>
> > >      return true;<br>
> > ><br>
> > >    return false;<br>
> > > @@ -6945,7 +6940,7 @@ bool CodeGenPrepare::optimizeInst(Instru<br>
> > >    }<br>
> > ><br>
> > >    if (auto *Cmp = dyn_cast<CmpInst>(I))<br>
> > > -    if (TLI && optimizeCmp(Cmp, ModifiedDT))<br>
> > > +    if (TLI && optimizeCmp(Cmp))<br>
> > >        return true;<br>
> > ><br>
> > >    if (LoadInst *LI = dyn_cast<LoadInst>(I)) {<br>
> > ><br>
> > > Modified: llvm/trunk/test/CodeGen/X86/cgp-usubo.ll<br>
> > > URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/cgp-usubo.ll?rev=359879&r1=359878&r2=359879&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/cgp-usubo.ll?rev=359879&r1=359878&r2=359879&view=diff</a><br>
> > > ==============================================================================<br>
> > > --- llvm/trunk/test/CodeGen/X86/cgp-usubo.ll (original)<br>
> > > +++ llvm/trunk/test/CodeGen/X86/cgp-usubo.ll Fri May  3 06:09:18 2019<br>
> > > @@ -121,7 +121,7 @@ define i1 @usubo_ne_constant0_op1_i32(i3<br>
> > >    ret i1 %ov<br>
> > >  }<br>
> > ><br>
> > > -; Verify insertion point for multi-BB.<br>
> > > +; This used to verify insertion point for multi-BB, but now we just bail out.<br>
> > ><br>
> > >  declare void @call(i1)<br>
> > ><br>
> > > @@ -131,14 +131,17 @@ define i1 @usubo_ult_sub_dominates_i64(i<br>
> > >  ; CHECK-NEXT:    testb $1, %cl<br>
> > >  ; CHECK-NEXT:    je .LBB8_2<br>
> > >  ; CHECK-NEXT:  # %bb.1: # %t<br>
> > > -; CHECK-NEXT:    subq %rsi, %rdi<br>
> > > -; CHECK-NEXT:    setb %al<br>
> > > -; CHECK-NEXT:    movq %rdi, (%rdx)<br>
> > > +; CHECK-NEXT:    movq %rdi, %rax<br>
> > > +; CHECK-NEXT:    subq %rsi, %rax<br>
> > > +; CHECK-NEXT:    movq %rax, (%rdx)<br>
> > >  ; CHECK-NEXT:    testb $1, %cl<br>
> > > -; CHECK-NEXT:    jne .LBB8_3<br>
> > > +; CHECK-NEXT:    je .LBB8_2<br>
> > > +; CHECK-NEXT:  # %bb.3: # %end<br>
> > > +; CHECK-NEXT:    cmpq %rsi, %rdi<br>
> > > +; CHECK-NEXT:    setb %al<br>
> > > +; CHECK-NEXT:    retq<br>
> > >  ; CHECK-NEXT:  .LBB8_2: # %f<br>
> > >  ; CHECK-NEXT:    movl %ecx, %eax<br>
> > > -; CHECK-NEXT:  .LBB8_3: # %end<br>
> > >  ; CHECK-NEXT:    retq<br>
> > >  entry:<br>
> > >    br i1 %cond, label %t, label %f<br>
> > ><br>
> > > Modified: llvm/trunk/test/Transforms/CodeGenPrepare/X86/optimizeSelect-DT.ll<br>
> > > URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/CodeGenPrepare/X86/optimizeSelect-DT.ll?rev=359879&r1=359878&r2=359879&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/CodeGenPrepare/X86/optimizeSelect-DT.ll?rev=359879&r1=359878&r2=359879&view=diff</a><br>
> > > ==============================================================================<br>
> > > --- llvm/trunk/test/Transforms/CodeGenPrepare/X86/optimizeSelect-DT.ll (original)<br>
> > > +++ llvm/trunk/test/Transforms/CodeGenPrepare/X86/optimizeSelect-DT.ll Fri May  3 06:09:18 2019<br>
> > > @@ -14,11 +14,10 @@ define i1 @PR41004(i32 %x, i32 %y, i32 %<br>
> > >  ; CHECK-NEXT:    br label [[SELECT_END]]<br>
> > >  ; CHECK:       select.end:<br>
> > >  ; CHECK-NEXT:    [[MUL:%.*]] = phi i32 [ [[REM]], [[SELECT_TRUE_SINK]] ], [ 0, [[ENTRY:%.*]] ]<br>
> > > -; CHECK-NEXT:    [[TMP0:%.*]] = call { i32, i1 } @llvm.usub.with.overflow.i32(i32 [[T1:%.*]], i32 1)<br>
> > > -; CHECK-NEXT:    [[MATH:%.*]] = extractvalue { i32, i1 } [[TMP0]], 0<br>
> > > -; CHECK-NEXT:    [[OV:%.*]] = extractvalue { i32, i1 } [[TMP0]], 1<br>
> > > -; CHECK-NEXT:    [[ADD:%.*]] = add i32 [[MATH]], [[MUL]]<br>
> > > -; CHECK-NEXT:    ret i1 [[OV]]<br>
> > > +; CHECK-NEXT:    [[NEG:%.*]] = add i32 [[T1:%.*]], -1<br>
> > > +; CHECK-NEXT:    [[ADD:%.*]] = add i32 [[NEG]], [[MUL]]<br>
> > > +; CHECK-NEXT:    [[TOBOOL:%.*]] = icmp eq i32 [[T1]], 0<br>
> > > +; CHECK-NEXT:    ret i1 [[TOBOOL]]<br>
> > >  ;<br>
> > >  entry:<br>
> > >    %rem = srem i32 %x, 2<br>
> > ><br>
> > > Modified: llvm/trunk/test/Transforms/CodeGenPrepare/X86/overflow-intrinsics.ll<br>
> > > URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/CodeGenPrepare/X86/overflow-intrinsics.ll?rev=359879&r1=359878&r2=359879&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/CodeGenPrepare/X86/overflow-intrinsics.ll?rev=359879&r1=359878&r2=359879&view=diff</a><br>
> > > ==============================================================================<br>
> > > --- llvm/trunk/test/Transforms/CodeGenPrepare/X86/overflow-intrinsics.ll (original)<br>
> > > +++ llvm/trunk/test/Transforms/CodeGenPrepare/X86/overflow-intrinsics.ll Fri May  3 06:09:18 2019<br>
> > > @@ -47,15 +47,16 @@ define i64 @uaddo3(i64 %a, i64 %b) nounw<br>
> > >    ret i64 %Q<br>
> > >  }<br>
> > ><br>
> > > +; TODO? CGP sinks the compare before we have a chance to form the overflow intrinsic.<br>
> > > +<br>
> > >  define i64 @uaddo4(i64 %a, i64 %b, i1 %c) nounwind ssp {<br>
> > >  ; CHECK-LABEL: @uaddo4(<br>
> > >  ; CHECK-NEXT:  entry:<br>
> > > +; CHECK-NEXT:    [[ADD:%.*]] = add i64 [[B:%.*]], [[A:%.*]]<br>
> > >  ; CHECK-NEXT:    br i1 [[C:%.*]], label [[NEXT:%.*]], label [[EXIT:%.*]]<br>
> > >  ; CHECK:       next:<br>
> > > -; CHECK-NEXT:    [[TMP0:%.*]] = call { i64, i1 } @llvm.uadd.with.overflow.i64(i64 [[B:%.*]], i64 [[A:%.*]])<br>
> > > -; CHECK-NEXT:    [[MATH:%.*]] = extractvalue { i64, i1 } [[TMP0]], 0<br>
> > > -; CHECK-NEXT:    [[OV:%.*]] = extractvalue { i64, i1 } [[TMP0]], 1<br>
> > > -; CHECK-NEXT:    [[Q:%.*]] = select i1 [[OV]], i64 [[B]], i64 42<br>
> > > +; CHECK-NEXT:    [[TMP0:%.*]] = icmp ugt i64 [[B]], [[ADD]]<br>
> > > +; CHECK-NEXT:    [[Q:%.*]] = select i1 [[TMP0]], i64 [[B]], i64 42<br>
> > >  ; CHECK-NEXT:    ret i64 [[Q]]<br>
> > >  ; CHECK:       exit:<br>
> > >  ; CHECK-NEXT:    ret i64 0<br>
> > > @@ -362,7 +363,7 @@ define i1 @usubo_ne_constant0_op1_i32(i3<br>
> > >    ret i1 %ov<br>
> > >  }<br>
> > ><br>
> > > -; Verify insertion point for multi-BB.<br>
> > > +; This used to verify insertion point for multi-BB, but now we just bail out.<br>
> > ><br>
> > >  declare void @call(i1)<br>
> > ><br>
> > > @@ -371,15 +372,14 @@ define i1 @usubo_ult_sub_dominates_i64(i<br>
> > >  ; CHECK-NEXT:  entry:<br>
> > >  ; CHECK-NEXT:    br i1 [[COND:%.*]], label [[T:%.*]], label [[F:%.*]]<br>
> > >  ; CHECK:       t:<br>
> > > -; CHECK-NEXT:    [[TMP0:%.*]] = call { i64, i1 } @llvm.usub.with.overflow.i64(i64 [[X:%.*]], i64 [[Y:%.*]])<br>
> > > -; CHECK-NEXT:    [[MATH:%.*]] = extractvalue { i64, i1 } [[TMP0]], 0<br>
> > > -; CHECK-NEXT:    [[OV1:%.*]] = extractvalue { i64, i1 } [[TMP0]], 1<br>
> > > -; CHECK-NEXT:    store i64 [[MATH]], i64* [[P:%.*]]<br>
> > > +; CHECK-NEXT:    [[S:%.*]] = sub i64 [[X:%.*]], [[Y:%.*]]<br>
> > > +; CHECK-NEXT:    store i64 [[S]], i64* [[P:%.*]]<br>
> > >  ; CHECK-NEXT:    br i1 [[COND]], label [[END:%.*]], label [[F]]<br>
> > >  ; CHECK:       f:<br>
> > >  ; CHECK-NEXT:    ret i1 [[COND]]<br>
> > >  ; CHECK:       end:<br>
> > > -; CHECK-NEXT:    ret i1 [[OV1]]<br>
> > > +; CHECK-NEXT:    [[OV:%.*]] = icmp ult i64 [[X]], [[Y]]<br>
> > > +; CHECK-NEXT:    ret i1 [[OV]]<br>
> > >  ;<br>
> > >  entry:<br>
> > >    br i1 %cond, label %t, label %f<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="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div>