Sanjoy Das via llvm-dev
2017-Apr-17 04:31 UTC
[llvm-dev] Question on induction variable simplification pass
Hi Pankaj, On April 14, 2017 at 4:55:16 PM, Chawla, Pankaj (pankaj.chawla at intel.com) wrote:> I have attached the IR I got by compiling with -O2. This is just before we widen the IV.Thanks!> To get the backedge taken count info I ran indvars on it and then replaced zext with sext. > > I think regardless of where we decide to add this transformation in the pipeline, it should > try to preserve as much information as it can. This means that we should generate sext > for signed IVs and vice-versa. I believe this is a better approach as it preserves the > information directly in the IR as opposed to relying on ScalarEvolution to deduce it.I'll be happy to review patches making indvars behave better here (i.e. not "break" loop trip counts like this). I don't think the IV is the most relevant bit here though -- it looks like (only a guess) indvars is faltering here: https://github.com/llvm-mirror/llvm/blob/master/lib/Transforms/Scalar/IndVarSimplify.cpp#L2240 and that logic needs to be made smarter to account for how much the RHS of the LFTR'ed exit condition is simplified after extension.> Moving it to a different location can be done separately. > > Do you agree?Sounds good! -- Sanjoy
Chawla, Pankaj via llvm-dev
2017-Apr-17 18:10 UTC
[llvm-dev] Question on induction variable simplification pass
Hi Sanjoy, Thanks for pointing me in the right direction. I am not really familiar with this piece of code. I will study it and then put up a patch for review. Thanks, Pankaj -----Original Message----- From: Sanjoy Das [mailto:sanjoy at playingwithpointers.com] Sent: Sunday, April 16, 2017 9:31 PM To: llvm-dev at lists.llvm.org; Chawla, Pankaj Subject: RE: [llvm-dev] Question on induction variable simplification pass Hi Pankaj, On April 14, 2017 at 4:55:16 PM, Chawla, Pankaj (pankaj.chawla at intel.com) wrote:> I have attached the IR I got by compiling with -O2. This is just before we widen the IV.Thanks!> To get the backedge taken count info I ran indvars on it and then replaced zext with sext. > > I think regardless of where we decide to add this transformation in > the pipeline, it should try to preserve as much information as it can. > This means that we should generate sext for signed IVs and vice-versa. > I believe this is a better approach as it preserves the information directly in the IR as opposed to relying on ScalarEvolution to deduce it.I'll be happy to review patches making indvars behave better here (i.e. not "break" loop trip counts like this). I don't think the IV is the most relevant bit here though -- it looks like (only a guess) indvars is faltering here: https://github.com/llvm-mirror/llvm/blob/master/lib/Transforms/Scalar/IndVarSimplify.cpp#L2240 and that logic needs to be made smarter to account for how much the RHS of the LFTR'ed exit condition is simplified after extension.> Moving it to a different location can be done separately. > > Do you agree?Sounds good! -- Sanjoy
Sanjoy Das via llvm-dev
2017-Apr-17 18:16 UTC
[llvm-dev] Question on induction variable simplification pass
Btw, I just noticed the triple response; sorry about that. Not sure what happened -- possibly a combination of operator error and spotty internet connection. -- Sanjoy On Mon, Apr 17, 2017 at 11:10 AM, Chawla, Pankaj <pankaj.chawla at intel.com> wrote:> Hi Sanjoy, > > Thanks for pointing me in the right direction. I am not really familiar with this piece of code. I will study it and then put up a patch for review. > > Thanks, > Pankaj > > -----Original Message----- > From: Sanjoy Das [mailto:sanjoy at playingwithpointers.com] > Sent: Sunday, April 16, 2017 9:31 PM > To: llvm-dev at lists.llvm.org; Chawla, Pankaj > Subject: RE: [llvm-dev] Question on induction variable simplification pass > > Hi Pankaj, > > On April 14, 2017 at 4:55:16 PM, Chawla, Pankaj (pankaj.chawla at intel.com) wrote: >> I have attached the IR I got by compiling with -O2. This is just before we widen the IV. > > Thanks! > >> To get the backedge taken count info I ran indvars on it and then replaced zext with sext. >> >> I think regardless of where we decide to add this transformation in >> the pipeline, it should try to preserve as much information as it can. >> This means that we should generate sext for signed IVs and vice-versa. >> I believe this is a better approach as it preserves the information directly in the IR as opposed to relying on ScalarEvolution to deduce it. > > I'll be happy to review patches making indvars behave better here (i.e. not "break" loop trip counts like this). > > I don't think the IV is the most relevant bit here though -- it looks like (only a guess) indvars is faltering here: > https://github.com/llvm-mirror/llvm/blob/master/lib/Transforms/Scalar/IndVarSimplify.cpp#L2240 > > and that logic needs to be made smarter to account for how much the RHS of the LFTR'ed exit condition is simplified after extension. > >> Moving it to a different location can be done separately. >> >> Do you agree? > > Sounds good! > > -- Sanjoy