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

[Björn Steinbrink]

Hi Hal,

do you think you could take another look at this?

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