This is an archive of the discontinued LLVM Phabricator instance.

[ARM] Restructure cpu handling in the driver to mostly use the triple
ClosedPublic

Authored by john.brawn on May 20 2015, 6:55 AM.

Details

Summary

Using the target cpu to determine some behaviour is sprinkled in several places in the driver, but in almost all the information that is needed can be found in the triple. Restructure things so that the triple is used, and the cpu is only used if the exact cpu name is needed.

Also add a check that the -mcpu argument is valid, and correct the -march argument checking so that it handles -march=native correctly. I would have liked to move these checks into the computation of the triple, but the triple is calculated several times in several places and that would lead to multiple error messages for the same thing.

Diff Detail

Repository
rL LLVM

Event Timeline

john.brawn updated this revision to Diff 26149.May 20 2015, 6:55 AM
john.brawn retitled this revision from to [ARM] Restructure cpu handling in the driver to mostly use the triple.
john.brawn updated this object.
john.brawn edited the test plan for this revision. (Show Details)
john.brawn added a reviewer: rengolin.
john.brawn set the repository for this revision to rL LLVM.
john.brawn added a subscriber: Unknown Object (MLST).
rengolin edited edge metadata.May 20 2015, 6:59 AM

Hi John,

I'm about to commit some changes to ARMTargetParser that may affect your patch. I have tested both LLVM and Clang with it and found no bugs, but that doesn't guarantee much. I've also added a lot more unit tests to the Triple interface, which should stabilise things.

I was going to refactor this part later, but it seems that your changes are mostly orthogonal to what I planned. I'd welcome if you could rebase your changes after I send my patch, to make sure they still work as you'd expect.

cheers,
--renato

Hi John,

The changes look fine with two comments. Don't worry too much if ARMTargetParser is a bit confusing, once we got all arch / cpu changes into it, I'll refactor it to make more sense. For now, I'm trying hard to not change the behaviour in any way.

cheers,
--renato

lib/Driver/Tools.cpp
620

These two should either go into Triple or ARMTargetParser. I'll do that move later, for now, they can stay here. Just add a comment:

// FIXME: Move to ARMTargetParser.
780

Not any more. Check ARMTargetParser::getCanonicalArch() and parseArch().

I've checked the iOS/Darwin behaviour and it seems reasonable from that perspective too.

Tim.

john.brawn updated this revision to Diff 26220.May 21 2015, 5:05 AM
john.brawn edited edge metadata.

Add a couple of FIXME comments, and use ARMTargetParser::getCanonicalArchName to decide if the -march argument is valid. Doing that means moving the handling of -march=native from getARMCPUForMArch to getARMArch, which I think should be OK.

rengolin accepted this revision.May 21 2015, 5:10 AM
rengolin edited edge metadata.

With the extra comment, LGTM.

Thanks!
--renato

lib/Driver/Tools.cpp
5732

FIXME: Move to ARMTargetParser.

This revision is now accepted and ready to land.May 21 2015, 5:10 AM
This revision was automatically updated to reflect the committed changes.