[PATCH] D131954: [LoongArch] Support Load and Store with 14-bit signed immediate operands

[WÁNG Xuěruì via Phabricator via llvm-commits]

xen0n added inline comments.


================
Comment at: llvm/lib/Target/LoongArch/LoongArchInstrInfo.td:952
+          (LDPTR_W BaseAddr:$rj, simm14_lsl2:$imm14)>;
+def : Pat<(i64 (load (AddLike BaseAddr:$rj, simm14_lsl2:$imm14))),
+          (LDPTR_D BaseAddr:$rj, simm14_lsl2:$imm14)>;
----------------
xen0n wrote:
> I'm not sure if these `AddLike`'s could be simplified into plain `add`.
> 
> @wangleiat any opinion?
So `AddLike` is needed due to frame index processing producing `or` nodes (confirmed off-line). No problem then.


================
Comment at: llvm/test/CodeGen/LoongArch/ldptr.ll:5
+
+;; Check that ldptr_w is not emitted for samll offsets.
+define signext i32 @ldptr_w_too_small_offset(ptr %p) nounwind {
----------------
"small" ;-)


================
Comment at: llvm/test/CodeGen/LoongArch/ldptr.ll:61
+
+;; Check that ldptr_d is not emitted for samll offsets.
+define i64 @ldptr_d_too_small_offset(ptr %p) nounwind {
----------------
same here


================
Comment at: llvm/test/CodeGen/LoongArch/ldptr.ll:65-67
+; LA32-NEXT:    ld.w $a2, $a0, 2040
+; LA32-NEXT:    ld.w $a1, $a0, 2044
+; LA32-NEXT:    move $a0, $a2
----------------
Hmm, I wonder why LLVM isn't producing `ld.w $a1, $a0, 2044` then simply `ld.w $a0, $a0, 2040`... I don't know if there's strong requirement to read the low bits first, but we're required to split the 64-bit access into two already, so I think it's not the case.

It would take another patch to fix this codegen nit though, if it's truly unexpected. No need to hurry this time.


================
Comment at: llvm/test/CodeGen/LoongArch/stptr.ll:5
+
+;; Check that stptr_w is not emitted for samll offsets.
+define void @stptr_w_too_small_offset(ptr %p, i32 signext %val) nounwind {
----------------
"small", also here.


================
Comment at: llvm/test/CodeGen/LoongArch/stptr.ll:58
+
+;; Check that stptr_d is not emitted for samll offsets.
+define void @stptr_d_too_small_offset(ptr %p, i64 %val) nounwind {
----------------
and here.

And by the way it's better to refer to the instruction in its mnemonic form, i.e. "stptr.d", because this is a natural language sentence so we aren't forced to speak valid identifiers at all times. Please change all such occurrences.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131954