[PATCH] [BUG 22755] Fix bugs in CombinToPreIndexedLoadStore

[Yin Ma]

Hi Hal,

Thanks, I will put more comments into it. 

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