[PATCH] D80750: llvm-link: Add module flag behavior MergeTargetID

[Yaxun Liu via Phabricator via llvm-commits]

yaxunl marked an inline comment as done.
yaxunl added a comment.

In D80750#2180284 <https://reviews.llvm.org/D80750#2180284>, @jdoerfert wrote:

> I believe there are three things in this patch, but feel free to correct me:
>
> 1. A way to specify a target triple + cpu. Basically like `target triple = ...` but in the module metadata plus some additional target cpu suffix, which is so far in the `target-cpu` function attribute list.
> 2. A way to specify global target features, which are so far in the `target-features` function attribute list.
> 3. Making llvm-link aware of 1) and 2) and verifying they match (under some rules).
>
> If this is the case, what is the benefit over a toplevel module entry that allows you to specify `target-cpu` and `target-feature` for the entire module? I mean, they seem to be very much the same thing as `target triple`, yet we go a totally different route to add them and verify the match during linking. I believe other people might benefit from this, e.g., to get rid of function-level attributes, so we should shoot for a generic solution.

The matching and merging rule we want to apply to the target features in target ID is not generic for arbitrary target features. We can apply these rules to target features only if they appear in a target ID.

Also we do not see strong motivation to enforce matching of target-cpu in general, otherwise such a rule should have been added to the linker. However, for target ID we have reasons to enforce such checking.

target ID is documented in https://reviews.llvm.org/D84822 It is general enough to be adopted by any targets if it is needed. In the degenerated case it can be simply the processor, in more general case it can includes target features that need to be consistent across modules. The usage of target-id module flag is like a contract saying that what needs to be matched, whereas in general such matching is not required.


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

https://reviews.llvm.org/D80750