[PATCH] D159206: [Clang] Propagate target-features if compatible when using mlink-builtin-bitcode

[Juan Manuel Martinez Caamaño via Phabricator via cfe-commits]

jmmartinez marked 2 inline comments as done.
jmmartinez added inline comments.


================
Comment at: clang/lib/CodeGen/CGCall.cpp:2034
+  }
+
+  FuncAttr.addAttribute("target-features", llvm::join(MergedFeatures, ","));
----------------
arsenm wrote:
> Really it would be less bad if the incompatible functions were not imported rather than the backend pass
I thought it was possible to have functions with incompatible features in the same module. 
e.g. one function compiled with some instruction set support, one without, and an ifunc that resolves to one or the other.

Maybe it's not the case in the context of `-mlink-builtin-bitcode`?


================
Comment at: clang/test/CodeGen/link-builtin-bitcode.c:17
+int __attribute__((target("extended-image-insts"))) attr_not_in_target(void) { return 42; }
+int __attribute__((target("no-gfx9-insts"))) attr_uncompatible(void) { return 42; }
 int x = 12;
----------------
arsenm wrote:
> This isn't a real target feature (do we not have a warning on this?)
> 
> s/uncompatible/incompatible
Only when the feature doesn't map to any "+" or "-" feature.

If I use "foo-insts" I get a warning because there are no +foo-insts / -foo-insts (and it still ends up in the generated llvm-ir)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159206