[PATCH] D23174: GVN-hoist: fix early exit logic

Sebastian Pop via llvm-commits llvm-commits at lists.llvm.org
Thu Aug 4 13:58:07 PDT 2016


On Thu, Aug 4, 2016 at 3:53 PM, Daniel Berlin <dberlin at dberlin.org> wrote:
> IMHO, you should just refactor that and call it "isRelevantInstruction" or
> something.

Ok, I will split that part off this patch and submit separately for review.

>
> There are others you are likely to want to ignore.

Yes, I'm pretty sure I forgot about the XYZ instruction that we
shouldn't look at for hoisting.

>
>
>
> On Thu, Aug 4, 2016 at 1:44 PM, Sebastian Pop <sebpop at gmail.com> wrote:
>>
>> sebpop added inline comments.
>>
>> ================
>> Comment at: lib/Transforms/Scalar/GVNHoist.cpp:897
>> @@ +896,3 @@
>> +          // Also do not bother analyzing the following kinds of
>> instructions.
>> +          if (!isa<TerminatorInst>(&I1))
>> +            II.insert(&I1, VN);
>> ----------------
>> dberlin wrote:
>> > I hope this is not a correctness fix :)
>> Nop. This is a compilation time fix: as I was saying in the commit
>> message, we should not try to analyze to hoist this kind of "scalar
>> instructions".
>>
>>
>> https://reviews.llvm.org/D23174
>>
>>
>>
>


More information about the llvm-commits mailing list