[AArch64] Add support for missing AArch64 opcodes in AArch64InstrInfo::getMemOpInfo

[romainthomas]

This PR is a small update to support more memory instructions in AArch64InstrInfo::getMemOpInfo

cc @sdesmalen-arm

[github-actions]

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be
notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write
permissions for the repository. In which case you can instead tag reviewers by
name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review
by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate
is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

[llvmbot]

@llvm/pr-subscribers-backend-aarch64

Author: Romain Thomas (romainthomas)

Changes

This PR is a small update to support more memory instructions in AArch64InstrInfo::getMemOpInfo

cc @sdesmalen-arm

---

Full diff: https://github.com/llvm/llvm-project/pull/97954.diff

1 Files Affected:

• (modified) llvm/lib/Target/AArch64/AArch64InstrInfo.cpp (+42)

diff --git a/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp b/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
index 93278a2ba0d09..2e383a1b51afd 100644
--- a/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
+++ b/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
@@ -3587,6 +3587,15 @@ bool AArch64InstrInfo::getMemOpInfo(unsigned Opcode, TypeSize &Scale,
   case AArch64::STURWi:
   case AArch64::STURSi:
   case AArch64::STLURWi:
+  case AArch64::LDRSpre:
+  case AArch64::LDRWpre:
+  case AArch64::LDRSWpre:
+  case AArch64::LDTRSWi:
+  case AArch64::STRSpre:
+  case AArch64::STRWpre:
+  case AArch64::LDTRWi:
+  case AArch64::LDTRXi:
+  case AArch64::STTRWi:
     Width = TypeSize::getFixed(4);
     Scale = TypeSize::getFixed(1);
     MinOffset = -256;
@@ -3602,6 +3611,16 @@ bool AArch64InstrInfo::getMemOpInfo(unsigned Opcode, TypeSize &Scale,
   case AArch64::STURHi:
   case AArch64::STURHHi:
   case AArch64::STLURHi:
+  case AArch64::LDRHHpre:
+  case AArch64::STRHHpre:
+  case AArch64::LDRHpre:
+  case AArch64::STRHpre:
+  case AArch64::LDRSHWpre:
+  case AArch64::LDRSHXpre:
+  case AArch64::LDTRHi:
+  case AArch64::LDTRSHWi:
+  case AArch64::LDTRSHXi:
+  case AArch64::STTRHi:
     Width = TypeSize::getFixed(2);
     Scale = TypeSize::getFixed(1);
     MinOffset = -256;
@@ -3617,6 +3636,16 @@ bool AArch64InstrInfo::getMemOpInfo(unsigned Opcode, TypeSize &Scale,
   case AArch64::STURBi:
   case AArch64::STURBBi:
   case AArch64::STLURBi:
+  case AArch64::LDRBpre:
+  case AArch64::STRBpre:
+  case AArch64::LDRBBpre:
+  case AArch64::STRBBpre:
+  case AArch64::LDRSBWpre:
+  case AArch64::LDRSBXpre:
+  case AArch64::LDTRBi:
+  case AArch64::LDTRSBWi:
+  case AArch64::LDTRSBXi:
+  case AArch64::STTRBi:
     Width = TypeSize::getFixed(1);
     Scale = TypeSize::getFixed(1);
     MinOffset = -256;
@@ -3676,6 +3705,12 @@ bool AArch64InstrInfo::getMemOpInfo(unsigned Opcode, TypeSize &Scale,
   case AArch64::STPSi:
   case AArch64::STNPWi:
   case AArch64::STNPSi:
+  case AArch64::LDPSpre:
+  case AArch64::LDPWpre:
+  case AArch64::LDPSWi:
+  case AArch64::STPSpre:
+  case AArch64::STPWpre:
+  case AArch64::LDPSWpre:
     Scale = TypeSize::getFixed(4);
     Width = TypeSize::getFixed(8);
     MinOffset = -64;
@@ -3717,11 +3752,14 @@ bool AArch64InstrInfo::getMemOpInfo(unsigned Opcode, TypeSize &Scale,
   case AArch64::LDPXpost:
   case AArch64::STPDpre:
   case AArch64::LDPDpost:
+  case AArch64::LDPXpre:
+  case AArch64::LDPDpre:
     Scale = TypeSize::getFixed(8);
     Width = TypeSize::getFixed(8);
     MinOffset = -512;
     MaxOffset = 504;
     break;
+  case AArch64::LDPQpre:
   case AArch64::STPQpre:
   case AArch64::LDPQpost:
     Scale = TypeSize::getFixed(16);
@@ -3733,6 +3771,9 @@ bool AArch64InstrInfo::getMemOpInfo(unsigned Opcode, TypeSize &Scale,
   case AArch64::STRDpre:
   case AArch64::LDRXpost:
   case AArch64::LDRDpost:
+  case AArch64::LDRXpre:
+  case AArch64::LDRDpre:
+  case AArch64::STTRXi:
     Scale = TypeSize::getFixed(1);
     Width = TypeSize::getFixed(8);
     MinOffset = -256;
@@ -3740,6 +3781,7 @@ bool AArch64InstrInfo::getMemOpInfo(unsigned Opcode, TypeSize &Scale,
     break;
   case AArch64::STRQpre:
   case AArch64::LDRQpost:
+  case AArch64::LDRQpre:
     Scale = TypeSize::getFixed(1);
     Width = TypeSize::getFixed(16);
     MinOffset = -256;

[davemgreen]

Some of these are being added in #97561, but from what I can tell with different widths and limits. Im hoping to use them to detect out of range immediates at compile time.

Are you sure these values are correct? (there is a chance I got them wrong in that patch).

[romainthomas]

Some of these are being added in #97561

Thanks for pointing out this PR. In the end, feel free to merge my PR into yours
if it makes more sense to have one single PR for these updates.

But from what I can tell with different widths and limits

After checking your PR, I can also see the differences.

For instance, you define STRWpre as:

llvm-project/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp

Lines 3681 to 3686 in 1422591

	case AArch64::STRWpre:
	Width = TypeSize::getFixed(32);
	Scale = TypeSize::getFixed(4);
	MinOffset = -256;
	MaxOffset = 255;
	break;

But according to llvm-mc it follows this encoding:

str     w8, [x1, #32]! // encoding: [0x28,0x0c,0x02,0xb8]
                       // <MCInst #6847 STRWpre
                       //  <MCOperand Reg:236>
                       //  <MCOperand Reg:212>
                       //  <MCOperand Reg:236>
                       //  <MCOperand Imm:32>>

Thus, in my understanding, the Scale should be 1 and the Width 4 ?

Regarding, the Width for the instructions that load/store pair of registers,
I understand the Width as twice the size of the register being stored/loaded
so LDPSpre should be 2 * 4 and not only 4?

ldp     s15, s11, [x3, #-0x40]! // encoding: [0x6f,0x2c,0xf8,0x2d]
                                // <MCInst #4424 LDPSpre
                                //  <MCOperand Reg:238>
                                //  <MCOperand Reg:187>
                                //  <MCOperand Reg:183>
                                //  <MCOperand Reg:238>
                                //  <MCOperand Imm:-16>>

llvm-project/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp

Lines 3858 to 3870 in 1422591

	case AArch64::LDPSpost:
	case AArch64::LDPSpre:
	case AArch64::LDPWpost:
	case AArch64::LDPWpre:
	case AArch64::STPSpost:
	case AArch64::STPSpre:
	case AArch64::STPWpost:
	case AArch64::STPWpre:
	Scale = TypeSize::getFixed(4);
	Width = TypeSize::getFixed(4);
	MinOffset = -256;
	MaxOffset = 252;
	break;

Note that my updates are coming from a manual analysis of the MC encoding.

[davemgreen]

Hmm Yeah I see. Good point. Some of the existing values are a bit all-over-the-place.

I've put up #98196 to try and clean up the existing values and uses. (Github seems to be quite selective about who can be added as a reviewer). I will rebase #97561 once that goes in.