[PATCH] D52149: add support functions for hard/soft float multilib distinction

Mon Oct 1 03:52:42 PDT 2018

kristina added a comment.

In https://reviews.llvm.org/D52149#1250710, @peter.smith wrote:

> I've added another couple of reviewers.
>
> At this stage I'm not yet sure that it is worth adding this functionality to Triple. In https://reviews.llvm.org/D52705 it seems like this is used to get the HF or non HF string name for the provided triple only when multilibs are used on Arm. As a result a lot of the code in the function (x86 handling) won't ever be exercised as it won't be called for other targets. A name change of the functions to getARMSoftFloatVariant() or getARMHardFloatVariant() and only handle the Arm cases would be a bit more honest, but it also points out that perhaps it would be better to handle as a local function called from findArmEABIMultilibs()?
>
> The similar functions getLittleEndianVariant() and getBigEndianVariant() in Triple are slightly more general. They are called for all targets and there are also at least 3 targets (Arm, AArch64 and Mips) that include the endianness in the Target. This makes it more useful to handle the functionality in Triple.
>
> If the consensus is to go ahead with this in triple please do add some tests to llvm/unittests/ADT/TripleTest.cpp

This mostly depends on the Clang change and if it goes through or not, I think. I think it's okay to land this for convenience for now (once the issues are addressed) and if the Clang differential does not get approved revert this, which is not especially difficult since the support lib is quite isolated. The core issue is in whether the diff that depends on this get approved or not, but I think since it does no harm and is not likely to be used by anything else, it would be okay to land for now and revert it if the Clang change is not approved. In either case this is better as a separate diff rather than a part of the Clang-specific diff, because of its isolation.

Repository:
  rL LLVM

https://reviews.llvm.org/D52149