This is an archive of the discontinued LLVM Phabricator instance.

[AArch64] Make Read Write System Registers Read Only
ClosedPublic

Authored by LukeGeeson on Feb 4 2020, 4:03 AM.

Details

Summary

This patch makes the following System Registers Read Only:

  • CurrentEL
  • ICH_MISR_EL2
  • PMBIDR_EL1
  • PMSIDR_EL1

as found in:
https://developer.arm.com/docs/ddi0595/e/aarch64-system-registers

Relative line numbers were also added to the tests so we get more
informative error messages on failure.

Diff Detail

Event Timeline

LukeGeeson created this revision.Feb 4 2020, 4:03 AM
LukeGeeson edited the summary of this revision. (Show Details)Feb 4 2020, 5:13 AM

Mostly LGTM, except for test nitpicks.

llvm/test/MC/AArch64/arm64-system-encoding.s
169–170

I think it would be much more readable to move this check and the line that invoked it into their own separate section of the test file, so that the checks are right next to the input instruction.

The way you've done it in armv8.2a-statistical-profiling.s for PMBIDR_EL1 and PMSIDR_EL1 looks fine to me.

llvm/test/MC/AArch64/basic-a64-instructions.s
4059–4060

Same comment here.

llvm/test/MC/AArch64/gicv3-regs.s
203–204

And here.

ostannard requested changes to this revision.Feb 4 2020, 5:46 AM
ostannard added a subscriber: ostannard.
ostannard added inline comments.
llvm/test/MC/AArch64/arm64-system-encoding.s
169–170

The NOECV check prefix isn't used by any FileCheck commands.

llvm/test/MC/AArch64/armv8.2a-statistical-profiling.s
1–10

The stderr from this llvm-mc execution is redirected to a temp file, but not checked.

7

Why are you just checking msr, and not mrs or psb?

llvm/test/MC/AArch64/basic-a64-instructions.s
4059–4060

This should be moved to basic-a64-diagnostics.s, there's a block of tests like this around line 3600.

llvm/test/MC/AArch64/gicv3-regs.s
203–204

Move to gicv3-regs-diagnostics.s.

This revision now requires changes to proceed.Feb 4 2020, 5:46 AM
LukeGeeson updated this revision to Diff 242338.Feb 4 2020, 7:39 AM

Moved erroneous changes into diagnostic files, fixed FileCheck output to handle errors, added instructions to NO_SPE_OUT

LukeGeeson marked 8 inline comments as done.Feb 4 2020, 7:40 AM
LukeGeeson added inline comments.
llvm/test/MC/AArch64/armv8.2a-statistical-profiling.s
1–10

added FileCheck command

7

good spot, fixed

ostannard added inline comments.Feb 4 2020, 9:27 AM
llvm/test/MC/AArch64/armv8.2a-statistical-profiling.s
7

This is now checking that the string msr, mrs, psb doesn't appear in the output, which isn't what we want to test. It also doesn't need the regex. Something like this would work (not actually tested):

// NO_SPE_OUT-NOT: msr
// NO_SPE_OUT-NOT: mrs
// NO_SPE_OUT-NOT: psb
LukeGeeson updated this revision to Diff 242551.Feb 5 2020, 3:44 AM

Corrected Filecheck checks

LukeGeeson marked 2 inline comments as done.Feb 5 2020, 3:44 AM
ostannard accepted this revision.Feb 5 2020, 5:04 AM

LGTM, thanks.

This revision is now accepted and ready to land.Feb 5 2020, 5:04 AM
LukeGeeson closed this revision.Feb 10 2020, 6:38 AM