[PATCH] D128876: [RISCV] Fix wrong register rename for store value during make-compressible optimization

[Alex Bradbury via Phabricator via llvm-commits]

asb accepted this revision.
asb added a comment.
This revision is now accepted and ready to land.

LGTM, though see suggestion about slightly more detailed comment.



================
Comment at: llvm/lib/Target/RISCV/RISCVMakeCompressible.cpp:298
+
+  // Skip first operand for store instruction, it's operand for store value,
+  // it's unsafe to rename if offset is non-zero.
----------------
This is a bit subtle, but easy to understand when spelled out so I'd be tempted to write a slightly more in-depth comment. e.g.:

Perhaps "Skip the first (value) operand to a store instruction (except if the store offset is zero) in order to avoid an incorrect transformation. e.g. `sd a0, 808(a0)` to `addi a2, a0, 768; sd a2, 40(a2)`"


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128876