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

[Hal Finkel via llvm-commits]

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