yaxunl marked 2 inline comments as done.
yaxunl added inline comments.
================
Comment at: clang/include/clang/Basic/Builtins.h:263
+ /// false if it is disabled.
+ bool isRequiredTargetFeaturesEnabled(
+ unsigned BuiltinID, const llvm::StringMap<bool> &TargetFetureMap) const;
----------------
tra wrote:
> I think we should boil it down to just
> `evaluateRequiredTargetFeatures(StringRef RequiredFeatures, ...
> TargetFeatureMap)`.
>
will do
================
Comment at: clang/lib/Basic/BuiltinTargetFeatures.h:32
+/// pairs.
+class TargetFeatures {
+ struct FeatureListStatus {
----------------
tra wrote:
> I don't think it needs to be part of the public API. Feature check function
> operates on `llvm::StringMap<bool>` and does not need to know about this
> class, and the implementation of this class can be moved into `Builtins.cpp`
> where it's actually used.
It is not a public API since it is under lib/Basic.
The reason it is made a header file because we have a unit test
clang/unittests/CodeGen/CheckTargetFeaturesTest.cpp for class TargetFeatures.
This is because TargetFeatures is relatively complicated.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D125829/new/
https://reviews.llvm.org/D125829
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
