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

[Jessica Clarke via Phabricator via llvm-commits]

jrtc27 added a comment.

Some comments about current hard-coded assumptions. Whilst only supporting LW/SW at the moment is fine, I don't think it's a good idea to have such assumptions embedded in the design; instead it should be approximately general enough that other widths and types can be added at a later date without overhauling its core (i.e. just adding some more cases). Though at that point you might as well just support LD/SD and FLW/FSW/FLD/FSD given it'd then be trivial...



================
Comment at: llvm/lib/Target/RISCV/RISCVCompressInstrs.cpp:99
+
+static const uint8_t CSWOffsetMask = 0b1111100;
+
----------------
Compute this based on the size of the register being loaded/stored, as this approach does not generalise.


================
Comment at: llvm/lib/Target/RISCV/RISCVCompressInstrs.cpp:104
+// Return 0 if the offset is already compressible.
+static int64_t getBaseAdjustForCompression(int64_t Offset) {
+  if (isShiftedUInt<5, 2>(Offset))
----------------
You'll need the type here.


================
Comment at: llvm/lib/Target/RISCV/RISCVCompressInstrs.cpp:141
+    if (RISCV::SPRegClass.contains(Base)) {
+      if (!isShiftedUInt<6, 2>(Offset) && NewBaseAdjust)
+        return RegImmPair(Base, NewBaseAdjust);
----------------
Ditto; compute this based on the size of the register.


================
Comment at: llvm/lib/Target/RISCV/RISCVCompressInstrs.cpp:189
+  }
+  return RegImmPair(Register(0), 0);
+}
----------------
NoRegister


================
Comment at: llvm/lib/Target/RISCV/RISCVCompressInstrs.cpp:244
+  // instruction we care about to the last.
+  return RS.scavengeRegisterBackwards(RISCV::GPRCRegClass,
+                                      FirstMI.getIterator(),
----------------
You need the type of the register (int or float) in order to support compressing floating-point loads/stores (and so we can support capabilities in our downstream CHERI fork, both for the base and for the source/destination).


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

https://reviews.llvm.org/D92105