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

[Gong LingQin via Phabricator via llvm-commits]

gonglingqin added inline comments.


================
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 {
----------------
xen0n wrote:
> "small" ;-)
Thank you for checking, I will fix it.


================
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 {
----------------
xen0n wrote:
> same here
Thanks.


================
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
----------------
xen0n wrote:
> 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.
Wow that's some serious simplification. I haven't seen optimizations like this on RISCV32 and ARM. Thanks.


================
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 {
----------------
xen0n wrote:
> "small", also here.
Thanks.


================
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 {
----------------
xen0n wrote:
> 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.
Thanks for the suggestion, I will change it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131954