[PATCH] D33074: InstCombine: Allow sinking instructions with more uses in the same block.

Kyle Butt via llvm-commits llvm-commits at lists.llvm.org
Thu May 11 07:34:52 PDT 2017


On Wed, May 10, 2017 at 8:39 PM, David Majnemer <david.majnemer at gmail.com>
wrote:

> Test case?
>

Test case forthcoming. I wanted to run the idea by some reviewers before
sinking too much time into it.


>
> On Wed, May 10, 2017 at 8:13 PM Dean Michael Berris via Phabricator via
> llvm-commits <llvm-commits at lists.llvm.org> wrote:
>
>> dberris added a comment.
>>
>> Drive-by-review.
>>
>>
>>
>> ================
>> Comment at: lib/Transforms/InstCombine/InstructionCombining.cpp:2920
>> +        UsesInOneBlock = true;
>> +      } else if (CurrentUserParent != UsersParent) {
>> +        UsesInOneBlock = false;
>> ----------------
>> Do you actually need the else here? I suspect CurrentUserParent will
>> never be nullptr, and UsersParent will never be nullptr here either after
>> the preceding conditional.
>>
>>
>> ================
>> Comment at: lib/Transforms/InstCombine/InstructionCombining.cpp:2927
>> +
>> +    if (UsesInOneBlock) {
>> +      BasicBlock *BB = I->getParent();
>> ----------------
>> Maybe clearer if you turn this into an early return?
>>
>> ```
>> if (!UsesInOneBlock)
>>   return false;
>>
>> // rest of code.
>> ```
>>
>>
>> Repository:
>>   rL LLVM
>>
>> https://reviews.llvm.org/D33074
>>
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170511/9cb41d75/attachment.html>


More information about the llvm-commits mailing list