Page MenuHomePhabricator

[ARM] add target arch definitions for 8.1-M and MVE.
ClosedPublic

Authored by SjoerdMeijer on Apr 15 2019, 5:57 AM.

Details

Summary

This adds:

  • LLVM subtarget features to make all the new instructions conditional on
  • CPU and FPU names for use on armclang's command line, with default FPUs set so that "armv8.1-m.main+fp" and "armv8.1-m.main+fp.dp" will select the right FPU features
  • architecture extension names "mve" and "mve.fp"
  • ABI build attribute support for v8.1-M (a new value for Tag_CPU_arch) and MVE (a new actual tag)

Diff Detail

Repository
rL LLVM

Event Timeline

simon_tatham created this revision.Apr 15 2019, 5:57 AM

Could probably also do with some testing in unittests/Support/ARMAttributeParser.cpp and TargetParserTest.cpp.

This adds:

  • LLVM subtarget features to make all the new instructions conditional on
  • CPU and FPU names for use on armclang's command line, with default FPUs set so that "armv8.1-m.main+fp" and "armv8.1-m.main+fp.dp" will select the right FPU features
  • architecture extension names "mve" and "mve.fp"
  • ABI build attribute support for v8.1-M (a new value for Tag_CPU_arch) and MVE (a new actual tag)

Looks like they could be separate commits. But if that makes things too complicated now, I think we can deal with this initial plumbing work.

llvm/include/llvm/Support/ARMTargetParser.h
48 ↗(On Diff #195156)

I think we need to regression test this new extension in unittests/Support/TargetParserTest.cpp too.

llvm/lib/Support/ARMAttributeParser.cpp
137 ↗(On Diff #195156)

what are these nullptrs, and how many do we need? :-)
More seriously, I haven't looked into this bit before, but at least a comment would be good here I guess.

ostannard added inline comments.
llvm/lib/Support/ARMAttributeParser.cpp
137 ↗(On Diff #195156)

The nullptrs are missing strings for "Arm v8-R", "Arm v8.1-A", "Arm v8.2-A" and "Arm v8.3-A", we should add these, but probably as a separate patch.

llvm/lib/Target/ARM/ARMSubtarget.h
114 ↗(On Diff #195156)

Nit: can we add a trailing comma to tidy up the next diff?

SjoerdMeijer commandeered this revision.May 29 2019, 8:18 AM
SjoerdMeijer edited reviewers, added: simon_tatham; removed: SjoerdMeijer.

I am helping Simon a bit with upstreaming.

I have:

  • rebased this change,
  • added a targetparser test,
  • added a build attribute test,
  • addressed a nit.

More targetparser and buildattribute tests!

This revision is now accepted and ready to land.May 30 2019, 1:21 AM
This revision was automatically updated to reflect the committed changes.
mstojanovic added inline comments.
llvm/trunk/unittests/Support/TargetParserTest.cpp
578

With this addition the test runs out of memory on a MIPS board. Shouldn't this loop go through each flag in the Extensions set or at least through every combination of the flags. It seems like overkill to loop from 0 to 16916342 and push mostly the same elements into the vector each time. Even it the platform doesn't run out of memory the test will be unnecessarily slow.