[PATCH] D108123: [Local] Move isFreeCall() before willReturn()

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added a comment.

In D108123#2946790 <https://reviews.llvm.org/D108123#2946790>, @guopeilin wrote:

> In D108123#2946738 <https://reviews.llvm.org/D108123#2946738>, @lebedev.ri wrote:
>
>> In D108123#2946733 <https://reviews.llvm.org/D108123#2946733>, @guopeilin wrote:
>>
>>> In D108123#2946665 <https://reviews.llvm.org/D108123#2946665>, @lebedev.ri wrote:
>>>
>>>> This test already passes without the code change: https://godbolt.org/z/48TnbeKj1
>>>
>>> The store is not expected, which will become unreachable after SimplifyCFG.
>>
>> Right, but that is the correct outcome.
>> Why is it not UB to free an `undef` (a.k.a. any random pointer)?
>> It is as per the current langref: https://alive2.llvm.org/ce/z/jBqe-E
>>
>> Also, i would like to note that the patch's description doesn't mention that this wants to make `free(undef)` non-UB.
>
> The key point is that function `free` can be overload, take this test case as an example, we can overload function `_ZdlPv` with an empty body. So the result becomes that we call a `free` function which actually does not free anything.  Absolutely this is not a way that `free an undef`.
> So it is better to delete an instruction that calling an empty function with an undefined argument.
> I will update this test case later, also the description.

Thanks, but this still does not sound good to me.
If you want to change semantics of `free()`/`_ZdlPv()` in such a way,
what you need to do is to mark it as `nobuiltin`: https://godbolt.org/z/a5P3TzG7T


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108123