Sirraide wrote: Regarding the effects implementation, my understanding of the situation is this: There are a number of effects that we support. Each effect has a set of properties, which are represented as flags. Furthermore, we can attach one or more effects to a function.
I understand that you’d want to avoid allocating memory for effects over and over again, but at the same time—I think it’s fine to just don’t cache effect sets at all. We already cache function types, so creating the same function type twice w/ the same effect set is also going to cache the effect set. I think just sorting them based on some ID should be enough. Sure, we’d be saving a bit of memory, but I don’t think the overhead of dealing w/ implementing that is worth it; we’re not going to start attaching dozens of effects to every function type; even if we infer them, we’re already creating AttributedTypes and storing things like arguments in the function types anyway, and the overhead of those is easily going to be much larger than that of storing the effects. If it should turn out that the memory overhead is unacceptable, then we can always look into caching them later, but as it stands, this just seems like premature optimisation to me (and I don’t particularly like throwing that phrase around). Of course, if you’ve already benchmarked this and found that caching them is necessary, then that would change things, but I’m not convinced that caching here would do much—or that we should be doing any caching without benchmarking it first. https://github.com/llvm/llvm-project/pull/84983 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
