[PATCH] D99100: [WIP] Implement RFC: Decomposing deref(N) into deref(N) + nofree

Philip Reames via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jul 14 16:21:26 PDT 2021


reames added a comment.

In D99100#2878515 <https://reviews.llvm.org/D99100#2878515>, @nikic wrote:

> In D99100#2878486 <https://reviews.llvm.org/D99100#2878486>, @reames wrote:
>
>> In D99100#2878416 <https://reviews.llvm.org/D99100#2878416>, @nikic wrote:
>>
>>> From a quick look at canBeFreed(), it seems like the case of a `nofree` argument isn't handled yet.
>>
>> That's because there was disagreement as to what the semantics of such an argument were.  I got frustrated in trying to drive that towards any useful conclusion, and don't plan to return to the topic.
>>
>> On the general topic, https://reviews.llvm.org/D101701 is still open, but that doesn't seem to directly involve the parameter attribute.   I can't find what I'm thinking of, so maybe I just misremembered that particular sub-piece being controversial?
>
> I think the controversy was about the function attribute only. It's pretty important to me that using `dereferenceable` plus `nofree` on an argument works (and does so without any additional nosync requirements), because that means you can get the current `dereferenceable` semantics back simply by emitting `dereferenceable nofree` in your frontend wherever you used plain `dereferenceable` before. That gives a clear migration path for which no regressions should be expected (is that right?)

To be clear, there is no current migration plan for which no regressions are expected.  I tried to come up with one, it failed miserably.  I'm no longer working towards that goal.

Your point about wanting nofree on a parameter to not require nosync runs exactly into the discussion I linked.

If you want to drive the nofree parameter case, feel free.  If you want to collect some numbers and tell me whether this a real or hypothetical regression, that would be super useful.

Lest I seem too dismissive, let me expand on something I was planning to include in the description of the real patch after we had some preliminary numbers.  We will infer nofree+nosync on the function if possible.  In practice, this covers a lot of the concerning cases I saw.  The biggest hesitation I have is that inlining small functions effectively destroys our ability to infer nofree/nosync regions.  We don't really have an answer for context nofree reasoning after heavy inlining.  If we're going to have a major regression, I'm expecting it to come from that interaction.


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

https://reviews.llvm.org/D99100



More information about the llvm-commits mailing list