Page MenuHomePhabricator

Switch kryo to use -mcpu=cortex-a57 when invoking the assembler
ClosedPublic

Authored by pirama on Nov 26 2017, 2:14 PM.

Details

Summary

Using -no-integrated-as causes -mcpu=kryo to fail since GNU assembler
doesn't recognize kryo. Cortex-a57 is the closest CPU to kryo that the
assembler does recognize. So we should switch the assembler to use
that instead.

Diff Detail

Repository
rL LLVM

Event Timeline

pirama created this revision.Nov 26 2017, 2:14 PM
pirama updated this revision to Diff 124318.Nov 26 2017, 2:40 PM

Few refactorings.

srhines added inline comments.Nov 27 2017, 9:24 AM
lib/Driver/ToolChains/Gnu.cpp
660 ↗(On Diff #124318)

Is it better to sink A into the if condition again?

test/Driver/as-mcpu.c
1 ↗(On Diff #124318)

cortex-a15 - this is missing the "a".

6 ↗(On Diff #124318)

cortex-a57

pirama updated this revision to Diff 124410.Nov 27 2017, 9:58 AM

Address review comments.

pirama updated this revision to Diff 124412.Nov 27 2017, 9:59 AM

Add missing space.

pirama marked 3 inline comments as done.Nov 27 2017, 10:00 AM
srhines added inline comments.Nov 27 2017, 10:01 AM
lib/Driver/ToolChains/Gnu.cpp
661 ↗(On Diff #124412)

Arg *A can also get moved here.

pirama updated this revision to Diff 124415.Nov 27 2017, 10:04 AM

Sink 'Arg *' declaration.

srhines accepted this revision.Nov 27 2017, 10:05 AM
This revision is now accepted and ready to land.Nov 27 2017, 10:05 AM

Am I correct in assuming this is going to be a problem for Falkor and Saphira as well? If so, can you add solutions for those as well? Cortex-a57 should be good enough for those targets as well.

pirama updated this revision to Diff 124417.Nov 27 2017, 10:28 AM

Normalize falkor and saphira as well.

mcrosier accepted this revision.Nov 27 2017, 11:08 AM
This revision was automatically updated to reflect the committed changes.

Thanks for the review. Now let's just hope the windows bots stay happy :)

Thanks for the review. Now let's just hope the windows bots stay happy :)

Actually, I just checked and it looks like falkor and saphira were both added as of a few weeks ago. I'll revert this part of the patch shortly.

Thanks for the review. Now let's just hope the windows bots stay happy :)

Actually, I just checked and it looks like falkor and saphira were both added as of a few weeks ago. I'll revert this part of the patch shortly.

Done so in r319323.