Page MenuHomePhabricator

[ARM] Introduce separate features for FP registers.
ClosedPublic

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

Details

Summary

The MVE extension in Arm v8.1-M permits the use of some move, load and
store isntructions which access the FP registers, even if there's no
actual FP support in the processor (in particular, if you have the
integer-only version of MVE).

Therefore, we need separate subtarget features to condition those
instructions on, which are implied by both FP and MVE but are not part
of either.

Diff Detail

Repository
rL LLVM

Event Timeline

simon_tatham created this revision.Apr 15 2019, 5:56 AM
ostannard added inline comments.
llvm/lib/Target/ARM/ARMInstrVFP.td
2357 ↗(On Diff #195152)

This only applies to VMSR at the moment, should it also cover the other four?

llvm/test/MC/ARM/mve-vmov-lane.s
14 ↗(On Diff #195152)

I don't think this instruction has been added at this point in the patch series, so this test should probably be moved to a later patch.

llvm/test/MC/Disassembler/ARM/mve-vmov-lane.txt
6 ↗(On Diff #195152)

Again, I think this need to be in a later patch.

simon_tatham marked an inline comment as done.Apr 17 2019, 9:00 AM
simon_tatham added inline comments.
llvm/lib/Target/ARM/ARMInstrVFP.td
2357 ↗(On Diff #195152)

Errrr. That's rather difficult to say.

The idea of the FP registers existing without FP has never come up before: the combination of v8.1-M and MVE is the first case where it's possible, which is why we haven't had to add this separate feature name before.

But according to the v8.1-M ArmARM, none of those four registers exists anyway in v8.1-M, with or without FP. FPEXC and FPINST[2] are for support code, which the modern fashion is to avoid needing anyway; FPSID could make sense in principle, but in fact it's never existed in the M profile as far as I can see.

So I think the answer is that no current version of the Arm architecture gives evidence for or against your question!

SjoerdMeijer commandeered this revision.May 30 2019, 2:34 AM
SjoerdMeijer updated this revision to Diff 202141.
SjoerdMeijer edited reviewers, added: simon_tatham; removed: SjoerdMeijer.

I've removed the instruction in the tests that are not yet supported; this will be added to another/later patch.

ostannard accepted this revision.May 30 2019, 2:49 AM

LGTM with one nit.

test/MC/ARM/mve-vmov-lane.s
3 ↗(On Diff #202141)

This comment isn't true yet, we should probably move this whole file to the patch which adds the v8.1M syntax change.

test/MC/Disassembler/ARM/mve-vmov-lane.txt
4 ↗(On Diff #202141)

Again, this isn't true yet, I think it'd be better to move the whole file to a later patch.

This revision is now accepted and ready to land.May 30 2019, 2:49 AM

Many thanks! I will remove those tests before committing!

This revision was automatically updated to reflect the committed changes.