[PATCH] D66973: [RISCV] Switch to the Machine Scheduler

[Sam Elliott via Phabricator via llvm-commits]

lenary added a comment.

Three nits about changes to the testcases that go beyond just reordering.

I don't think this should block the patch from landing, but I do think that we should keep an eye on why they are happening, and hope to get rid of them when we introduce a schedule.



================
Comment at: llvm/test/CodeGen/RISCV/atomic-cmpxchg.ll:2176
 ; RV32I-NEXT:    sw ra, 12(sp)
+; RV32I-NEXT:    mv a5, a4
 ; RV32I-NEXT:    sw a2, 4(sp)
----------------
This patch does seem to have added some additional `mv` instructions in this test (and others). It's usually because there's some kind of two-way register copy happening, which now uses a temporary register.


================
Comment at: llvm/test/CodeGen/RISCV/callee-saved-fpr32s.ll:109
 ; ILP32F-LP64F-NEXT:    lui a0, %hi(var)
+; ILP32F-LP64F-NEXT:    flw ft0, %lo(var)(a0)
 ; ILP32F-LP64F-NEXT:    addi a1, a0, %lo(var)
----------------
I don't know where this came from, but having both `flw ... %lo(var)(a0)` followed by `addi ... %lo(var)` does not seem so amenable for linker relaxation, though @jrtc27 would have a better idea about this than me.




================
Comment at: llvm/test/CodeGen/RISCV/callee-saved-gprs.ll:46
+; RV32I-NEXT:    sw a1, 28(sp)
+; RV32I-NEXT:    addi a2, a0, %lo(var)
 ;
----------------
We seem to have gained a `lw; sw` pair here. Not entirely sure why. There are a few additions like this in this testcase.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66973