[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