[PATCH] [BUG 22755] Fix bugs in CombinToPreIndexedLoadStore

[Hal Finkel]

----- Original Message -----
> From: "Yin Ma" <yinma at codeaurora.org>
> To: "Hal Finkel" <hfinkel at anl.gov>
> Cc: llvm-commits at cs.uiuc.edu
> Sent: Monday, May 18, 2015 5:05:12 PM
> Subject: RE: [PATCH] [BUG 22755] Fix bugs in CombinToPreIndexedLoadStore
> 
> Hi Hal,
> 
> Thanks for working on this. It looks good for me.
> I think this line should serve the purpose
> if (Use.getUser() == Ptr.getNode() || Use != BasePtr)
>          continue;

Yes, that was exactly the intent. Thanks!

 -Hal

> 
> Yin
> 
> -----Original Message-----
> From: Hal Finkel [mailto:hfinkel at anl.gov]
> Sent: Monday, May 18, 2015 8:51 AM
> To: Yin Ma
> Cc: llvm-commits at cs.uiuc.edu
> Subject: Re: [PATCH] [BUG 22755] Fix bugs in
> CombinToPreIndexedLoadStore
> 
> Hi Yin,
> 
> I rewrote the loop in a slightly different way, which should fix this
> problem (r237576). Please confirm that this does indeed fix the
> problem.
> 
>  -Hal
> 
> ----- Original Message -----
> > From: "Yin Ma" <yinma at codeaurora.org>
> > To: "Hal Finkel" <hfinkel at anl.gov>
> > Cc: llvm-commits at cs.uiuc.edu
> > Sent: Thursday, April 23, 2015 4:01:55 PM
> > Subject: RE: [PATCH] [BUG 22755] Fix bugs in
> > CombinToPreIndexedLoadStore
> > 
> > Hi Hal,
> > 
> > Could you help me to do this? I was busy in other projects right
> > now.
> > Thanks a lot.
> > 
> > Yin
> > 
> > -----Original Message-----
> > From: Hal Finkel [mailto:hfinkel at anl.gov]
> > Sent: Wednesday, April 22, 2015 6:23 PM
> > To: Yin Ma
> > Cc: llvm-commits at cs.uiuc.edu
> > Subject: Re: [PATCH] [BUG 22755] Fix bugs in
> > CombinToPreIndexedLoadStore
> > 
> > ----- Original Message -----
> > > From: "Yin Ma" <yinma at codeaurora.org>
> > > To: "Hal Finkel" <hfinkel at anl.gov>
> > > Cc: llvm-commits at cs.uiuc.edu
> > > Sent: Tuesday, March 31, 2015 1:01:22 PM
> > > Subject: RE: [PATCH] [BUG 22755] Fix bugs in
> > > CombinToPreIndexedLoadStore
> > > 
> > > Hi Hal,
> > > 
> > > Thanks, I will put more comments into it.
> > 
> > If you won't be able to get to this soon, I'll be happy to do it
> > for
> > you. I don't want us to forget about it.
> > 
> > Thanks again,
> > Hal
> > 
> > > 
> > > Yin
> > > 
> > > -----Original Message-----
> > > From: Hal Finkel [mailto:hfinkel at anl.gov]
> > > Sent: Tuesday, March 31, 2015 10:14 AM
> > > To: Yin Ma
> > > Cc: llvm-commits at cs.uiuc.edu
> > > Subject: Re: [PATCH] [BUG 22755] Fix bugs in
> > > CombinToPreIndexedLoadStore
> > > 
> > > Hi Yin,
> > > 
> > > I apologize for taking so long to look at this. The patch is
> > > fine,
> > > my only request is that comment you currently add:
> > > 
> > > +      // Must come from the same resNo.
> > > +      if (Op0.getResNo() != Ptr.getOperand(0).getResNo()) {
> > > +        OtherUses.clear();
> > > +        break;
> > > +      }
> > > 
> > > could better explain what is going on. In the bug report, you
> > > explain that this is to prevent picking up an add of the updated
> > > index from a pre-increment load with the result of the load
> > > itself.
> > > Could you please add text to that effect in the comment.
> > > Otherwise,
> > > LGTM.
> > > 
> > > Thanks again,
> > > Hal
> > > 
> > > ----- Original Message -----
> > > > From: "Yin Ma" <yinma at codeaurora.org>
> > > > To: llvm-commits at cs.uiuc.edu
> > > > Cc: hfinkel at anl.gov
> > > > Sent: Wednesday, March 11, 2015 12:40:45 PM
> > > > Subject: [PATCH] [BUG 22755] Fix bugs in
> > > > CombinToPreIndexedLoadStore
> > > > 
> > > > 
> > > > 
> > > > 
> > > > Hi,
> > > > 
> > > > 
> > > > 
> > > > This simple change is for Bug 22755
> > > > 
> > > > https://llvm.org/bugs/show_bug.cgi?id=22755
> > > > 
> > > > 
> > > > 
> > > > Basically, the code tried to fold two adds. However, in a
> > > > situation where it is a preindex load L converted before, it is
> > > > possible that the input of one add is from the result of L and
> > > > the
> > > > input of another add is from the updated index of L. These two
> > > > adds cannot be combined together clearly, so we need check to
> > > > make
> > > > sure two adds are from the same ResNo by adding code in the
> > > > patch.
> > > > 
> > > > 
> > > > 
> > > > Unfortunately, I don’t have a test case for it. However, it
> > > > should
> > > > be quite clear to see the change.
> > > > 
> > > > Please review.
> > > > 
> > > > 
> > > > 
> > > > Thanks,
> > > > 
> > > > 
> > > > 
> > > > Yin
> > > 
> > > --
> > > Hal Finkel
> > > Assistant Computational Scientist
> > > Leadership Computing Facility
> > > Argonne National Laboratory
> > > 
> > > 
> > 
> > --
> > Hal Finkel
> > Assistant Computational Scientist
> > Leadership Computing Facility
> > Argonne National Laboratory
> > 
> > 
> 
> --
> Hal Finkel
> Assistant Computational Scientist
> Leadership Computing Facility
> Argonne National Laboratory
> 
> 

-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory