[PATCH] D152236: [RISCV] Add special case to selectImm for constants that can be created with (ADD (SLLI C, 32), C).

[Philip Reames via Phabricator via llvm-commits]

reames added a comment.

General approach seems entirely reasonable to me.



================
Comment at: llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp:212
+  // constant pool.
+  if (Seq.size() > 2) {
+    int64_t LoVal = SignExtend64<32>(Imm);
----------------
Very minor: This code can produce a sequence with minimum length of 3, so you can avoid running it when the original sequence length is 3.  


================
Comment at: llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp:225
+            0);
+        return SDValue(CurDAG->getMachineNode(RISCV::ADDI, DL, VT, Lo, SLLI),
+                       0);
----------------
If you swap these operands, does that help add immediate formation when the two halves are very small?

I'm thinking of the case 0x00020002.  This can be done as LI + SHL + ADDI.  With only a single register.  

This might not matter, and we might need a second special case for this, if so, feel free to punt to future patch.  

Thinking about, it's probably a subcase of having a general 32 bit constant hi, + a 12 bit low half which can always be done in 4 instructions, and 3 depending on the exact details of the high half.  


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152236