[PATCH] D104798: [WPD] Don't optimize calls more than once

Wed Jun 23 19:56:10 PDT 2021

aeubanks added a comment.

In D104798#2837187 <https://reviews.llvm.org/D104798#2837187>, @tejohnson wrote:

> In D104798#2836775 <https://reviews.llvm.org/D104798#2836775>, @aeubanks wrote:
>
>> This probably requires more test coverage, but I'm not super familiar
>>
>> In D104798#2836614 <https://reviews.llvm.org/D104798#2836614>, @tejohnson wrote:
>>
>>>> WPD currently assumes that each vtable load corresponds to one virtual call. However, with -fstrict-vtable-pointers this may not be true.
>>>
>>> In the test case there is still a one to one correlation between the vtable load and virtual call. Do you mean that it can correspond to more than one type test assume sequence?
>>
>> Originally there are two vtable loads, each with a type test assume sequence. The vtable loads become coalesced, leaving two type test assume sequences for one vtable load.
>
> I assume -fstrict-vtable-pointers was what allowed the vtable loads to be coalesced? The description seems wrong though, since it is talking about one vtable load to one virtual call, and here that is still the case. I'm a little confused - in the test case why would there originally have been 2 vtable loads for that single virtual call?

Oh my test case is confusing. I omitted the second virtual call in the test case and the crash still reproduced.

> It looks like the issue is that we no longer have a one to one correspondence between type test assume sequences and virtual calls - is that correct?

Yeah I think your statement makes more sense.

> Also, do we need to do something similar for singleImplDevirt, or why does this issue not occur there?

Ah I missed that. Probably it just happened that it didn't trigger in Chrome. Added to that part.

I'll try to come up with tests for all of the cases.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104798/new/

https://reviews.llvm.org/D104798