[PATCH] D92105: [RISCV] Add pre-emit pass to make more instructions compressible

[Jessica Clarke via Phabricator via llvm-commits]

jrtc27 added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVMakeCompressible.cpp:146
+static bool isCompressibleLoad(const unsigned Opcode) {
+  return Opcode == RISCV::LW || Opcode == RISCV::FLW || Opcode == RISCV::LD ||
+         Opcode == RISCV::FLD;
----------------
You don't want to be transforming FLW on RV64, that's reused for the LD encodings (ditto stores)


================
Comment at: llvm/lib/Target/RISCV/RISCVMakeCompressible.cpp:164
+//              uncompressed).
+//   {RISCV::NoRegister, 0}  - No suitable optimization found for this
+//                            instruction.
----------------
Weird spacing


================
Comment at: llvm/test/CodeGen/RISCV/make-compressible-i32-f-d.mir:1
+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
+# RUN: llc -o - %s -mtriple=riscv32 -mattr=+c,+f,+d -simplify-mir \
----------------
The names for these are a bit dodgy IMO in that they encode far too much about the specific instruction sets used, so changing that requires changing the name. Maybe make-compressible.mir and the RV64-only one to be make-compressible-rv64.mir? Or split fp out from i32 into its own file?


================
Comment at: llvm/test/CodeGen/RISCV/make-compressible-i32-f-d.mir:42
+  entry:
+    %g = ptrtoint i32* %p to i32
+    store volatile i32 1, i32* %p, align 4
----------------
Please don't, ptrtoint is a terrible thing... store `%p` itself to a bitcasted `%p`


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

https://reviews.llvm.org/D92105