This is an archive of the discontinued LLVM Phabricator instance.

[ARM64] Simplify RegisterInfos_arm64.h with macro based RegisterInfo array
ClosedPublic

Authored by omjavaid on Aug 29 2019, 3:21 AM.

Details

Summary

This patches paves way for upcoming SVE RegisterInfo definitions. This is cosmetic change which allows us to define ARM64 RegisterInfo using macros.

In future we ll have define two different RegisterInfos to choose between SVE vs non-SVE RegisterInfo with decision being made at thread creation.

Diff Detail

Repository
rL LLVM

Event Timeline

omjavaid created this revision.Aug 29 2019, 3:21 AM

LGTM. In debugserver we have the definition for the V registers invaliding the D and S registers it contains. If the user modifies v10, we want any cached s10 and d10 values to be marked as invalid / refresh them. The same thing with the X and W registers. But the definitions you're rewriting already had this as incorrect, you've just replicated it more compactly, so I wouldn't call that a reason to hold off. Let's give Pavel a chance to look at this too.

labath accepted this revision.Aug 29 2019, 11:30 PM

Looks fine to me.

This revision is now accepted and ready to land.Aug 29 2019, 11:30 PM

Just a few nits around DEFINE_GPR64 and how the "alt" names are specified. See inline comments.

source/Plugins/Process/Utility/RegisterInfos_arm64.h
492 ↗(On Diff #217812)

Since most definitions don't have an alt, might be nice to make two:

DEFINE_GPR64(reg, generic_kind) ...
DEFINE_GPR64_ALT(reg, alt, generic_kind) ...

Also be nice to not assume "alt" is already a string and just add the #alt into the definition. See comment below for more details

534 ↗(On Diff #217812)

If we make changes to remote "alt" from DEFINE_GPR64 this becomes:

DEFINE_GPR64(x0, LLDB_REGNUM_GENERIC_ARG1),
563 ↗(On Diff #217812)

If we make suggested changes to DEFINE_GPR64 by adding a DEFINE_GPR64_ALT this becomes:

DEFINE_GPR64_alt(fp, x29, LLDB_REGNUM_GENERIC_FP),

Note no quotes on the "x29"

omjavaid marked 3 inline comments as done.Sep 2 2019, 4:20 AM

Thanks @clayborg suggestions noted and will be fixed in committed patch.

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptSep 2 2019, 4:52 AM