[PATCH] D37215: [ValueTracking] improve reverse assumption inference

[Hal Finkel via llvm-commits]

On 08/31/2017 03:37 PM, Ariel Ben-Yehuda wrote:
> But the code as present does an `argmemonly` check - I didn't put it there.

Indeed. This is wrong (the code here is assuming that loops can be 
infinite only if they essentially contain volatile writes representing 
I/O, and this is incorrect. Actual volatile accesses and atomics don't 
have this property). Nevertheless, this is a pre-existing problem, and 
fixing it is going to be a larger project. The only things in the 
current list that aren't read-none/argmemonly are 
ptr_annotation/var_annotation (and Intrinsic::annotation is missing it 
seems). Can you add those so we don't regress and then we can move 
forward with this.

Thanks again,
Hal

>
> On Thu, Aug 31, 2017 at 11:33 PM, Hal Finkel <hfinkel at anl.gov> wrote:
>> On 08/31/2017 03:22 PM, Ariel Ben-Yehuda wrote:
>>> Are not all the `isAssumeLikeIntrinsic` intrinsics are argmemonly?
>>
>> They might be, but the argmemonly check does not seem correct. I don't see
>> what argmemonly has to do with the semantics for which you're checking
>> (which, as explained in the comment above, is that the call much return
>> (i.e. not exit the process or hang, etc.).
>>
>>   -Hal
>>
>>
>>> On Thu, Aug 31, 2017 at 5:41 PM, Hal Finkel via Phabricator
>>> <reviews at reviews.llvm.org> wrote:
>>>> hfinkel added a comment.
>>>>
>>>> This looks good, however, you need to either:
>>>>
>>>> 1. Don't remove isAssumeLikeIntrinsic, but instead, update
>>>> isGuaranteedToTransferExecutionToSuccessor to call it.
>>>> 2. Update isGuaranteedToTransferExecutionToSuccessor to have the complete
>>>> list from isAssumeLikeIntrinsic.
>>>>
>>>> Right now, isGuaranteedToTransferExecutionToSuccessor only checks:
>>>>
>>>>     return CS.onlyReadsMemory() || CS.onlyAccessesArgMemory() ||
>>>>            match(I, m_Intrinsic<Intrinsic::assume>());
>>>>
>>>>
>>>> https://reviews.llvm.org/D37215
>>>>
>>>>
>>>>
>> --
>> Hal Finkel
>> Lead, Compiler Technology and Programming Languages
>> Leadership Computing Facility
>> Argonne National Laboratory
>>

-- 
Hal Finkel
Lead, Compiler Technology and Programming Languages
Leadership Computing Facility
Argonne National Laboratory