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

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added inline comments.


================
Comment at: clang/lib/CodeGen/CGCall.cpp:2017
+    for (StringRef Feature : llvm::split(FFeatures.getValueAsString(), ',')) {
+      bool EnabledForFunc = Feature[0] == '+';
+      StringRef Name = Feature.substr(1);
----------------
Do you need to guard against empty string?


================
Comment at: clang/lib/CodeGen/CGCall.cpp:2018
+      bool EnabledForFunc = Feature[0] == '+';
+      StringRef Name = Feature.substr(1);
+      auto TEntry = TFeatures.find(Name);
----------------
consume_front


================
Comment at: clang/lib/CodeGen/CGCall.cpp:2021
+
+      // if the feature is not set for the target-opts, it must be preserved
+      if (TEntry == TFeatures.end()) {
----------------
Capitalize


================
Comment at: clang/lib/CodeGen/CGCall.cpp:2027
+
+      // if the feature is enabled for one and disabled for the other, they are
+      // not compatible
----------------
Capitalize


================
Comment at: clang/lib/CodeGen/CGCall.cpp:2030-2031
+      bool EnabledForTarget = TEntry->second;
+      if (EnabledForTarget != EnabledForFunc)
+        return;
+    }
----------------
Early return breaks the other features


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159206