[PATCH 1/2] [InstCombine] Properly combine metadata when replacing a load with another

[Hal Finkel]

LGTM, thanks!

 -Hal

----- Original Message -----
> From: "Bjorn Steinbrink" <bsteinbr at gmail.com>
> To: "Hal Finkel" <hfinkel at anl.gov>
> Cc: llvm-commits at cs.uiuc.edu
> Sent: Tuesday, June 23, 2015 7:38:57 AM
> Subject: [PATCH 1/2] [InstCombine] Properly combine metadata when replacing a load with another
> 
> Hi Hal,
> 
> 2015-06-23 3:16 GMT+02:00 Hal Finkel <hfinkel at anl.gov>:
> > Dropping all AA metadata on these loads when we combine them seems
> > quite unfortunate; we should at least be able to keep the AA
> > metadata
> > from the original load.
> 
> Oops! I had intentionally skipped the AA metadata IDs here because I
> wanted the AA metadata to be preserved but forgot that
> combineMetadata
> drops all unknown metadata. Thanks!
> 
> > FindAvailableLoadedValue will return the the
> > AATags as an optional output parameter. You could then call
> > setAAMetadata after you run the combining. Please add a test case
> > for
> > this.
> 
> Done.
> 
> > Also, while you're here, as follow-up, it seems like we should be
> > able
> > to pass the AA parameter to FindAvailableLoadedValue too. Should be
> > easy to come up with a test: just stick a store to some noalias
> > pointer in between the two loads.
> 
> Added in a second patch.
> 
> Thanks,
> Björn
> 
> > Thanks again,
> > Hal
> >
> > ----- Original Message -----
> >> From: "Björn Steinbrink" <bsteinbr at gmail.com>
> >> To: llvm-commits at cs.uiuc.edu
> >> Sent: Monday, June 22, 2015 7:56:56 AM
> >> Subject: [PATCH] [InstCombine] Properly combine metadata when
> >> replacing a     load with another
> >>
> >>
> >> Not doing this can lead to misoptimizations down the line, e.g.
> >> because
> >> of range metadata on the replacing load excluding values that are
> >> valid
> >> for the load that is being replaced.
> >> ---
> >>  .../InstCombine/InstCombineLoadStoreAlloca.cpp        | 12
> >>  +++++++++++-
> >>  test/Transforms/InstCombine/load-range-metadata.ll    | 19
> >>  +++++++++++++++++++
> >>  2 files changed, 30 insertions(+), 1 deletion(-)
> >>  create mode 100644
> >>  test/Transforms/InstCombine/load-range-metadata.ll
> >>
> >>
> >> _______________________________________________
> >> llvm-commits mailing list
> >> llvm-commits at cs.uiuc.edu
> >> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> >>
> >
> > --
> > Hal Finkel
> > Assistant Computational Scientist
> > Leadership Computing Facility
> > Argonne National Laboratory
> 

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