Page MenuHomePhabricator

[Driver] Improve support for Gentoo arm*-hardfloat-*-*eabi triples
AbandonedPublic

Authored by mgorny on Oct 17 2016, 11:09 AM.

Details

Summary

Add support for the alternative arm*-hardfloat-* triples that are used
by Gentoo to the GCC installation detector, in order to make it possible
to link to gcc libraries while using the standard clang -target triples.
Add tests based on armhf Gentoo environment.

Diff Detail

Event Timeline

mgorny updated this revision to Diff 74872.Oct 17 2016, 11:09 AM
mgorny retitled this revision from to [Driver] Support "hardfloat" vendor triples used by Gentoo.
mgorny updated this object.
mgorny added subscribers: cfe-commits, zlei.

It seems like we should teach Triple how to parse this triples correctly (i.e. to recognize that the vendor is ARM), and then canonicalize the environment to GNUEABIHF (or whatever)?

It seems like we should teach Triple how to parse this triples correctly (i.e. to recognize that the vendor is ARM), and then canonicalize the environment to GNUEABIHF (or whatever)?

I've attempted that but altering the triple resulted in clang being unable to find gcc files (since the triple didn't match anymore). My previous patch is available here: https://595834.bugs.gentoo.org/attachment.cgi?id=450004. If you think that I've did something stupid-wrong there and there's a better way of doing this via Triple API, I'll gladly update the patch.

Second ping. @hfinkel, could you suggest me how to proceed with this?

It seems like we should teach Triple how to parse this triples correctly (i.e. to recognize that the vendor is ARM), and then canonicalize the environment to GNUEABIHF (or whatever)?

I've attempted that but altering the triple resulted in clang being unable to find gcc files (since the triple didn't match anymore). My previous patch is available here: https://595834.bugs.gentoo.org/attachment.cgi?id=450004. If you think that I've did something stupid-wrong there and there's a better way of doing this via Triple API, I'll gladly update the patch.

I think that you should update the triple class to return meaningful results and then also enhance Clang's search logic to find the GCC installation directory.

mgorny updated this revision to Diff 80118.Dec 2 2016, 12:08 PM
mgorny retitled this revision from [Driver] Support "hardfloat" vendor triples used by Gentoo to [Driver] Improve support for Gentoo arm*-hardfloat-*-*eabi triples.
mgorny updated this object.
mgorny added a reviewer: atanasyan.
rengolin added inline comments.
lib/Driver/ToolChains.cpp
1674

You're not testing this...

test/Driver/linux-ld.c
1016

where is 4.9.3 coming from?

mgorny added inline comments.Dec 2 2016, 1:10 PM
lib/Driver/ToolChains.cpp
1674

Well, I presumed I don't have to test every single possible triple. But sure, I can add a test for that.

test/Driver/linux-ld.c
1016

It's the version used in the input tree, i.e. the two added files above.

rengolin added inline comments.Dec 2 2016, 1:17 PM
lib/Driver/ToolChains.cpp
1674

You should test every new thing that you add.

test/Driver/linux-ld.c
1016

Right, I mean is this "4.9.3" there because the driver "found" it there, or is it generated by the compiler version you have on your machine, and that's why you created the directory with "4.9.3" in it?

I'm worried that if it's the latter, the test will fail on any machine that doesn't have that compiler version. But I really don't know how it works, so it was an honest question. :)

mgorny added inline comments.Dec 2 2016, 2:18 PM
test/Driver/linux-ld.c
1016

It's there because Driver founds it in Input directory structure. I don't have that version installed locally, if that's what worries you. You could say it's intentionally different from my install but I've simply copied the version used by other Gentoo tests.

rengolin added inline comments.Dec 2 2016, 2:21 PM
test/Driver/linux-ld.c
1016

That answers my question, thanks! :)

compnerd edited edge metadata.Dec 5 2016, 5:27 PM

Please don't canonicalize the triple that way. The behavior of armv7-unknown-linux-gnueabi is different from armv7-unknown-linux-gnueabihf which is different from armv7-unknown-linux-gnueabihf -mfloat-abi=hard. I agree that this is absolutely terrible and unexpected, and a disaster from a user's perspective, but this is the world that we live in order to maintain compatibility with GCC.

mgorny added a comment.Dec 6 2016, 4:03 AM

How should I proceed, then? Was something along the lines of the earlier diff a better idea?

rengolin edited edge metadata.Dec 6 2016, 4:17 AM

I think this patch should move to canonicalise the triple as per all the others, in the right place (vendor/environment).

If you find hardfloat or on where the vendor should be, and there are no other possible vendors, just append hf to the environment and mark the vendor as unknown. This works already with gnueabihf, eabihf and muslabihf (doubts about this last one).

What the clang driver accepts is different than what the rest of the compiler should use.

mgorny added a comment.Dec 6 2016, 6:25 AM

I think this patch should move to canonicalise the triple as per all the others, in the right place (vendor/environment).

If you find hardfloat or on where the vendor should be, and there are no other possible vendors, just append hf to the environment and mark the vendor as unknown. This works already with gnueabihf, eabihf and muslabihf (doubts about this last one).

What the clang driver accepts is different than what the rest of the compiler should use.

Ok but I think that then clang will not be able to find the gcc directory. Am I missing something obvious?

We already massage the triple in many cases, and what goes into IR is not always the same as what is used in Clang.

Examples are x86_64's "x32" extension to the ABI, ARM -> Thumb triples, adding compulsory dashes and unknowns (ex. x86_64--linux-gnu), etc.

You just have to find the right places.

mgorny abandoned this revision.Dec 12 2016, 7:33 AM

I give up. I'm just going to complain to Gentoo ARM people a lot and maybe they'll fix their triples.