This is an archive of the discontinued LLVM Phabricator instance.

[mips] Add MIPS ABI enumeration and getter function to the Triple class
AbandonedPublic

Authored by atanasyan on Oct 11 2017, 7:31 AM.

Details

Summary

MIPS uses multiple ABIs. Now LLVM supports three of them: O32, N32, and N64. The Triple::Environment has items GNU, GNUABIN32, and GNUABI64, but these items do not cover all possible cases. Ideally, we have to add something like AndroidN64, MuslN32, MuslN64 etc.

This change selects another approach. It adds MipsABI enumeration to the Triple class and treats MIPS ABI like the SubArchType. To recognize ABI it parses "environmet" component of a target triple and retrieve ABI type from it.

This change allows to keep selected MIPS ABI in one place - a target triple. In the next patches I'm going to stop using MCTargetOptions::ABIName for propagating MIPS ABI and to fix some issues which require type of selected MIPS ABI in places where MCTargetOptions::ABIName is not available at all (like MipsMCAsmInfo).

Diff Detail

Repository
rL LLVM

Event Timeline

atanasyan created this revision.Oct 11 2017, 7:31 AM
rengolin edited edge metadata.Oct 23 2017, 1:54 PM

Hi Simon,

I really don't like treating Mips different than all others. But I really don't know enough about the Mips ABI problems to come up with a better way (all I know it's that they're worse than ARM's ABI problems).

I'm hoping that Eric knows better. :)

--renato

Hi Renato,

I really don't like treating Mips different than all others. But I really don't know enough about the Mips ABI problems to come up with a better way (all I know it's that they're worse than ARM's ABI problems).

This patch is really MIPS specific. But I think the problem is actually more general. Now, for example, we have three "environment" GNU, EABI, and Musl and "HF" ABI variants for each of this environment. If we support more C libraries, we will have to add new Environment items for each supported environment+ABI pair. Something like NewLib, NewLibHF etc. The idea of this patch is to separate notions of Environment and ABI and escape growing the Environment enumeration if we need to add new ABI for existing "environment".

This patch is really MIPS specific. But I think the problem is actually more general. Now, for example, we have three "environment" GNU, EABI, and Musl and "HF" ABI variants for each of this environment. If we support more C libraries, we will have to add new Environment items for each supported environment+ABI pair. Something like NewLib, NewLibHF etc. The idea of this patch is to separate notions of Environment and ABI and escape growing the Environment enumeration if we need to add new ABI for existing "environment".

I agree that we could clean up a lot of crud by separating environment from ABI, but this is not what this patch is doing, at least for every other non-Mips architecture. Adding more special treatments to the already special Triple class won't help.

I also agree that ARM hasn't made that easy to begin with, and a lot of it is ARM's fault (the arch, not the company:).

@echristo do you have any visibility on Triple changes that would help this cleanup to happen soon?

cheers,
--renato

This patch is really MIPS specific. But I think the problem is actually more general. Now, for example, we have three "environment" GNU, EABI, and Musl and "HF" ABI variants for each of this environment. If we support more C libraries, we will have to add new Environment items for each supported environment+ABI pair. Something like NewLib, NewLibHF etc. The idea of this patch is to separate notions of Environment and ABI and escape growing the Environment enumeration if we need to add new ABI for existing "environment".

I agree that we could clean up a lot of crud by separating environment from ABI, but this is not what this patch is doing, at least for every other non-Mips architecture. Adding more special treatments to the already special Triple class won't help.

I also agree that ARM hasn't made that easy to begin with, and a lot of it is ARM's fault (the arch, not the company:).

From my point of view, we have two options: a) exclude ABI information and pass it everywhere as a separate argument; b) keep ABI inside a triple, but separate environment and ABI entities. A disadvantage of the first option is that we have to change a lot of public API including the "C" interface. This patch tries to go alongside the second option. I agree that the patch changes MIPS parts only and does not touch ARM-specific ABIs. I'd be happy to provide more general solution and extend idea of the patch on other architectures if I get some sort of approval.

atanasyan abandoned this revision.Jul 4 2018, 7:29 AM