[PATCH] D12660: Make the default triple optional by allowing an empty string

Mehdi AMINI via llvm-commits llvm-commits at lists.llvm.org
Wed Sep 9 22:10:40 PDT 2015

joker.eph marked 4 inline comments as done.
joker.eph added a comment.

In http://reviews.llvm.org/D12660#242029, @probinson wrote:

> Does llvm-mc always need a triple?  That is, would it be better to add test/tools/llvm-mc/lit.local.cfg (like you did for test/DebugInfo) instead of adding REQUIRES to each test/tools/llvm-mc/* file?

Created test/tools/llvm-mc/Generic/

> It seems weird for target-specific tests to require a default triple, but I guess those are the kinds of tests where you and jyknight are talking about iterating over some list of triples, and that seems like a good future direction.

Do you mean the ones in test/CodeGen/PowerPC, test/CodeGen/X86, etc.?
They really either shouldn't be in these directory, or should specify a triple. This is a bit messy but I wasn't sure which triple to define for these.

> Have you thought about moving test/DebugInfo/*.ll to a Generic subdirectory?  Not saying you should or should not, but it seems possibly sub-optimal to require a default triple for *all* DebugInfo tests, even the target-specific ones.  But maybe it's okay the way it is; I haven't done any particular research or thought about it much.


> See also inline comments.

Comment at: test/Feature/optnone-llc.ll:7
@@ -6,3 +6,3 @@
-; REQUIRES: asserts, native
+; REQUIRES: asserts
probinson wrote:
> I have generally started associating "test uses llc" with "REQUIRES: default_triple" but here we have a test that doesn't?  It seems like the configuration you use (host=X86, default-triple empty, only-target=GPU) should fail here.  That is, I'd expect llc to fall back on the host (X86-ish) triple and therefore fail.  Doesn't it?
This was a mistake (I ran the tests with a release build and was covered by the "REQUIRES: asserts")

Comment at: test/lit.cfg:346
@@ -344,1 +345,3 @@
+# Native compilation: host arch == default triple arch
 # FIXME: Consider cases that target can be executed
probinson wrote:
> Given that we both misunderstood the purpose of this feature, it would be good to improve the commentary a little ("needed for JIT" or something like that).
What about renaming it then? 
I can do that in a separate patch.


More information about the llvm-commits mailing list