https://github.com/AaronBallman requested changes to this pull request.
Thank you for working on this, but I'm generally not in favor of this patch. I
do not think we should expose all LLVM IR (function) attributes directly in
Clang for a whole host of reasons:
* User experience is very poor. Some LLVM IR attributes have specific
requirements and those won't be checked by the compiler frontend. For example,
what does it mean to put the `alignstack` LLVM IR attribute on a function
that's marked with the `naked` attribute (so no prolog or epilog is generated)?
What does it mean to put an `alloc-family` LLVM IR attribute on a function that
returns `void`? Etc.
* LLVM IR attributes are implementation details, Clang attributes are part of
our contract with users. LLVM IR attributes can change or be removed
specifically because they're not user facing, the same is not true of
attributes exposed in Clang.
* This isn't feature-testable (`__has_c|cpp_attribute` will return `true` for
`llvm_fn_attr` but there's no way to test for specific LLVM IR attributes.
* The feature is not ergonomic with attribute argument handling. For example,
the LLVM IR `allocsize` attribute accepts multiple values, but we don't support
that (we could, but that has other issues like accepting more values than the
IR attribute allows), the `memory` attribute uses an enumeration of values, but
we don't support that, etc.
* This duplicates functionality already exposed via other Clang attributes,
such as `convergent`, `hot`, `cold`, etc but there may be subtle differences in
behavior (the frontend will look at `ReturnsTwiceAttr` for more than just
codegen, but it won't for `llvm_fn_attr("returns_twice")`).
In general, I think we want to expose LLVM IR attributes manually and only when
they have a known use case that warrants their support as an extension in Clang.
https://github.com/llvm/llvm-project/pull/83059
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
