[PATCH] D85130: [SCEVExpander] [PowerPC] clear scev rewriter before deleting instructions.

[ChenZheng via Phabricator via llvm-commits]

shchenz created this revision.
shchenz added reviewers: hfinkel, lebedev.ri, samparker, mkazantsev, jsji, PowerPC.
Herald added subscribers: llvm-commits, steven.zhang, wuzish, asbirlea, javed.absar, george.burgess.iv, kbarton, hiraditya, nemanjai.
Herald added a project: LLVM.
shchenz requested review of this revision.

In PowerPC pass `PPCLoopInstrFormPrep`, we forget to clear scev rewriter `SCEVE` before we call `RecursivelyDeleteTriviallyDeadInstructions`, this will cause some our downstream tests fail if we turn on assertion. 
The failure is like:

  While deleting: i32* %uglygep16341635
  An asserting value handle still pointed to this value!
  UNREACHABLE executed at ../lib/IR/Value.cpp:1012!
  
   #9 0x0000000012902514 llvm::llvm_unreachable_internal(char const*, char const*, unsigned int) 
  #10 0x000000001218b458 llvm::ValueHandleBase::ValueIsDeleted(llvm::Value*) 
  #11 0x000000001218b95c llvm::Value::~Value() 
  #12 0x00000000120e3580 llvm::Instruction::~Instruction() 
  #14 0x00000000120e37dc llvm::Instruction::eraseFromParent() 
  #15 0x0000000012a2c1e8 llvm::RecursivelyDeleteTriviallyDeadInstructions(llvm::SmallVectorImpl<llvm::WeakTrackingVH>&, llvm::TargetLibraryInfo const*, llvm::MemorySSAUpdater*
  , std::function<void (llvm::Value*)>) 
  #16 0x0000000012a2bad4 llvm::RecursivelyDeleteTriviallyDeadInstructions(llvm::Value*, llvm::TargetLibraryInfo const*, llvm::MemorySSAUpdater*, std::function<void (llvm::Valu
  e*)>)
  #17 0x00000000112b4dc0 (anonymous namespace)::PPCLoopInstrFormPrep::rewriteLoadStores(llvm::Loop*, (anonymous namespace)::Bucket&, llvm::SmallSet<llvm::BasicBlock*, 16u, std
  ::less<llvm::BasicBlock*> >&, (anonymous namespace)::InstrForm)
  #18 0x00000000112b1b48 (anonymous namespace)::PPCLoopInstrFormPrep::runOnFunction(llvm::Function&)
  #19 0x00000000121204ec llvm::FPPassManager::runOnFunction(llvm::Function&) 
  #20 0x0000000012128650 llvm::FPPassManager::runOnModule(llvm::Module&)
  #21 0x0000000012120d84 llvm::legacy::PassManagerImpl::run(llvm::Module&)
  #22 0x0000000012128c5c llvm::legacy::PassManager::run(llvm::Module&)
  #23 0x0000000012c22634 clang::EmitBackendOutput(clang::DiagnosticsEngine&, clang::HeaderSearchOptions const&, clang::CodeGenOptions const&, clang::TargetOptions const&, clan

According to `https://reviews.llvm.org/rL109478`, before deleting instructions, scev expander's client should clear the containers which contain the values associating with AssertingVH handles.

IndVarSimplify and LSR should be ok. PowerPC `PPCLoopInstrFormPrep` misses this. Seems HardwareLoops also misses this?

The downstream test is very big and I failed to narrow down it to a small case. Since this patch is quite straightforward, I left it without test. Hope this is ok.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85130

Files:
  llvm/lib/Target/PowerPC/PPCLoopInstrFormPrep.cpp


Index: llvm/lib/Target/PowerPC/PPCLoopInstrFormPrep.cpp
===================================================================
--- llvm/lib/Target/PowerPC/PPCLoopInstrFormPrep.cpp
+++ llvm/lib/Target/PowerPC/PPCLoopInstrFormPrep.cpp
@@ -606,6 +606,10 @@
       NewBasePtr = NewPHI;
   }
 
+  // Clear the rewriter cache, because values that are in the rewriter's cache
+  // can be deleted below, causing the AssertingVH in the cache to trigger.
+  SCEVE.clear();
+
   if (Instruction *IDel = dyn_cast<Instruction>(BasePtr))
     BBChanged.insert(IDel->getParent());
   BasePtr->replaceAllUsesWith(NewBasePtr);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D85130.282601.patch
Type: text/x-patch
Size: 608 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200803/2ab15e05/attachment.bin>