[PATCH] Let llc and opt override "-target-cpu" and "-target-features" using command line options "-mcpu" and "-mattr"

Akira Hatanaka ahatanak at gmail.com
Wed May 6 13:51:18 PDT 2015


In http://reviews.llvm.org/D9537#167157, @echristo wrote:

> I agree, no TargetOptions for this. I don't necessarily agree with rewriting, but it seems to be a small enough change and will work for what the overrides are meant to be for. Some discipline in reviewing testcases to make sure that a lot of overrides aren't there for incoming testcases is going to be necessary for clean tests. One inline comment which basically sums up to "why not two functions"? As long as you have a reasonable answer go ahead.


I'm not sure if I understood your comment about testcases. Do you mean people shouldn't duplicate functions in test cases just to test different target-cpu and target-feature strings but instead should use -mcpu and -mattrs (which I think makes sense)? Should the one inline comment go into the test cases I added?


================
Comment at: include/llvm/CodeGen/CommandFlags.h:268
@@ -262,1 +267,3 @@
 
+static inline std::pair<std::string, std::string> getCPUFeaturesStr() {
+  std::string CPUStr;
----------------
echristo wrote:
> Why a pair?
Is it better to split this into two functions getCPUStr and getFeaturesStr? Or you meant the two strings (cpu and features strings) should be passed by reference?

http://reviews.llvm.org/D9537

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list