[llvm] r244879 - [LIR] Start leveraging the fundamental guarantees of a loop in

Chandler Carruth via llvm-commits llvm-commits at lists.llvm.org
Thu Aug 13 14:40:29 PDT 2015


Gah. This looks very much like there are underlying bugs in the loop pass
manager. We could probably end up hitting them at any time in the future.

Sadly, this was just a drive-by cleanup while I had the code in my head.
Thanks for reverting, I'll probably leave it reverted until I or someone
else has time to really figure out why we aren't preserving loop canonical
form.

-Chandler

On Thu, Aug 13, 2015 at 4:12 AM Renato Golin via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> Hi Chandler,
>
> I got this failure on my prototype bot:
>
>
> http://buildmaster.tcwglab.linaro.org/builders/clang-cmake-aarch64-prototype/builds/87
>
> clang-3.8:
> /home/rengolin/devel/buildslave/clang-cmake-aarch64-prototype/llvm/lib/Transforms/Scalar/LoopIdiomRecognize.cpp:836:
> bool {anonymous}::LoopIdiomRecognize::recognizePopcount(): Assertion
> `CurLoop->isLoopSimplifyForm() && "Loop passes require simplified
> form!"' failed.
> 0  clang-3.8 0x00000000013613c4
> llvm::sys::PrintStackTrace(llvm::raw_ostream&) + 80
> Stack dump:
> 0.      Program arguments:
>
> /home/rengolin/devel/buildslave/clang-cmake-aarch64-prototype/stage1.install/bin/clang-3.8
> -cc1 -triple aarch64-unknown-linux-gnu -emit-obj -disable-free
> -main-file-name 2004-03-15-IndirectGoto.c -mrelocation-model static
> -mthread-model posix -fmath-errno -masm-verbose -mconstructor-aliases
> -fuse-init-array -target-cpu cortex-a57 -target-feature +neon
> -target-feature +crc -target-feature +crypto -target-abi aapcs
> -dwarf-column-info -coverage-file
>
> /home/rengolin/devel/buildslave/clang-cmake-aarch64-prototype/test/sandbox/build/SingleSource/Regression/C/Output/2004-03-15-IndirectGoto.llvm.o
> -resource-dir
> /home/rengolin/devel/buildslave/clang-cmake-aarch64-prototype/stage1.install/bin/../lib/clang/3.8.0
> -I
> /home/rengolin/devel/buildslave/clang-cmake-aarch64-prototype/test/sandbox/build/SingleSource/Regression/C
> -I
> /home/rengolin/devel/buildslave/clang-cmake-aarch64-prototype/test/test-suite/SingleSource/Regression/C
> -I
> /home/rengolin/devel/buildslave/clang-cmake-aarch64-prototype/test/test-suite/include
> -I ../../../include -D _GNU_SOURCE -D __STDC_LIMIT_MACROS -D NDEBUG
> -internal-isystem /usr/local/include -internal-isystem
>
> /home/rengolin/devel/buildslave/clang-cmake-aarch64-prototype/stage1.install/bin/../lib/clang/3.8.0/include
> -internal-externc-isystem /usr/include/aarch64-linux-gnu
> -internal-externc-isystem /include -internal-externc-isystem
> /usr/include -O3 -fdebug-compilation-dir
>
> /home/rengolin/devel/buildslave/clang-cmake-aarch64-prototype/test/sandbox/build/SingleSource/Regression/C
> -ferror-limit 19 -fmessage-length 0 -mstackrealign
> -fallow-half-arguments-and-returns -fno-signed-char -fobjc-runtime=gcc
> -fdiagnostics-show-option -vectorize-loops -vectorize-slp -o
> Output/2004-03-15-IndirectGoto.llvm.o -x c
>
> /home/rengolin/devel/buildslave/clang-cmake-aarch64-prototype/test/test-suite/SingleSource/Regression/C/2004-03-15-IndirectGoto.c
> 1.      <eof> parser at end of file
> 2.      Per-module optimization passes
> 3.      Running pass 'CallGraph Pass Manager' on module
>
> '/home/rengolin/devel/buildslave/clang-cmake-aarch64-prototype/test/test-suite/SingleSource/Regression/C/2004-03-15-IndirectGoto.c'.
> 4.      Running pass 'Loop Pass Manager' on function '@main'
> 5.      Running pass 'Recognize loop idioms' on basic block '%L3'
> clang-3.8: error: unable to execute command: Aborted
> clang-3.8: error: clang frontend command failed due to signal (use -v
> to see invocation)
>
> 244880 still has the same problem...
>
> cheers,
> --renato
>
> On 13 August 2015 at 10:56, Chandler Carruth via llvm-commits
> <llvm-commits at lists.llvm.org> wrote:
> > Author: chandlerc
> > Date: Thu Aug 13 04:56:20 2015
> > New Revision: 244879
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=244879&view=rev
> > Log:
> > [LIR] Start leveraging the fundamental guarantees of a loop in
> > simplified form to remove redundant checks and simplify the code for
> > popcount recognition. We don't actually need to handle all of these
> > cases.
> >
> > I've left a FIXME for one in particular until I finish inspecting to
> > make sure we don't actually *rely* on the predicate in any way.
> >
> > Modified:
> >     llvm/trunk/lib/Transforms/Scalar/LoopIdiomRecognize.cpp
> >
> > Modified: llvm/trunk/lib/Transforms/Scalar/LoopIdiomRecognize.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/LoopIdiomRecognize.cpp?rev=244879&r1=244878&r2=244879&view=diff
> >
> ==============================================================================
> > --- llvm/trunk/lib/Transforms/Scalar/LoopIdiomRecognize.cpp (original)
> > +++ llvm/trunk/lib/Transforms/Scalar/LoopIdiomRecognize.cpp Thu Aug 13
> 04:56:20 2015
> > @@ -830,29 +830,32 @@ bool LoopIdiomRecognize::recognizePopcou
> >    // non-compact loop. Therefore, recognizing popcount idiom only makes
> sense
> >    // in a compact loop.
> >
> > -  // Give up if the loop has multiple blocks or multiple backedges.
> > -  if (CurLoop->getNumBackEdges() != 1 || CurLoop->getNumBlocks() != 1)
> > +  assert(CurLoop->isLoopSimplifyForm() &&
> > +         "Loop passes require simplified form!");
> > +
> > +  // Give up if the loop has multiple blocks.
> > +  if (CurLoop->getNumBlocks() != 1)
> >      return false;
> >
> > -  BasicBlock *LoopBody = *(CurLoop->block_begin());
> > -  if (LoopBody->size() >= 20) {
> > -    // The loop is too big, bail out.
> > +  // If the loop is too big, bail out.
> > +  BasicBlock &LoopBB = *CurLoop->getHeader();
> > +  if (LoopBB.size() >= 20)
> >      return false;
> > -  }
> >
> >    // It should have a preheader containing nothing but an unconditional
> branch.
> > -  BasicBlock *PH = CurLoop->getLoopPreheader();
> > -  if (!PH)
> > -    return false;
> > -  if (&PH->front() != PH->getTerminator())
> > +  BasicBlock &PH = *CurLoop->getLoopPreheader();
> > +  if (&PH.front() != PH.getTerminator())
> >      return false;
> > -  auto *EntryBI = dyn_cast<BranchInst>(PH->getTerminator());
> > +  // FIXME: Technically, it shouldn't matter what instruction we use as
> > +  // a terminator, the only property needed is the definition of a
> preheader:
> > +  // a single loop predecessor whose only successor is the loop header.
> > +  auto *EntryBI = dyn_cast<BranchInst>(PH.getTerminator());
> >    if (!EntryBI || EntryBI->isConditional())
> >      return false;
> >
> >    // It should have a precondition block where the generated popcount
> instrinsic
> >    // function can be inserted.
> > -  auto *PreCondBB = PH->getSinglePredecessor();
> > +  auto *PreCondBB = PH.getSinglePredecessor();
> >    if (!PreCondBB)
> >      return false;
> >    auto *PreCondBI = dyn_cast<BranchInst>(PreCondBB->getTerminator());
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
> _______________________________________________
> llvm-commits mailing list
> 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/20150813/4491e217/attachment.html>


More information about the llvm-commits mailing list