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

[Hal Finkel]

----- Original Message -----
> From: "Björn Steinbrink" <bsteinbr at gmail.com>
> To: "Hal Finkel" <hfinkel at anl.gov>
> Cc: "Commit Messages and Patches for LLVM" <llvm-commits at cs.uiuc.edu>
> Sent: Monday, June 29, 2015 2:38:39 PM
> Subject: Re: [PATCH 1/2] [InstCombine] Properly combine metadata when replacing a load with another
> 
> Hi Hal,
> 
> do you think you could take another look at this?

Yes, will do soon.

 -Hal

> 
> Thanks!
> Björn
> 
> 2015-06-23 14:38 GMT+02:00 Bjorn Steinbrink <bsteinbr at gmail.com>:
> > 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