[llvm][lit] Omit vendor in triples for "native" feature

[tambry]

The vendor component may be different, e.g. x86_64-linux-gnu vs x86_64-pc-linux-gnu, but it's still native.

[llvmbot]

@llvm/pr-subscribers-testing-tools

Author: Raul Tambre (tambry)

Changes

The vendor component may be different, e.g. x86_64-linux-gnu vs x86_64-pc-linux-gnu, but it's still native.

---

Full diff: https://github.com/llvm/llvm-project/pull/136325.diff

1 Files Affected:

• (modified) llvm/utils/lit/lit/llvm/config.py (+12-2)

diff --git a/llvm/utils/lit/lit/llvm/config.py b/llvm/utils/lit/lit/llvm/config.py
index c134cda90e2ee..0b40a407f8f69 100644
--- a/llvm/utils/lit/lit/llvm/config.py
+++ b/llvm/utils/lit/lit/llvm/config.py
@@ -24,6 +24,16 @@ def user_is_root():
     return False
 
 
+def remove_triple_vendor(triple):
+    components = triple.split("-")
+
+    # Remove vendor component (e.g. unknown, pc) if present.
+    if len(components) == 4:
+        del components[1]
+
+    return "-".join(components)
+
+
 class LLVMConfig(object):
     def __init__(self, lit_config, config):
         self.lit_config = lit_config
@@ -108,14 +118,14 @@ def __init__(self, lit_config, config):
         elif platform.system() == "OS/390":
             features.add("system-zos")
 
-        # Native compilation: host arch == default triple arch
+        # Native compilation: host arch == default triple arch (sans vendor)
         # Both of these values should probably be in every site config (e.g. as
         # part of the standard header.  But currently they aren't)
         host_triple = getattr(config, "host_triple", None)
         target_triple = getattr(config, "target_triple", None)
         features.add("host=%s" % host_triple)
         features.add("target=%s" % target_triple)
-        if host_triple and host_triple == target_triple:
+        if host_triple and remove_triple_vendor(host_triple) == remove_triple_vendor(target_triple):
             features.add("native")
 
         # Sanitizers.

[github-actions]

✅ With the latest revision this PR passed the Python code formatter.

[DavidSpickett]

Sorry for the delay, was busy with dev meeting and so on.

There are some 3 component triples that, if you removed the vendor, would not be compatible with each other. For example aarch64-amazon-linux (#136114).

So this is why you are only doing this conversion for 4 element triples, right?

A 4 element triple should have the environment / c library (never quite sure what that bit is) specified. Instead of doing something like implying it from the vendor like aarch64-amazon-linux does (for better or worse).

Or in other words, vendor should not change ABI, if the ABI part has been specified already.

Though I wonder why vendor is even a thing, if it has no impact on anything. I guess for fully specified "triples" (quads), that might be expected, but for 3 element triples the vendor would be significant.

If this is your logic, please add that in a comment in the file and in the PR description. That way if we are wrong, it will be easier for someone to state why.

Also if you encountered this through some specific scenario, add that in the PR description. Might help someone who suddenly has more tests running, or not running, after this change.

[DavidSpickett]

Without vendor assuming the vendor is not significant to ABI.

Maybe you could say:
Native compilation: host arch == default triple arch (for ABI purposes)

[tambry]

Background

I was building for Debian and passing DEB_HOST_MULTIARCH from dpkg-architecture as LLVM_HOST_TRIPLE. LLVM however strongly prefers 4 element target triples so I switched to passing 4 element tiples for LLVM_DEFAULT_TARGET_TRIPLE and keeping LLVM_HOST_TRIPLE as 3 element. E.g. x86_64-linux-gnu vs x86_64-pc-linux-gnu. This revealed that issue that everything more or less worked, sans a bunch of tests not being run.

I however went ahead and switched to 4 element triples whereever possible to better follow LLVM upstream and reduce headaches caused by the 3 vs 4 element difference throughout the stack. So I no longer really require this change myself.

PR

Looking at this again based on your ideas... We're using a llvm::Triple::VendorType to decide ABI-relevant things in quite many cases. There's also llvm::Triple::isCompatibleWith() which handles some of the cases. I think the "native" part here is taken to mean "it can run on the host system"? Which sounds like it'd still be true even with the minor ABI changes. But I'm not too certain about that... I'll just close this out rather than making an iffy possibly-not-quite-correct change.

Thanks for the review!