[llvm] r347934 - [SCEV] Guard movement of insertion point for loop-invariants

via llvm-commits llvm-commits at lists.llvm.org
Wed Dec 5 22:54:40 PST 2018


Thanks David!

To confirm, I get the same crash here running it locally.

-Warren

From: David Jones <dlj at google.com>
Sent: Wednesday, December 5, 2018 10:25 PM
To: Ristow, Warren <warren.ristow at sony.com>
Cc: llvm-commits at lists.llvm.org; Reid Kleckner <rnk at google.com>
Subject: Re: [llvm] r347934 - [SCEV] Guard movement of insertion point for loop-invariants

I decided to start over with a different failing target, and managed to get a pretty small test case through a combination of iterative manual reduction and creduce (I un-shortened a few names while I was trimming out the last bits I could by hand):


$ ./clang -x c++ -c -o /dev/null -O3 - <<EOF
struct ReallyBadIterator {
  int b;
  ReallyBadIterator(int c) : b(c) {}
  void operator+(long);
};
struct {
  int size_;
  unsigned size() { return size_; }
} really_bad_vector;

char *scribble(char *x);

int mystery_value;

void h() {
  char *buf;
  int as = (unsigned)really_bad_vector.size_ / mystery_value, at;
  for (; mystery_value;) {
    int au = at + as;
    ReallyBadIterator i = really_bad_vector.size();
    i + au;
    buf = scribble(buf);
    at = au;
  }
}
EOF

Stack dump:
0.         Program arguments: /usr/local/google/home/dlj/tmp/bug-r347934.5/clang -cc1 -triple x86_64-grtev4-linux-gnu -emit-obj -disable-free -disable-llvm-verifier -discard-value-names -main-file-name - -mrelocation-model static -mthread-model posix -fmath-errno -masm-verbose -mconstructor-aliases -munwind-tables -fuse-init-array -target-cpu x86-64 -dwarf-column-info -debugger-tuning=gdb -momit-leaf-frame-pointer -coverage-notes-file /dev/null.gcno -resource-dir /usr/local/google/home/dlj/tmp/lib/clang/google3-trunk -internal-isystem /usr/lib/gcc/x86_64-linux-gnu/7.3.0/../../../../include/c++/7.3.0 -internal-isystem /usr/lib/gcc/x86_64-linux-gnu/7.3.0/../../../../include/x86_64-linux-gnu/c++/7.3.0 -internal-isystem /usr/lib/gcc/x86_64-linux-gnu/7.3.0/../../../../include/x86_64-linux-gnu/c++/7.3.0 -internal-isystem /usr/lib/gcc/x86_64-linux-gnu/7.3.0/../../../../include/c++/7.3.0/backward -internal-isystem /usr/local/include -internal-isystem /usr/local/google/home/dlj/tmp/lib/clang/google3-trunk/include -internal-externc-isystem /usr/include/x86_64-linux-gnu -internal-externc-isystem /include -internal-externc-isystem /usr/include -O3 -fdeprecated-macro -fdebug-compilation-dir /usr/local/google/home/dlj/tmp/bug-r347934.5 -ferror-limit 19 -fmessage-length 211 -fobjc-runtime=gcc -fcxx-exceptions -fexceptions -fdiagnostics-show-option -fcolor-diagnostics -vectorize-loops -vectorize-slp -o /dev/null -x c++ - -faddrsig
1.         <eof> parser at end of file
2.         Per-module optimization passes
3.         Running pass 'CallGraph Pass Manager' on module '-'.
4.         Running pass 'Combine redundant instructions' on function '@_Z1hv'
#0 0x0000560bad809b58 llvm::sys::RunSignalHandlers() /proc/self/cwd/third_party/llvm/llvm/lib/Support/Signals.cpp:68:18
#1 0x0000560bad80bc56 SignalHandler(int) /proc/self/cwd/third_party/llvm/llvm/lib/Support/Unix/Signals.inc:358:1
#2 0x00007ff8455269a0 __restore_rt (/usr/grte/v4/lib64/libpthread.so.0+0xf9a0)
#3 0x0000560bad20b5ff llvm::Use::get() const /proc/self/cwd/third_party/llvm/llvm/include/llvm/IR/Use.h:108:31
#4 0x0000560bad20b5ff llvm::PHINode::getOperand(unsigned int) const /proc/self/cwd/third_party/llvm/llvm/include/llvm/IR/Instructions.h:2714:0
#5 0x0000560bad20b5ff llvm::PHINode::getIncomingValue(unsigned int) const /proc/self/cwd/third_party/llvm/llvm/include/llvm/IR/Instructions.h:2603:0
#6 0x0000560bad20b5ff llvm::InstCombiner::visitPHINode(llvm::PHINode&) /proc/self/cwd/third_party/llvm/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp:1238:0
#7 0x0000560bad1da7f4 llvm::InstCombiner::run() /proc/self/cwd/third_party/llvm/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp:3192:22
#8 0x0000560bad1dc1c4 combineInstructionsOverFunction(llvm::Function&, llvm::InstCombineWorklist&, llvm::AAResults*, llvm::AssumptionCache&, llvm::TargetLibraryInfo&, llvm::DominatorTree&, llvm::OptimizationRemarkEmitter&, bool, llvm::LoopInfo*) /proc/self/cwd/third_party/llvm/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp:3424:13
#9 0x0000560bad1dc56f llvm::InstructionCombiningPass::runOnFunction(llvm::Function&) /proc/self/cwd/third_party/llvm/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp:3485:1
#10 0x0000560bad72b78c llvm::FPPassManager::runOnFunction(llvm::Function&) /proc/self/cwd/third_party/llvm/llvm/lib/IR/LegacyPassManager.cpp:1645:11
#11 0x0000560bad542bc0 (anonymous namespace)::CGPassManager::RunPassOnSCC(llvm::Pass*, llvm::CallGraphSCC&, llvm::CallGraph&, bool&, bool&) /proc/self/cwd/third_party/llvm/llvm/lib/Analysis/CallGraphSCCPass.cpp:178:25
#12 0x0000560bad542bc0 (anonymous namespace)::CGPassManager::RunAllPassesOnSCC(llvm::CallGraphSCC&, llvm::CallGraph&, bool&) /proc/self/cwd/third_party/llvm/llvm/lib/Analysis/CallGraphSCCPass.cpp:442:0
#13 0x0000560bad542bc0 (anonymous namespace)::CGPassManager::runOnModule(llvm::Module&) /proc/self/cwd/third_party/llvm/llvm/lib/Analysis/CallGraphSCCPass.cpp:498:0
#14 0x0000560bad72bf15 (anonymous namespace)::MPPassManager::runOnModule(llvm::Module&) /proc/self/cwd/third_party/llvm/llvm/lib/IR/LegacyPassManager.cpp:1744:27
#15 0x0000560bad72bf15 llvm::legacy::PassManagerImpl::run(llvm::Module&) /proc/self/cwd/third_party/llvm/llvm/lib/IR/LegacyPassManager.cpp:1857:0
#16 0x0000560baad3e598 (anonymous namespace)::EmitAssemblyHelper::EmitAssembly(clang::BackendAction, std::unique_ptr<llvm::raw_pwrite_stream, std::default_delete<llvm::raw_pwrite_stream> >) /proc/self/cwd/third_party/llvm/llvm/tools/clang/lib/CodeGen/BackendUtil.cpp:868:3
#17 0x0000560baad3e598 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> >) /proc/self/cwd/third_party/llvm/llvm/tools/clang/lib/CodeGen/BackendUtil.cpp:1299:0
#18 0x0000560baad152d2 std::unique_ptr<llvm::raw_pwrite_stream, std::default_delete<llvm::raw_pwrite_stream> >::~unique_ptr() /proc/self/cwd/third_party/crosstool/v18/stable/toolchain/bin/../lib/gcc/x86_64-grtev4-linux-gnu/4.9.x-google/../../../../x86_64-grtev4-linux-gnu/include/c++/4.9.x-google/bits/unique_ptr.h:235:6
#19 0x0000560baad152d2 clang::BackendConsumer::HandleTranslationUnit(clang::ASTContext&) /proc/self/cwd/third_party/llvm/llvm/tools/clang/lib/CodeGen/CodeGenAction.cpp:293:0
#20 0x0000560bab73a853 __gnu_cxx::__normal_iterator<std::unique_ptr<clang::TemplateInstantiationCallback, std::default_delete<clang::TemplateInstantiationCallback> >*, std::vector<std::unique_ptr<clang::TemplateInstantiationCallback, std::default_delete<clang::TemplateInstantiationCallback> >, std::allocator<std::unique_ptr<clang::TemplateInstantiationCallback, std::default_delete<clang::TemplateInstantiationCallback> > > > >::__normal_iterator(std::unique_ptr<clang::TemplateInstantiationCallback, std::default_delete<clang::TemplateInstantiationCallback> >* const&) /proc/self/cwd/third_party/crosstool/v18/stable/toolchain/bin/../lib/gcc/x86_64-grtev4-linux-gnu/4.9.x-google/../../../../x86_64-grtev4-linux-gnu/include/c++/4.9.x-google/bits/stl_iterator.h:729:20
#21 0x0000560bab73a853 std::vector<std::unique_ptr<clang::TemplateInstantiationCallback, std::default_delete<clang::TemplateInstantiationCallback> >, std::allocator<std::unique_ptr<clang::TemplateInstantiationCallback, std::default_delete<clang::TemplateInstantiationCallback> > > >::begin() /proc/self/cwd/third_party/crosstool/v18/stable/toolchain/bin/../lib/gcc/x86_64-grtev4-linux-gnu/4.9.x-google/../../../../x86_64-grtev4-linux-gnu/include/c++/4.9.x-google/bits/stl_vector.h:597:0
#22 0x0000560bab73a853 void clang::finalize<std::vector<std::unique_ptr<clang::TemplateInstantiationCallback, std::default_delete<clang::TemplateInstantiationCallback> >, std::allocator<std::unique_ptr<clang::TemplateInstantiationCallback, std::default_delete<clang::TemplateInstantiationCallback> > > > >(std::vector<std::unique_ptr<clang::TemplateInstantiationCallback, std::default_delete<clang::TemplateInstantiationCallback> >, std::allocator<std::unique_ptr<clang::TemplateInstantiationCallback, std::default_delete<clang::TemplateInstantiationCallback> > > >&, clang::Sema const&) /proc/self/cwd/third_party/llvm/llvm/tools/clang/include/clang/Sema/TemplateInstCallback.h:55:0
#23 0x0000560bab73a853 clang::ParseAST(clang::Sema&, bool, bool) /proc/self/cwd/third_party/llvm/llvm/tools/clang/lib/Parse/ParseAST.cpp:177:0
#24 0x0000560bab583b6a clang::FrontendAction::Execute() /proc/self/cwd/third_party/llvm/llvm/tools/clang/lib/Frontend/FrontendAction.cpp:921:10
#25 0x0000560bab40d848 clang::CompilerInstance::ExecuteAction(clang::FrontendAction&) /proc/self/cwd/third_party/llvm/llvm/tools/clang/lib/Frontend/CompilerInstance.cpp:969:11
#26 0x0000560baaadccf2 clang::ExecuteCompilerInvocation(clang::CompilerInstance*) /proc/self/cwd/third_party/llvm/llvm/tools/clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp:267:25
#27 0x0000560baaacc205 cc1_main(llvm::ArrayRef<char const*>, char const*, void*) /proc/self/cwd/third_party/llvm/llvm/tools/clang/tools/driver/cc1_main.cpp:219:13
#28 0x0000560baaada07a ExecuteCC1Tool(llvm::ArrayRef<char const*>, llvm::StringRef) /proc/self/cwd/third_party/llvm/llvm/tools/clang/tools/driver/driver.cpp:310:12
#29 0x0000560baaada07a main /proc/self/cwd/third_party/llvm/llvm/tools/clang/tools/driver/driver.cpp:382:0
#30 0x00007ff845294bbd __libc_start_main (/usr/grte/v4/lib64/libc.so.6+0x38bbd)
#31 0x0000560baaacbb69 _start /usr/grte/v4/debug-src/src/csu/../sysdeps/x86_64/start.S:121:0
clang: error: unable to execute command: Segmentation fault
clang: error: clang frontend command failed due to signal (use -v to see invocation)
clang version google3-trunk (trunk r347980)
Target: x86_64-grtev4-linux-gnu
Thread model: posix
InstalledDir: /usr/local/google/home/dlj/tmp/bug-r347934.5/.


On Wed, Dec 5, 2018 at 3:59 PM <warren.ristow at sony.com<mailto:warren.ristow at sony.com>> wrote:
> 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.
>
> Sorry about the revert, and thanks for your help.

Sorry for the trouble in the first place.  Thanks for looking further!  It’s very much appreciated.

-Warren

From: David Jones <dlj at google.com<mailto:dlj at google.com>>
Sent: Wednesday, December 5, 2018 3:17 PM
To: Ristow, Warren <warren.ristow at sony.com<mailto:warren.ristow at sony.com>>
Cc: llvm-commits at lists.llvm.org<mailto:llvm-commits at lists.llvm.org>; Reid Kleckner <rnk at google.com<mailto:rnk at google.com>>
Subject: Re: [llvm] r347934 - [SCEV] Guard movement of insertion point for loop-invariants

On Wed, Dec 5, 2018 at 2:56 PM <warren.ristow at sony.com<mailto:warren.ristow at sony.com>> wrote:
Hi David,

[+ Reid]

> ...
> 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.
>
> 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.)

Sorry to hear about that.  I definitely won't be able to author a timely fix, so please do go ahead and revert it.

OK... sorry for bringing out the big hammer. :-/

FYI: Committed revision 348426.



As it happens, while looking at the history of this SCEV div-by-0 area, in prep for a follow-up patch:
https://reviews.llvm.org/D55232

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:

  ‘’’
  Reverts r289412. It caused an OOB PHI operand access in instcombine when
  ASan is enabled. Reduction in progress.
  ‘’’

> 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.)

Possibly Reid has access to a reduced test-case from back then (or can create one now)?

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!


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.

Sorry about the revert, and thanks for your help.

--dlj


Thanks,
-Warren

From: David Jones <dlj at google.com<mailto:dlj at google.com>>
Sent: Wednesday, December 5, 2018 2:25 PM
To: Ristow, Warren <warren.ristow at sony.com<mailto:warren.ristow at sony.com>>
Cc: llvm-commits at lists.llvm.org<mailto:llvm-commits at lists.llvm.org>
Subject: Re: [llvm] r347934 - [SCEV] Guard movement of insertion point for loop-invariants

Hi Warren --

I've observed several Clang crashes on SEGV, which bisect to this change.

The crashes generally look something like this:

$ opt -O2 -o /dev/null bugpoint-reduced-simplified.ll
Stack dump:
0.         Program arguments: opt -O2 -o /dev/null bugpoint-reduced-simplified.ll
1.         Running pass 'CallGraph Pass Manager' on module 'bugpoint-reduced-simplified.ll'.
2.         Running pass 'Combine redundant instructions' on function '@_Z[redacted]'
#0 0x0000558aa2221628 llvm::sys::RunSignalHandlers() (opt+0x1f3b628)
#1 0x0000558aa2223746 SignalHandler(int) (opt+0x1f3d746)
#2 0x00007f5def60a9a0 __restore_rt (/usr/grte/v4/lib64/libpthread.so.0+0xf9a0)
#3 0x0000558aa1bbec7f llvm::InstCombiner::visitPHINode(llvm::PHINode&) (opt+0x18d8c7f)
#4 0x0000558aa1b8db24 llvm::InstCombiner::run() (opt+0x18a7b24)
#5 0x0000558aa1b8f504 combineInstructionsOverFunction(llvm::Function&, llvm::InstCombineWorklist&, llvm::AAResults*, llvm::AssumptionCache&, llvm::TargetLibraryInfo&, llvm::DominatorTree&, llvm::OptimizationRemarkEmitter&, bool, llvm::LoopInfo*) (opt+0x18a9504)
#6 0x0000558aa1b8f8af llvm::InstructionCombiningPass::runOnFunction(llvm::Function&) (opt+0x18a98af)
#7 0x0000558aa207d2fc llvm::FPPassManager::runOnFunction(llvm::Function&) (opt+0x1d972fc)
#8 0x0000558aa1f062e0 (anonymous namespace)::CGPassManager::runOnModule(llvm::Module&) (opt+0x1c202e0)
#9 0x0000558aa207da85 llvm::legacy::PassManagerImpl::run(llvm::Module&) (opt+0x1d97a85)
#10 0x0000558aa0a8900b main (opt+0x7a300b)
#11 0x00007f5def378bbd __libc_start_main (/usr/grte/v4/lib64/libc.so.6+0x38bbd)
#12 0x0000558aa0a740e9 _start /usr/grte/v4/debug-src/src/csu/../sysdeps/x86_64/start.S:121:0
[1]    17713 segmentation fault  opt -O2 -o /dev/null bugpoint-reduced-simplified.ll


gdb's backtrace is a bit more useful:

#0  0x000055555958b1d4 in llvm::Use::get (this=<optimized out>) at third_party/llvm/llvm/include/llvm/IR/Use.h:108
#1  llvm::PHINode::getOperand (this=<optimized out>, i_nocapture=4294967295) at third_party/llvm/llvm/include/llvm/IR/Instructions.h:2714
#2  llvm::PHINode::getIncomingValue (this=<optimized out>, i=4294967295) at third_party/llvm/llvm/include/llvm/IR/Instructions.h:2603
#3  0x000055555c41abba in llvm::InstCombiner::visitPHINode (this=0x7ffff4e3b970, PN=...) at third_party/llvm/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp:1238
#4  0x000055555c378489 in llvm::InstCombiner::run (this=<optimized out>) at third_party/llvm/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp:3192
#5  0x000055555c37b6d2 in combineInstructionsOverFunction (F=..., Worklist=..., AA=0x60600002b7c0, AC=..., TLI=..., DT=..., ORE=..., ExpensiveCombines=<optimized out>, LI=<optimized out>)
    at third_party/llvm/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp:3424
#6  0x000055555c37be37 in llvm::InstructionCombiningPass::runOnFunction (this=<optimized out>, F=...) at third_party/llvm/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp:3483
#7  0x000055555d39b609 in llvm::FPPassManager::runOnFunction (this=<optimized out>, F=...) at third_party/llvm/llvm/lib/IR/LegacyPassManager.cpp:1644
#8  0x000055555ce8f11b in (anonymous namespace)::CGPassManager::RunPassOnSCC (this=<optimized out>, P=0x614000009a40, CurSCC=..., CG=..., CallGraphUpToDate=<optimized out>, DevirtualizedCall=<optimized out>)
    at third_party/llvm/llvm/lib/Analysis/CallGraphSCCPass.cpp:178
#9  (anonymous namespace)::CGPassManager::RunAllPassesOnSCC (this=<optimized out>, CurSCC=..., CG=..., DevirtualizedCall=<optimized out>) at third_party/llvm/llvm/lib/Analysis/CallGraphSCCPass.cpp:442
#10 (anonymous namespace)::CGPassManager::runOnModule (this=0x614000009840, M=...) at third_party/llvm/llvm/lib/Analysis/CallGraphSCCPass.cpp:498
#11 0x000055555d39ca7a in (anonymous namespace)::MPPassManager::runOnModule (this=<optimized out>, M=...) at third_party/llvm/llvm/lib/IR/LegacyPassManager.cpp:1744
#12 llvm::legacy::PassManagerImpl::run (this=0x619000020f50, M=...) at third_party/llvm/llvm/lib/IR/LegacyPassManager.cpp:1857
#13 0x00005555588bfb58 in main (argc=<optimized out>, argv=<optimized out>) at third_party/llvm/llvm/tools/opt/opt.cpp:828



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.)

The stack trace pretty clearly points to an unchecked out-of-range return value here:

https://git.llvm.org/klaus/llvm/blob/master/lib/Transforms/InstCombine/InstCombinePHI.cpp#L-1238

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.

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.)

--David Jones

On Thu, Nov 29, 2018 at 4:05 PM Warren Ristow via llvm-commits <llvm-commits at lists.llvm.org<mailto:llvm-commits at lists.llvm.org>> wrote:
Author: wristow
Date: Thu Nov 29 16:02:54 2018
New Revision: 347934

URL: http://llvm.org/viewvc/llvm-project?rev=347934&view=rev
Log:
[SCEV] Guard movement of insertion point for loop-invariants

r320789 suppressed moving the insertion point of SCEV expressions with
dev/rem operations to the loop header in non-loop-invariant situations.
This, and similar, hoisting is also unsafe in the loop-invariant case,
since there may be a guard against a zero denominator. This is an
adjustment to the fix of r320789 to suppress the movement even in the
loop-invariant case.

This fixes PR30806.

Differential Revision: https://reviews.llvm.org/D54713

Added:
    llvm/trunk/test/Transforms/LoopVectorize/pr30806.ll
Modified:
    llvm/trunk/lib/Analysis/ScalarEvolutionExpander.cpp

Modified: llvm/trunk/lib/Analysis/ScalarEvolutionExpander.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/ScalarEvolutionExpander.cpp?rev=347934&r1=347933&r2=347934&view=diff
==============================================================================
--- llvm/trunk/lib/Analysis/ScalarEvolutionExpander.cpp (original)
+++ llvm/trunk/lib/Analysis/ScalarEvolutionExpander.cpp Thu Nov 29 16:02:54 2018
@@ -1732,49 +1732,50 @@ Value *SCEVExpander::expand(const SCEV *
   // Compute an insertion point for this SCEV object. Hoist the instructions
   // as far out in the loop nest as possible.
   Instruction *InsertPt = &*Builder.GetInsertPoint();
-  for (Loop *L = SE.LI.getLoopFor(Builder.GetInsertBlock());;
-       L = L->getParentLoop())
-    if (SE.isLoopInvariant(S, L)) {
-      if (!L) break;
-      if (BasicBlock *Preheader = L->getLoopPreheader())
-        InsertPt = Preheader->getTerminator();
-      else {
-        // LSR sets the insertion point for AddRec start/step values to the
-        // block start to simplify value reuse, even though it's an invalid
-        // position. SCEVExpander must correct for this in all cases.
-        InsertPt = &*L->getHeader()->getFirstInsertionPt();
-      }
-    } else {
-      // We can move insertion point only if there is no div or rem operations
-      // otherwise we are risky to move it over the check for zero denominator.
-      auto SafeToHoist = [](const SCEV *S) {
-        return !SCEVExprContains(S, [](const SCEV *S) {
-                  if (const auto *D = dyn_cast<SCEVUDivExpr>(S)) {
-                    if (const auto *SC = dyn_cast<SCEVConstant>(D->getRHS()))
-                      // Division by non-zero constants can be hoisted.
-                      return SC->getValue()->isZero();
-                    // All other divisions should not be moved as they may be
-                    // divisions by zero and should be kept within the
-                    // conditions of the surrounding loops that guard their
-                    // execution (see PR35406).
-                    return true;
-                  }
-                  return false;
-                });
-      };
-      // If the SCEV is computable at this level, insert it into the header
-      // after the PHIs (and after any other instructions that we've inserted
-      // there) so that it is guaranteed to dominate any user inside the loop.
-      if (L && SE.hasComputableLoopEvolution(S, L) && !PostIncLoops.count(L) &&
-          SafeToHoist(S))
-        InsertPt = &*L->getHeader()->getFirstInsertionPt();
-      while (InsertPt->getIterator() != Builder.GetInsertPoint() &&
-             (isInsertedInstruction(InsertPt) ||
-              isa<DbgInfoIntrinsic>(InsertPt))) {
-        InsertPt = &*std::next(InsertPt->getIterator());
+
+  // We can move insertion point only if there is no div or rem operations
+  // otherwise we are risky to move it over the check for zero denominator.
+  auto SafeToHoist = [](const SCEV *S) {
+    return !SCEVExprContains(S, [](const SCEV *S) {
+              if (const auto *D = dyn_cast<SCEVUDivExpr>(S)) {
+                if (const auto *SC = dyn_cast<SCEVConstant>(D->getRHS()))
+                  // Division by non-zero constants can be hoisted.
+                  return SC->getValue()->isZero();
+                // All other divisions should not be moved as they may be
+                // divisions by zero and should be kept within the
+                // conditions of the surrounding loops that guard their
+                // execution (see PR35406).
+                return true;
+              }
+              return false;
+            });
+  };
+  if (SafeToHoist(S)) {
+    for (Loop *L = SE.LI.getLoopFor(Builder.GetInsertBlock());;
+         L = L->getParentLoop()) {
+      if (SE.isLoopInvariant(S, L)) {
+        if (!L) break;
+        if (BasicBlock *Preheader = L->getLoopPreheader())
+          InsertPt = Preheader->getTerminator();
+        else
+          // LSR sets the insertion point for AddRec start/step values to the
+          // block start to simplify value reuse, even though it's an invalid
+          // position. SCEVExpander must correct for this in all cases.
+          InsertPt = &*L->getHeader()->getFirstInsertionPt();
+      } else {
+        // If the SCEV is computable at this level, insert it into the header
+        // after the PHIs (and after any other instructions that we've inserted
+        // there) so that it is guaranteed to dominate any user inside the loop.
+        if (L && SE.hasComputableLoopEvolution(S, L) && !PostIncLoops.count(L))
+          InsertPt = &*L->getHeader()->getFirstInsertionPt();
+        while (InsertPt->getIterator() != Builder.GetInsertPoint() &&
+               (isInsertedInstruction(InsertPt) ||
+                isa<DbgInfoIntrinsic>(InsertPt)))
+          InsertPt = &*std::next(InsertPt->getIterator());
+        break;
       }
-      break;
     }
+  }

   // Check to see if we already expanded this here.
   auto I = InsertedExpressions.find(std::make_pair(S, InsertPt));

Added: llvm/trunk/test/Transforms/LoopVectorize/pr30806.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/LoopVectorize/pr30806.ll?rev=347934&view=auto
==============================================================================
--- llvm/trunk/test/Transforms/LoopVectorize/pr30806.ll (added)
+++ llvm/trunk/test/Transforms/LoopVectorize/pr30806.ll Thu Nov 29 16:02:54 2018
@@ -0,0 +1,65 @@
+; RUN: opt -loop-vectorize -S < %s 2>&1 | FileCheck %s
+
+; Produced from test-case:
+;
+; void testGuardedInnerLoop(uint32_t *ptr, uint32_t denom, uint32_t numer, uint32_t outer_lim)
+; {
+;   for(uint32_t outer_i = 0; outer_i < outer_lim; ++outer_i) {
+;     if (denom > 0) {
+;       const uint32_t lim = numer / denom;
+;
+;       for (uint32_t i = 0; i < lim; ++i)
+;         ptr[i] = 1;
+;     }
+;   }
+; }
+
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128-ni:1"
+target triple = "x86_64-unknown-linux-gnu"
+
+define void @testGuardedInnerLoop(i32* %ptr, i32 %denom, i32 %numer, i32 %outer_lim) {
+entry:
+  %cmp1 = icmp eq i32 %outer_lim, 0
+  br i1 %cmp1, label %exit, label %loop1.preheader
+
+; Verify that a 'udiv' does not appear between the 'loop1.preheader' label, and
+; whatever label comes next.
+loop1.preheader:
+; CHECK-LABEL: loop1.preheader:
+; CHECK-NOT: udiv
+; CHECK-LABEL: :
+  br label %loop1
+
+loop1:
+  %outer_i = phi i32 [ %inc1, %loop2.exit ], [ 0, %loop1.preheader ]
+  %0 = add i32 %denom, -1
+  %1 = icmp ult i32 %0, %numer
+  br i1 %1, label %loop2.preheader, label %loop2.exit
+
+; Verify that a 'udiv' does appear between the 'loop2.preheader' label, and
+; whatever label comes next.
+loop2.preheader:
+; CHECK-LABEL: loop2.preheader:
+; CHECK: udiv
+; CHECK-LABEL: :
+  %lim = udiv i32 %numer, %denom
+  %2 = zext i32 %lim to i64
+  br label %loop2
+
+loop2:
+  %indvar.loop2 = phi i64 [ 0, %loop2.preheader ], [ %indvar.loop2.next, %loop2 ]
+  %arrayidx = getelementptr inbounds i32, i32* %ptr, i64 %indvar.loop2
+  store i32 1, i32* %arrayidx, align 4
+  %indvar.loop2.next = add nuw nsw i64 %indvar.loop2, 1
+  %cmp2 = icmp ult i64 %indvar.loop2.next, %2
+  br i1 %cmp2, label %loop2, label %loop2.exit
+
+loop2.exit:
+  %inc1 = add nuw i32 %outer_i, 1
+  %exitcond = icmp eq i32 %inc1, %outer_lim
+  br i1 %exitcond, label %exit, label %loop1
+
+exit:
+  ret void
+}


_______________________________________________
llvm-commits mailing list
llvm-commits at lists.llvm.org<mailto:llvm-commits at lists.llvm.org>
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20181206/0797ce3d/attachment.html>


More information about the llvm-commits mailing list