This is an archive of the discontinued LLVM Phabricator instance.

[Triple] Add method for triple canonicalization
AbandonedPublic

Authored by phosek on May 3 2017, 3:29 PM.

Details

Summary

Today, even with normalization it's still possible to get different string representation for the same triple. Specifically, using the short form <arch>-<os> after normalization returns <arch>--<os> (even though the vendor is correctly recognized as unknown) while <arch>-unknown-<os> gives <arch>-unknown-<os>. There should be a way to produce a single canonical triple variant independent of the input.

This is especially desirable in the Clang driver where triple may be used to specify path on the filesystem for a particular target.

Diff Detail

Repository
rL LLVM

Event Timeline

phosek created this revision.May 3 2017, 3:29 PM
phosek added a comment.May 3 2017, 3:31 PM

I have also considered just adding a default argument to normalize (e.g. bool Canonicalize = true) would that be more preferable?

rengolin added inline comments.May 5 2017, 7:29 AM
lib/Support/Triple.cpp
916

A canonical representation should not have optional features.

Also, it should actually have 5 fields, including the object format, which is needed by FreeBSD.

rengolin edited edge metadata.May 5 2017, 7:39 AM

Also, where would this be used?

Also, where would this be used?

The plan was to use this in D32613 in the getTargetDir function to ensure that we always get the same path, no matter whether the user specified the target as <arch>-fuchsia or <arch>-unknown-fuchsia. I can do that transformation in the Fuchsia driver, but there are some other drivers where this might be useful, e.g. https://reviews.llvm.org/diffusion/L/browse/cfe/trunk/lib/Driver/ToolChains/CloudABI.cpp;302457$109, but maybe it'd be better to just put a utility method into a ToolChain class?

rengolin requested changes to this revision.May 20 2017, 1:10 PM

Right, I think this is a useful thing to have, but it needs a better plan.

To solve your other problem, I recommend implementing there, locally, for fucsia.

The triple handling, both in clang and llvm, is really confusing and broken. We already have one canonicalisation process and it clearly doesn't work for all cases, adding a new one won't help.

After you solve the problem for fucsia, you should go on the list and bring a proposal to have a single canonicalisation process and representation (Daniel Sanders and Simon Dardis were working on one, with Eric Christopher's help).

The biggest problem is that there are too many users of the triple and none of them use quite the same things in the same way.

Once we have a consensus on the list, we should be producing patches. Not before.

cheers,
--renato

This revision now requires changes to proceed.May 20 2017, 1:10 PM
phosek abandoned this revision.Oct 30 2018, 12:25 PM

This is no longer needed.