[PATCH] D13305: [InstCombine] Remove trivially empty lifetime start/end ranges.
Arnaud A. de Grandmaison via llvm-commits
llvm-commits at lists.llvm.org
Thu Oct 1 01:24:03 PDT 2015
aadg added a comment.
I intended to catch another trivially empty ranges in a follow-up patch: the non strictly nested ones you refer to. The goal was to quickly fix an issue (PR24598), then improve the lifetime markers cleanup.
I think we should teach this cleanup to address less trivial cases, especially the one where the start dominates the end (garanteed by design) and the end post-dominate the start and the alloca is not used in between. I think this would catch lots of cases. But that's for another patch.
And the loop unroller probably needs to learn how not to mess to badly with the lifetime markers :).
Thanks for the reviews !
I'll fix and push this patch.
================
Comment at: lib/Transforms/InstCombine/InstCombineCalls.cpp:1398
@@ +1397,3 @@
+ // immediately followed by an end (ignoring debuginfo in between).
+ BasicBlock::iterator BI = II, BE = II->getParent()->end();
+ for (++BI; BI != BE; ++BI) {
----------------
chandlerc wrote:
> These should be loop variables.
I agree on the principle, but BI needs to be incremented before entering the loop.
================
Comment at: test/Transforms/InstCombine/lifetime.ll:2
@@ +1,3 @@
+; RUN: opt < %s -instcombine -S | FileCheck %s
+; RUN: opt < %s -instcombine -S | grep lifetime | count 6
+
----------------
chandlerc wrote:
> manmanren wrote:
> > It is probably better to just check the 6 occurrences with a single RUN command.
> This already is checked by the FileCheck line. I think this can just be deleted.
Those 2 RUN lines were a very clumsy way of making sure the "if" basic block has been emptied, and the "else" was not modified. One RUN line is enough indeed, and I will add 2 checks:
CHECK: if:
CHECK-NEXT: br label %fin
http://reviews.llvm.org/D13305
More information about the llvm-commits
mailing list