[PATCH v2 1/1] utils: Fix segfault in flattencfg
Tom Stellard
tom at stellard.net
Mon Aug 11 02:22:16 PDT 2014
On Sun, Aug 10, 2014 at 05:47:35PM -0400, Jan Vesely wrote:
> 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>
> From 509d65e62475b7d34966c38b0177648482a31ce8 Mon Sep 17 00:00:00 2001
> From: Jan Vesely <jan.vesely at rutgers.edu>
> Date: Sat, 9 Aug 2014 16:41:29 -0400
> Subject: [PATCH v3 1/1] utils: Fix segfault in flattencfg
>
> v2: continue iterating through the rest of the bb
> use for loop
>
> v3: initialize FlattenCFG pass in ScalarOps
> add test
>
> Signed-off-by: Jan Vesely <jan.vesely at rutgers.edu>
> ---
> lib/Transforms/Scalar/Scalar.cpp | 1 +
> lib/Transforms/Utils/FlattenCFG.cpp | 9 +++++----
> test/Transforms/Util/flattencfg.ll | 23 +++++++++++++++++++++++
> 3 files changed, 29 insertions(+), 4 deletions(-)
> create mode 100644 test/Transforms/Util/flattencfg.ll
>
> diff --git a/lib/Transforms/Scalar/Scalar.cpp b/lib/Transforms/Scalar/Scalar.cpp
> index 12df676..4bebb42 100644
> --- a/lib/Transforms/Scalar/Scalar.cpp
> +++ b/lib/Transforms/Scalar/Scalar.cpp
> @@ -38,6 +38,7 @@ void llvm::initializeScalarOpts(PassRegistry &Registry) {
> initializeDSEPass(Registry);
> initializeGVNPass(Registry);
> initializeEarlyCSEPass(Registry);
> + initializeFlattenCFGPassPass(Registry);
> initializeIndVarSimplifyPass(Registry);
This change should be committed separately. I'll let Matt or someone
else comment on the rest of the patch.
-Tom
> initializeJumpThreadingPass(Registry);
> initializeLICMPass(Registry);
> 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;
> }
> diff --git a/test/Transforms/Util/flattencfg.ll b/test/Transforms/Util/flattencfg.ll
> new file mode 100644
> index 0000000..949d7a2
> --- /dev/null
> +++ b/test/Transforms/Util/flattencfg.ll
> @@ -0,0 +1,23 @@
> +; RUN: opt -flattencfg -S < %s | FileCheck %s
> +
> +; Function Attrs: nounwind
> +; CHECK-LABEL: test_1
> +define void @test_1(i32 %in_a) #0 {
> +entry:
> + %cmp0 = icmp eq i32 %in_a, -1
> + %cmp1 = icmp ne i32 %in_a, 0
> + %cond0 = and i1 %cmp0, %cmp1
> + br i1 %cond0, label %b0, label %b1
> +
> +b0: ; preds = %entry
> + %cmp2 = icmp eq i32 %in_a, 0
> + %cmp3 = icmp ne i32 %in_a, 1
> + %cond1 = or i1 %cmp2, %cmp3
> + br i1 %cond1, label %exit, label %b1
> +
> +b1: ; preds = %entry, %b0
> + br label %exit
> +
> +exit: ; preds = %entry, %b0, %b1
> + ret void
> +}
> --
> 1.9.3
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list