This is an archive of the discontinued LLVM Phabricator instance.

[AArch64] Check 128-bit Sysreg Builtins
ClosedPublic

Authored by lenary on Dec 16 2022, 8:03 AM.

Details

Summary

This patch contains several related changes:

  1. We move to using TARGET_BUILTIN for the 128-bit system register builtins to give better error messages when d128 has not been enabled, or has been enabled in a per-function manner.
  1. We now validate the inputs to the 128-bit system register builtins, like we validate the other system register builtins.
  1. We update the list of named PSTATE accessors for MSR (immediate), and now correctly enforce the expected ranges of the immediates. There is a long comment about how we chose to do this to comply with the ACLE when most of the PSTATE accessors for MSR (immediate) have aliased system registers for MRS/MSR which expect different values. In short, the MSR (immediate) names are prioritised, rather than falling-back to the register form when the value is out of range.

Diff Detail

Event Timeline

lenary created this revision.Dec 16 2022, 8:03 AM
Herald added a project: Restricted Project. · View Herald TranscriptDec 16 2022, 8:03 AM
lenary requested review of this revision.Dec 16 2022, 8:03 AM
Herald added a project: Restricted Project. · View Herald TranscriptDec 16 2022, 8:03 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
tmatheson accepted this revision.Dec 20 2022, 4:45 AM

Couple of nits, since you will be updating this anyway after dropping D140221, otherwise LGTM.

clang/lib/Sema/SemaChecking.cpp
8297–8298

It might be more readable to outline this whole branch and remove the redundant "else".

8298–8300

This comment style is a bit confusing; the actual code says if(NOT a write) return false; imo the code is readable enough without them.

8336–8342

nit: stick to early return pattern

This revision is now accepted and ready to land.Dec 20 2022, 4:45 AM
lenary updated this revision to Diff 484281.Dec 20 2022, 8:09 AM
lenary edited the summary of this revision. (Show Details)
lenary marked 3 inline comments as done.Dec 20 2022, 8:10 AM

Nits addressed. Will land in the new year, when I'm back at work. Right now it's time for a break!

lenary marked an inline comment as not done.Dec 20 2022, 8:10 AM
lenary added inline comments.Jan 23 2023, 7:22 AM
clang/lib/Sema/SemaChecking.cpp
8297–8298

Sorry, this is not done, but I'm not sure that fundamentally helps this very long function doing too many different things be more readable. I'm going to land this today.

This revision was landed with ongoing or failed builds.Jan 23 2023, 7:25 AM
This revision was automatically updated to reflect the committed changes.