[llvm] [RISCV] Generalize the (ADD (SLLI X, 32), X) special case in constant… (PR #66931)

[Philip Reames via llvm-commits]

================
@@ -195,28 +195,41 @@ static SDValue selectImm(SelectionDAG *CurDAG, const SDLoc &DL, const MVT VT,
   RISCVMatInt::InstSeq Seq =
       RISCVMatInt::generateInstSeq(Imm, Subtarget.getFeatureBits());
 
-  // See if we can create this constant as (ADD (SLLI X, 32), X) where X is at
+  // See if we can create this constant as (ADD (SLLI X, C), X) where X is at
   // worst an LUI+ADDIW. This will require an extra register, but avoids a
   // constant pool.
   // If we have Zba we can use (ADD_UW X, (SLLI X, 32)) to handle cases where
   // low and high 32 bits are the same and bit 31 and 63 are set.
   if (Seq.size() > 3) {
     int64_t LoVal = SignExtend64<32>(Imm);
-    int64_t HiVal = SignExtend64<32>(((uint64_t)Imm - (uint64_t)LoVal) >> 32);
-    if (LoVal == HiVal ||
-        (Subtarget.hasStdExtZba() && Lo_32(Imm) == Hi_32(Imm))) {
-      RISCVMatInt::InstSeq SeqLo =
-          RISCVMatInt::generateInstSeq(LoVal, Subtarget.getFeatureBits());
-      if ((SeqLo.size() + 2) < Seq.size()) {
-        SDValue Lo = selectImmSeq(CurDAG, DL, VT, SeqLo);
-
-        SDValue SLLI = SDValue(
-            CurDAG->getMachineNode(RISCV::SLLI, DL, VT, Lo,
-                                   CurDAG->getTargetConstant(32, DL, VT)),
-            0);
-        // Prefer ADD when possible.
-        unsigned AddOpc = (LoVal == HiVal) ? RISCV::ADD : RISCV::ADD_UW;
-        return SDValue(CurDAG->getMachineNode(AddOpc, DL, VT, Lo, SLLI), 0);
+    if (LoVal != 0) {
+      // Subtract the LoVal to emulate the effect of the final add.
+      uint64_t Tmp = (uint64_t)Imm - (uint64_t)LoVal;
+
+      // Use trailing zero counts to figure how far we need to shift LoVal to
+      // line up with the remaining constant.
+      unsigned TzLo = llvm::countr_zero((uint64_t)LoVal);
+      unsigned TzHi = llvm::countr_zero(Tmp);
+      assert(TzLo < 32 && TzHi >= 32);
+      unsigned ShiftAmt = TzHi - TzLo;
+
+      bool MatchedAdd = Tmp == ((uint64_t)LoVal << ShiftAmt);
+
+      if (MatchedAdd ||
+          (Subtarget.hasStdExtZba() && Lo_32(Imm) == Hi_32(Imm))) {
----------------
preames wrote:

The matching low/high case is now different enough I'd be tempted to just make it it's own standalone return block.  There is some duplication, but there's also a bunch of reasoning commingled with the current structure.  

https://github.com/llvm/llvm-project/pull/66931