<div dir="ltr"><br><br><div class="gmail_quote">On Wed, May 6, 2015 at 1:51 PM Akira Hatanaka <<a href="mailto:ahatanak@gmail.com">ahatanak@gmail.com</a>> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">In <a href="http://reviews.llvm.org/D9537#167157" target="_blank">http://reviews.llvm.org/D9537#167157</a>, @echristo wrote:<br>
<br>
> 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.<br>
<br>
<br>
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?<br>
<br>
<br></blockquote><div><br></div><div>My comment about testcases is that someone shouldn't have:</div><div><br></div><div>; RUN: llc -march=foo</div><div><br></div><div>as the only run line with a function with:</div><div><br></div><div>target-cpu="notfoo"</div><div><br></div><div>when they could just change the arch on the function to say "foo" rather than "notfoo".</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
================<br>
Comment at: include/llvm/CodeGen/CommandFlags.h:268<br>
@@ -262,1 +267,3 @@<br>
<br>
+static inline std::pair<std::string, std::string> getCPUFeaturesStr() {<br>
+  std::string CPUStr;<br>
----------------<br>
echristo wrote:<br>
> Why a pair?<br>
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?<br>
<br></blockquote><div><br></div><div>I meant split into two functions, any particular reason?</div><div><br></div><div>-eric</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<a href="http://reviews.llvm.org/D9537" target="_blank">http://reviews.llvm.org/D9537</a><br>
<br>
EMAIL PREFERENCES<br>
  <a href="http://reviews.llvm.org/settings/panel/emailpreferences/" target="_blank">http://reviews.llvm.org/settings/panel/emailpreferences/</a><br>
<br>
<br>
</blockquote></div></div>