efriedma added a comment.
Do the macros you're defining here match gcc?
================
Comment at: lib/Basic/Targets/RISCV.cpp:68
+
+bool RISCVTargetInfo::hasFeature(StringRef Feature) const {
+ return llvm::StringSwitch<bool>(Feature)
----------------
asb wrote:
> It seems a number of other targets also return true for the archname, e.g.
> "riscv".
>
> Is it the case that hasFeature should always support at least everything
> detected by handleTargetFeatures? If so, a comment to document this would be
> helpful.
>
> I can see this method was introduced in rC149227 and is primarily motivated
> by module requirements.
>
> @doug.gregor / @rsmith : could you please advise on the implementation and
> testing of this function? Which features should be hasFeature check for? I
> note that e.g. lib/Basic/Targets/ARM.cpp only supports a subset of the
> features in hasFeature compared to handleTargetFeatures.
I think your assessment is right... and also, it looks like we have poor test
coverage for the set of features which are actually supported. So probably
some targets are missing relevant features.
Should be straightforward to test, I think; you just need to write a module.map
with appropriate "requires" lines, and a test file which tries to include those
modules. See test/Modules/Inputs/module.map.
Repository:
rC Clang
https://reviews.llvm.org/D44727
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
