jmmartinez 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); ---------------- arsenm wrote: > Do you need to guard against empty string? I checked and I saw some tests without it. I'm adding the condition for it. ================ Comment at: clang/lib/CodeGen/CGCall.cpp:2018 + bool EnabledForFunc = Feature[0] == '+'; + StringRef Name = Feature.substr(1); + auto TEntry = TFeatures.find(Name); ---------------- arsenm wrote: > consume_front I used drop_front(1) instead, consume_front would only work with "+" and not drop "-". ================ Comment at: clang/lib/CodeGen/CGCall.cpp:2030-2031 + bool EnabledForTarget = TEntry->second; + if (EnabledForTarget != EnabledForFunc) + return; + } ---------------- arsenm wrote: > Early return breaks the other features I did not understand this remark. If the features are not compatible, we do not add a "target-features" entry in the new "FuncAttrs". Then, the old "target-features" entry is kept in the Function coming from the builtin. If you think it would be better to set the target-features in FuncAttrs to the old value in any case. If that's the case I've added the following code: if (EnabledForTarget != EnabledForFunc) { FuncAttr.addAttribute(FFeatures); return; } Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159206/new/ https://reviews.llvm.org/D159206 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits