This is an archive of the discontinued LLVM Phabricator instance.

Handle ARMv7K as an alias, instead of fake architecture (NFC)
ClosedPublic

Authored by tyomitch on Nov 17 2015, 2:33 PM.

Details

Summary

This follows D14577 to treat ARMv7K as an alias for ARMv7-A,
instead of an architecture in its own right.

Diff Detail

Event Timeline

tyomitch updated this revision to Diff 40436.Nov 17 2015, 2:33 PM
tyomitch retitled this revision from to Handle ARMv7K as an alias, instead of fake architecture (NFC).
tyomitch updated this object.
tyomitch added reviewers: t.p.northover, rengolin.
tyomitch added a subscriber: llvm-commits.
rengolin requested changes to this revision.Nov 18 2015, 3:48 AM
rengolin edited edge metadata.
rengolin added inline comments.
lib/Target/ARM/ARMSubtarget.cpp
156 ↗(On Diff #40436)

I strongly oppose to going back to string parsing all over the place.

lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp
1053 ↗(On Diff #40436)

Idem.

This revision now requires changes to proceed.Nov 18 2015, 3:48 AM
tyomitch updated this revision to Diff 40525.Nov 18 2015, 9:09 AM
tyomitch edited edge metadata.

Thanks Renato, I see your point.

How about this?

t.p.northover edited edge metadata.Nov 18 2015, 2:34 PM

I'm not really sure about this. To the extent that the arch part of the triple means anything on MachO targets, it's a CPU choice (possibly with accompanying ABI upgrades, like x86_64h).

Defaulting watchOS to Cortex-A7 achieves the right thing on existing triples, but for what seem like dodgy reasons to me.

lib/Support/Triple.cpp
1378

tvOS probably ought to default to "cyclone" if anything.

tyomitch updated this revision to Diff 40626.Nov 19 2015, 4:03 AM
tyomitch edited edge metadata.

Thanks Tim! One more attempt...

That's probably OK for now, as far as I'm concerned. If and when we ever care about v7k outside watchOS we can decide what to do about it.

That's probably OK for now, as far as I'm concerned. If and when we ever care about v7k outside watchOS we can decide what to do about it.

Thanks Tim!
Do you have any comments on D14755 (the ARMv6J patch)? @rengolin suggested you took a look...

This revision was automatically updated to reflect the committed changes.