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

[Ariel Ben-Yehuda via llvm-commits]

But the code as present does an `argmemonly` check - I didn't put it there.

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
>