[PATCH] D156383: [RISCV][GlobalISel] Legalize constants, undefined values, extension instructions, and (un)merge instructions for narrow types

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.cpp:36
+
+    if (DstSize == 2 * XLen && !Query.Types[0].isVector())
+      return false; // Extending to a scalar s128 needs narrowing.
----------------
Can't we just handle 2 * XLen in the next if. We're not trying to support vectors so we don't need a vector exception


================
Comment at: llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.cpp:37
+    if (DstSize == 2 * XLen && !Query.Types[0].isVector())
+      return false; // Extending to a scalar s128 needs narrowing.
+
----------------
This comment is wrong. This isn't s128 if XLen is 32.


================
Comment at: llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.cpp:41
+    // make sure it's a power of 2.
+    if (DstSize < 1 || DstSize > 2 * XLen || !isPowerOf2_32(DstSize))
+      return false;
----------------
Why are we allowing 2 and 4 here?


================
Comment at: llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.cpp:47
+    // Make sure we fit in a register otherwise. Don't bother checking that
+    // the source type is below 128 bits. We shouldn't be allowing anything
+    // through which is wider than the destination in the first place.
----------------
128 here is also wrong


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156383