[PATCH] D16774: AMDGPU: Stop pointlessly looking at users after a bad one is found

[David Blaikie via llvm-commits]

On Mon, Feb 1, 2016 at 12:06 PM, Matt Arsenault <Matthew.Arsenault at amd.com>
wrote:

> On 02/01/2016 10:12 AM, David Blaikie wrote:
>
>
>
> On Mon, Feb 1, 2016 at 9:51 AM, Matt Arsenault via llvm-commits <
> <llvm-commits at lists.llvm.org>llvm-commits at lists.llvm.org> wrote:
>
>> arsenm created this revision.
>> arsenm added a reviewer: tstellarAMD.
>> arsenm added a subscriber: llvm-commits.
>> Herald added a subscriber: arsenm.
>>
>> http://reviews.llvm.org/D16774
>>
>> Files:
>>   lib/Target/AMDGPU/AMDGPUPromoteAlloca.cpp
>>
>> Index: lib/Target/AMDGPU/AMDGPUPromoteAlloca.cpp
>> ===================================================================
>> --- lib/Target/AMDGPU/AMDGPUPromoteAlloca.cpp
>> +++ lib/Target/AMDGPU/AMDGPUPromoteAlloca.cpp
>> @@ -402,7 +402,10 @@
>>  static bool collectUsesWithPtrTypes(Value *Val, std::vector<Value*>
>> &WorkList) {
>>    bool Success = true;
>>    for (User *User : Val->users()) {
>> -    if(std::find(WorkList.begin(), WorkList.end(), User) !=
>> WorkList.end())
>> +    if (!Success)
>> +      return false;
>> +
>> +    if (std::find(WorkList.begin(), WorkList.end(), User) !=
>> WorkList.end())
>>        continue;
>>      if (CallInst *CI = dyn_cast<CallInst>(User)) {
>>        // TODO: We might be able to handle some cases where the callee is
>> a
>> @@ -429,10 +432,10 @@
>>        continue;
>>
>>      WorkList.push_back(User);
>> -
>> -    Success &= collectUsesWithPtrTypes(User, WorkList);
>> +    Success = collectUsesWithPtrTypes(User, WorkList);
>>
>
> Why not just drop the Success variable entirely and return from here:
>
> if (!collectUsesWithPtrTypes(...))
>   return false
>
> Return here is what I tried first, but this breaks some cases due to the
> !isPointerTy check above. Call users without pointer types would then fall
> through and incorrectly be changed.
>

Sorry, couldn't really follow... the original code would return false if
collectUsesWithPtrTypes ever returns false, right? Were there side effects
that were desired after that point?


> I'm planning on fixing some of the intrinsic handling here, so maybe after
> that this will be OK.
>
>
> Or, possibly better yet:
>
> return llvm::all_of(Val->users(), [&](User *User) {
>   ...
>   return collectUsesWithPtrTypes(User, WorkList);
> });
>
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160201/262d781e/attachment.html>