This is an archive of the discontinued LLVM Phabricator instance.

[AArch64, ARM] Add v8.1a architecture and generic cpu
ClosedPublic

Authored by vsukharev on Mar 20 2015, 3:05 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

vsukharev updated this revision to Diff 22390.Mar 20 2015, 3:05 PM
vsukharev retitled this revision from to [AArch64, ARM] Add v8.1a architecture and generic cpu.
vsukharev updated this object.
vsukharev edited the test plan for this revision. (Show Details)
vsukharev set the repository for this revision to rL LLVM.
vsukharev added a subscriber: Unknown Object (MLST).
t.p.northover accepted this revision.Mar 20 2015, 7:11 PM
t.p.northover added a reviewer: t.p.northover.
t.p.northover added a subscriber: t.p.northover.

This looks OK, with one small clarification. (Feel free to commit with that change unless you disagree).

Tim.

lib/Target/ARM/ARMAsmPrinter.cpp
669 ↗(On Diff #22390)

I think this is a bit non-obvious. I grant that it's correct (and equivalent to !StringRef::startswith), but I think this looks too much like "if generic is in CPUString".

I'd suggest an explicit comparison against 0, which might make people at least consider whether string::find returns an index and what it returns for failure.

This revision is now accepted and ready to land.Mar 20 2015, 7:11 PM
vsukharev updated this revision to Diff 22477.Mar 23 2015, 9:08 AM
vsukharev edited edge metadata.
  • CPUString.find("generic") is replaced with more understandable construction
  • The first revision was based not on trunk, but on patches, that are not yet approved. To be able to commit, definitions of "V8_1" subtarget feature added from http://reviews.llvm.org/D8503 (AArch64) and http://reviews.llvm.org/D8498 (ARM). Now the second revision is based on ToT.

Tim, would you please approve the second change?

emaste added a subscriber: emaste.Mar 24 2015, 8:45 AM
This revision was automatically updated to reflect the committed changes.