[PATCH] D142733: Add _Optional as fast qualifier

Sun Jan 29 12:24:07 PST 2023

chrisbazley added a comment.

Hi, thanks very much for looking at my patch. I added the link that you proposed to all of the patches in the stack.

> Assuming that we want to go in this direction, it seems quite expensive to model this as a fast qualifier rather than an extended qualifier.

True, and I'm not at all wedded to the idea of _Optional being a fast qualifier. I added it in the simplest way I knew how, having no prior experience of the codebase. What would be really useful would be if you could show/explain how to add it in a less risky way. If not, I can look into it myself.

> Are these annotations expected to be so common that it's better to increase the alignment of all types than perform extra allocations and indirections for _Optional qualifiers?

Given time, I hope so, but realistically not in the near future.

> Have you measured the memory impact of increasing the alignment of Type?

No, because I didn't think it necessary for the purpose of prototyping. If there's any prospect of getting my patches merged then I'd be delighted to invest the time... but only if increasing the alignment is a necessary thing to do.

> I think that should be a prerequisite to adding any new kind of fast qualifier, and if we do add such a qualifier, we should select carefully which qualifier gets this precious bit in QualType.

I believe that over time, _Optional would be a much more appropriate use of the precious bit currently occupied by volatile because I expect it to be used more heavily, but I can't make that argument yet.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142733