[PATCH] D96632: [THUMB2] add .w suffixes for ldr/str w/ immediates

[David Spickett via Phabricator via llvm-commits]

DavidSpickett added a comment.

I just realised that you're only adding the ones that would encode to T4, so the answer to some of my comments will be no, T3 isn't covered. Correct?

I'm not sure how much complexity adding T3 would be. We're ok to encode any pre increment, non writeback into T3 so it could be another alias, with a different immediate check.



================
Comment at: llvm/lib/Target/ARM/ARMInstrThumb2.td:1568
+// .w suffixes; t2InstAlias isn't descriptive enough for $Rn = $Rn_wb
+// Constraint.
+def t2LDR_PRE_imm : t2AsmPseudo<"ldr${p}.w $Rt, $addr!",
----------------
Do you mean "$Rn != $Rn_wb". I see what you mean but out of the context of the armarm, != makes more immediate sense.


================
Comment at: llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp:7672
+                   "destination register and base register can't be identical");
+    if (Inst.getOpcode() == ARM::t2LDR_POST_imm ||
+        Inst.getOpcode() == ARM::t2STR_POST_imm) {
----------------
You'll add a check here for PRE too, or is that not needed for some reason?

This would use the imm12 range from the T3 encoding I think.


================
Comment at: llvm/test/MC/ARM/thumb2-ldr.w-str.w.s:81
+str.w r3, [r0, #-4]!
+
+@ CHECK: ldr r3, [r1], #4   @ encoding: [0x51,0xf8,0x04,0x3b]
----------------
Add pre-increment, no writeback here.


================
Comment at: llvm/test/MC/ARM/thumb2-ldr.w-str.w.s:91
+
+@@ LDR pre-increment
+@ Vary Rt.
----------------
You should check pre-increment without writeback, that would use the T3 encoding and have a greater immediate range.


================
Comment at: llvm/test/MC/ARM/thumb2-ldr.w-str.w.s:105
+ldr.w lr, [r1, #-4]!
+ldr.w pc, [r1, #-4]!
+@ Vary Rn.
----------------
For varying the registers you could cut it down to:
* one gpr
* pc (if allowed)
* sp (if allowed)

There's not much utility to the rest.


================
Comment at: llvm/test/MC/ARM/thumb2-ldr.w-str.w.s:126
+@ Condition codes.
+ldreq.w r1, [r0, #255]!
+ldrne.w r1, [r0, #255]!
----------------
I'd just pick two here. That shows we pass them through correctly, we can assume the rest are ok.


================
Comment at: llvm/test/MC/ARM/thumb2-ldr.w-str.w.s:296
+
+@@ STR pre-increment
+@ Vary Rt.
----------------
pre-increment without writeback


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D96632/new/

https://reviews.llvm.org/D96632