paulkirth added a comment.
> I agree that `omitprofile` probably doesn't communicate the correct meaning > very well. I like `nodirectprofile` because it hopefully implies that > indirect profiles are allowed. I'm wondering if anyone else has suggestions. > > Is there any precedent for renaming function attributes? My assumption was > that users would add the `noprofile` attribute to source mostly for > correctness concerns and `noprofile` is easy to remember and already in use, > so I'd rather not change it. I agree with you that it's probably best to avoid changing the spelling of `noprofile`, but since you're making a change in the area, we may as well discuss if 1) that is still the best name, and 2) if we want to keep it. I don't know if we've ever changed the spelling of a `FnAttribute` before. IIRC we've dropped them, and changed their format e.g., from being a boolean to taking a value or an enum. At least that's my memory, which seems to correspond to the autoupgrade stuff in `BitcodeReader`. But I think the project's stance on this(like most things) is that "if there's a good reason, then we should do it." At least if we can maintain our stated compatibility guarantees. This is a place where I think we should get additional feedback before proceeding though. It might be worth asking about this on discourse. Even if we don't want to make such a change in these patches, it may be good for the project to have a policy (or at least guidelines) about changing attributes. > I actually do have a change in `PGOInstrumentation.cpp` that skips profiling > functions with this new attribute. As far as I know, this stack has all the > necessary code changes. I can't believe I missed that. I looked three times before I commented, and blew right passed it every time. That's what happens when I look at code late on a Friday... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130807/new/ https://reviews.llvm.org/D130807 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
