[PATCH] D103007: [LoopUtils] Avoid expanding complicated SCEVNAry when rewriteLoopExitValues

[guopeilin via Phabricator via llvm-commits]

guopeilin added a comment.

Actually, this transformation doesn't take too much time. However the IR it generated is very long.  And this will have a great effect on the further pass. More specifically, the expanding SCEV which replaces the variable `%add.i.2.i`  contains thousands of operands.
So if you use `opt -indvars -S`, you can see the program keeps printing the IR all the time.
Besides, I use `clang -O3 test.cpp` and find that the program gets a segment fault with a stack dump like the followings:

  ...
  llvm::sys::RunSignalHandlers()
  SignalHandler(int)
  llvm::IntegerType::classof(llvm::Type const*)
  llvm::isa_impl<llvm::IntegerType, llvm::Type, void>::doit(llvm::Type const&)
  llvm::isa_impl_cl<llvm::IntegerType, llvm::Type const*>::doit(llvm::Type const*) 
  llvm::isa_impl_wrap<llvm::IntegerType, llvm::Type const*, llvm::Type const*>::doit(llvm::Type const* const&)
  llvm::isa_impl_wrap<llvm::IntegerType, llvm::Type* const, llvm::Type const*>::doit(llvm::Type* const&) 
  bool llvm::isa<llvm::IntegerType, llvm::Type*>(llvm::Type* const&) 
  llvm::cast_retty<llvm::IntegerType, llvm::Type*>::ret_type llvm::cast<llvm::IntegerType, llvm::Type>(llvm::Type*)
  llvm::EVT::getEVT(llvm::Type*, bool)
  llvm::TargetLoweringBase::getValueType(llvm::DataLayout const&, llvm::Type*, bool) const
  llvm::SelectionDAGBuilder::getValueImpl(llvm::Value const*)
  llvm::SelectionDAGBuilder::getValue(llvm::Value const*)
  llvm::SelectionDAGBuilder::visitSelect(llvm::User const&)
  llvm::SelectionDAGBuilder::visit(unsigned int, llvm::User const&)
  llvm::SelectionDAGBuilder::getValueImpl(llvm::Value const*)
  llvm::SelectionDAGBuilder::getValue(llvm::Value const*)
  llvm::SelectionDAGBuilder::visitSelect(llvm::User const&)
  llvm::SelectionDAGBuilder::visit(unsigned int, llvm::User const&)
  llvm::SelectionDAGBuilder::getValueImpl(llvm::Value const*)
  llvm::SelectionDAGBuilder::getValue(llvm::Value const*)
  llvm::SelectionDAGBuilder::visitBinary(llvm::User const&, unsigned int)
  llvm::SelectionDAGBuilder::visitAdd(llvm::User const&)
  llvm::SelectionDAGBuilder::visit(unsigned int, llvm::User const&) 
  llvm::SelectionDAGBuilder::getValueImpl(llvm::Value const*)
  llvm::SelectionDAGBuilder::getValue(llvm::Value const*)
  llvm::SelectionDAGBuilder::visitICmp(llvm::User const&)
  (handreds of other function visit and getValue and so on)
  ...
  ...
  clang-10: error: unable to execute command: Segmentation fault (core dumped)

>From the stack dump, we can see that during the building SelectionDAG, visit thousands of operands finally exhaust the stack and left no enough space for a new Value. And the incomplete Value finally causes Segment Fault.
In short, any way that tries to visit the operands generated by expanding the extremely complicated SCEV computed in this test case will get stuck. 1. we using `-S` to print the IR will get stuck. 2. Building SelectionDAG by visiting all operands will get stuck.
One more thing is that ISel pass is controlled by `llc`, so I cannot reproduce the segment fault here cause commands `opt -indvars test-case.ll | llc ` doesn't work as I want. This will still get stuck in `opt` cause it wants to generate IR first. It would be greatly appreciated if you have any ideas about this.

As for the magic number, I guess that the key point is that we need to add a case for complicated SCEV. And there might have another way to determine whether a SCEV is complicated rather than counting the operands.

In D103007#2777055 <https://reviews.llvm.org/D103007#2777055>, @lebedev.ri wrote:

> I'm very wary of more magic numbers.
> Could you please specify, is the problem the time the transformation takes,
> or it's effect on some further passes?




Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103007/new/

https://reviews.llvm.org/D103007