[PATCH v2 1/1] utils: Fix segfault in flattencfg

[Jan Vesely]

On Sat, 2014-08-09 at 16:26 -0400, Tom Stellard wrote:
> On 08/09/2014 03:52 PM, Jan Vesely wrote:
> > On Fri, 2014-08-08 at 14:18 -0700, Matt Arsenault wrote:
> >> On Aug 8, 2014, at 1:41 PM, Jan Vesely <jan.vesely at rutgers.edu> wrote:
> >>
> >>> v2: continue iterating through the rest of the bb
> >>>     use for loop
> >>>
> >>> Signed-off-by: Jan Vesely <jan.vesely at rutgers.edu>
> >>> ---
> >>> lib/Transforms/Utils/FlattenCFG.cpp | 9 +++++----
> >>> 1 file changed, 5 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/lib/Transforms/Utils/FlattenCFG.cpp b/lib/Transforms/Utils/FlattenCFG.cpp
> >>> index 51ead40..4eb3e3d 100644
> >>> --- a/lib/Transforms/Utils/FlattenCFG.cpp
> >>> +++ b/lib/Transforms/Utils/FlattenCFG.cpp
> >>> @@ -238,9 +238,13 @@ bool FlattenCFGOpt::FlattenParallelAndOr(BasicBlock *BB, IRBuilder<> &Builder,
> >>>      // Do branch inversion.
> >>>      BasicBlock *CurrBlock = LastCondBlock;
> >>>      bool EverChanged = false;
> >>> -    while (1) {
> >>> +    for (;CurrBlock != FirstCondBlock;
> >>> +          CurrBlock = CurrBlock->getSinglePredecessor()) {
> >>>        BranchInst *BI = dyn_cast<BranchInst>(CurrBlock->getTerminator());
> >>>        CmpInst *CI = dyn_cast<CmpInst>(BI->getCondition());
> >>> +      if (!CI)
> >>> +        continue;
> >>> +
> >>>        CmpInst::Predicate Predicate = CI->getPredicate();
> >>>        // Canonicalize icmp_ne -> icmp_eq, fcmp_one -> fcmp_oeq
> >>>        if ((Predicate == CmpInst::ICMP_NE) || (Predicate == CmpInst::FCMP_ONE)) {
> >>> @@ -248,9 +252,6 @@ bool FlattenCFGOpt::FlattenParallelAndOr(BasicBlock *BB, IRBuilder<> &Builder,
> >>>          BI->swapSuccessors();
> >>>          EverChanged = true;
> >>>        }
> >>> -      if (CurrBlock == FirstCondBlock)
> >>> -        break;
> >>> -      CurrBlock = CurrBlock->getSinglePredecessor();
> >>>      }
> >>>      return EverChanged;
> >>>    }
> >>
> >> Needs tests
> >
> > sorry, I misunderstood what you meant by test the first time.
> >
> > I wanted to add a flattencfg pass specific test but ran into problems
> >
> > is there an easy way to run only the flattencfg pass?
> > opt -pass_name works only for analytical passes,
> > and llc ... -stop-after=flattencfg reports pass not registered.
> >
> 
> You should be able to run any pass using opt.  Did you look at the 
> output of opt -help-hidden ?
> 
> Actually, flattencfg doens't show up for me in the help options.
> This may be because it doesn't get initialized with the other passes in
> lib/Transforms/Scalar/Scalar.cpp.  Try adding the initialize call there 
> and see if that makes it work with opt.

this worked, thank you. v3 is attached.

the test is a simplified situation from the mad_sat implementation.
the crash occurs at the end of b0.

jan

> 
> -Tom
> 
> 
> > I guess I'll have to add a codegen test.
> >
> > I also found the parallel{or,and}ifcollapse tests, that are marked XFAIL
> > for unrelated reason. Should I try to fix those or add a new test?
> >
> > jan
> >
> 

-- 
Jan Vesely <jan.vesely at rutgers.edu>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-utils-Fix-segfault-in-flattencfg.patch
Type: text/x-patch
Size: 3338 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140810/11b0306d/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140810/11b0306d/attachment.sig>