[PATCH] D88460: Strlen loop idiom recognition
Eli Friedman via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Oct 15 14:46:20 PDT 2020
efriedma added inline comments.
================
Comment at: llvm/lib/Transforms/Scalar/LoopIdiomRecognize.cpp:1691
+ }
+ if (!ResInst)
+ return false;
----------------
anjankgk wrote:
> efriedma wrote:
> > anjankgk wrote:
> > > efriedma wrote:
> > > > anjankgk wrote:
> > > > > efriedma wrote:
> > > > > > anjankgk wrote:
> > > > > > > efriedma wrote:
> > > > > > > > I'm not sure how ResInst is connected to the rest of this transform; why are we transforming the operand of some random PHI node?
> > > > > > > I added a comment for clarity -
> > > > > > > // Finally, we find the phi node that corresponds to the distance between the
> > > > > > > // pointers and replace it's uses by the call to strlen in the transformed
> > > > > > > // code.
> > > > > > >
> > > > > > The patch doesn't appear to prove that anywhere.
> > > > > I am getting the `sub` instruction saved in ResInst and replacing that in line# 1689. Is that something that you prefer to be done in a different way?
> > > > How do you know it's a sub instruction?
> > > I agree. The current code is actually just looking for the phi node from the predecessor (`LoopExitBB`) block. I think, I should add a code to ensure that is actually a `sub` instruction. However, I am afraid that that could again lead to a pattern-matching kind of code which was initially posted.
> > What happens if we ignore ResInst, and rewrite LCSSAPhi instead? That still eliminates the inner loop; later passes should optimize the resulting arithmetic.
> I tried a few things, but I am not sure how to do that.
> This is the i/p BB:
>
> ```
> for.end: ; preds = %for.inc
> %incdec.ptr.lcssa = phi i8* [ %incdec.ptr, %for.inc ]
> %sub.ptr.lhs.cast = ptrtoint i8* %incdec.ptr.lcssa to i64
> %sub.ptr.rhs.cast = ptrtoint i8* %Str to i64
> %sub.ptr.sub = sub i64 %sub.ptr.lhs.cast, %sub.ptr.rhs.cast
> br label %cleanup
> ```
> return type of strlen is i64 (same as ResInt which is %sub.ptr.sub), as compared to the type of lcssa instrn (%incdec.ptr.lcssa) which is a i8*. I am not clear how I can rewrite lcssa with the strlen as suggested.
`%incdec.ptr.lcssa` is going to be equivalent to something like `getelementptr i8, i8* %Str, i64 %strlen`. Speaking in generic terms, replace `%Str` with the start value of LCSSAEv. Add the two values together with SCEV, and use SCEVExpander to materialize it.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D88460/new/
https://reviews.llvm.org/D88460
More information about the llvm-commits
mailing list