[RISCV] Improve constant materialization for stores of i16 or i32 negative constants.

[craig.topper]

Summary

DAGCombiner::visitStore can clear the upper bits of constants
used by stores. This leads prevents them from being recognized as
sign extended negative values making them more expensive to
materialize.

This patch uses the hasAllNBitUsers method from D107658 to make
a negative constant if none of the users care about the upper bits.

[craig.topper]

Update comment

[luismarques]

LGTM.

I wondered if we could avoid this in the first place. I looked at the DAGCombiner::visitSTORE transformation some days ago but it wasn't obvious to me what could be done there. (BTW, can we even access the TargetTransformInfo from there, e.g. to call getIntMatCost?)

llvm/test/CodeGen/RISCV/imm.ll

481

Remove comment.

498

Ditto.

llvm/lib/Target/RISCV/RISCVISelDAGToDAG.h

public:
   bool selectShiftMask32(SDValue N, SDValue &ShAmt) {
     return selectShiftMask(N, 32, ShAmt);
   }
 
   bool selectSExti32(SDValue N, SDValue &Val);
   bool selectZExti32(SDValue N, SDValue &Val);
 
   bool hasAllNBitUsers(SDNode *Node, unsigned Bits) const;
+  bool hasAllHUsers(SDNode *Node) const { return hasAllNBitUsers(Node, 16); }
   bool hasAllWUsers(SDNode *Node) const { return hasAllNBitUsers(Node, 32); }
 
   bool selectVLOp(SDValue N, SDValue &VL);
 
   bool selectVSplat(SDValue N, SDValue &SplatVal);
   bool selectVSplatSimm5(SDValue N, SDValue &SplatVal);
   bool selectVSplatUimm5(SDValue N, SDValue &SplatVal);
   bool selectVSplatSimm5Plus1(SDValue N, SDValue &SplatVal);

llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp

void RISCVDAGToDAGISel::Select(SDNode *Node) {
   case ISD::Constant: {
     auto *ConstNode = cast<ConstantSDNode>(Node);
     if (VT == XLenVT && ConstNode->isNullValue()) {
       SDValue New =
           CurDAG->getCopyFromReg(CurDAG->getEntryNode(), DL, RISCV::X0, XLenVT);
       ReplaceNode(Node, New.getNode());
       return;
     }
-    ReplaceNode(Node,
-                selectImm(CurDAG, DL, ConstNode->getSExtValue(), *Subtarget));
+    int64_t Imm = ConstNode->getSExtValue();
+    // If the upper XLen-16 bits are not used, try to convert this to a simm12
+    // by sign extending bit 15.
+    if (isUInt<16>(Imm) && isInt<12>(SignExtend64(Imm, 16)) &&
+        hasAllHUsers(Node))
+      Imm = SignExtend64(Imm, 16);
+    // If the upper 32-bits are not used try to convert this into a simm32 by
+    // sign extending bit 32.
+    if (!isInt<32>(Imm) && isUInt<32>(Imm) && hasAllWUsers(Node))
+      Imm = SignExtend64(Imm, 32);
+
+    ReplaceNode(Node, selectImm(CurDAG, DL, Imm, *Subtarget));
     return;
   }
   case ISD::FrameIndex: {
     SDValue Imm = CurDAG->getTargetConstant(0, DL, XLenVT);
     int FI = cast<FrameIndexSDNode>(Node)->getIndex();
     SDValue TFI = CurDAG->getTargetFrameIndex(FI, VT);
     ReplaceNode(Node, CurDAG->getMachineNode(RISCV::ADDI, DL, VT, TFI, Imm));
     return;

 // the add/sub/mul/shl to become non-W instructions. By checking the users we
 // may be able to use a W instruction and CSE with the other instruction if
 // this has happened. We could try to detect that the CSE opportunity exists
 // before doing this, but that would be more complicated.
 // TODO: Does this need to look through AND/OR/XOR to their users to find more
 // opportunities.
 bool RISCVDAGToDAGISel::hasAllNBitUsers(SDNode *Node, unsigned Bits) const {
   assert((Node->getOpcode() == ISD::ADD || Node->getOpcode() == ISD::SUB ||
-          Node->getOpcode() == ISD::MUL || Node->getOpcode() == ISD::SHL) &&
+          Node->getOpcode() == ISD::MUL || Node->getOpcode() == ISD::SHL ||
+          isa<ConstantSDNode>(Node)) &&
          "Unexpected opcode");
 
   for (auto UI = Node->use_begin(), UE = Node->use_end(); UI != UE; ++UI) {
     SDNode *User = *UI;
     // Users of this node should have already been instruction selected
     if (!User->isMachineOpcode())
       return false;
 

llvm/test/CodeGen/RISCV/calling-conv-half.ll

 ; RV32IF-NEXT:    lw ra, 12(sp) # 4-byte Folded Reload
 ; RV32IF-NEXT:    addi sp, sp, 16
 ; RV32IF-NEXT:    ret
 ;
 ; RV64IF-LABEL: caller_half_on_stack:
 ; RV64IF:       # %bb.0:
 ; RV64IF-NEXT:    addi sp, sp, -16
 ; RV64IF-NEXT:    sd ra, 8(sp) # 8-byte Folded Spill
-; RV64IF-NEXT:    addi a0, zero, -183
-; RV64IF-NEXT:    slli a0, a0, 40
-; RV64IF-NEXT:    srli t0, a0, 32
+; RV64IF-NEXT:    lui a0, 1048565
+; RV64IF-NEXT:    addiw t0, a0, -1792
 ; RV64IF-NEXT:    addi a0, zero, 1
 ; RV64IF-NEXT:    addi a1, zero, 2
 ; RV64IF-NEXT:    addi a2, zero, 3
 ; RV64IF-NEXT:    addi a3, zero, 4
 ; RV64IF-NEXT:    addi a4, zero, 5
 ; RV64IF-NEXT:    addi a5, zero, 6
 ; RV64IF-NEXT:    addi a6, zero, 7
 ; RV64IF-NEXT:    addi a7, zero, 8

llvm/test/CodeGen/RISCV/imm.ll

 ; RV64I-NEXT:    slli a0, a0, 13
 ; RV64I-NEXT:    addi a0, a0, 837
 ; RV64I-NEXT:    slli a0, a0, 12
 ; RV64I-NEXT:    addi a0, a0, 1656
 ; RV64I-NEXT:    ret
   ret i64 -1152921504301427080 ; 0xF000_0000_1234_5678
 }
 
 ; FIXME: This should use a single ADDI for the immediate.
 define void @imm_store_i16_neg1(i16* %p) nounwind {
 ; RV32I-LABEL: imm_store_i16_neg1:
 ; RV32I:       # %bb.0:
-; RV32I-NEXT:    lui a1, 16
-; RV32I-NEXT:    addi a1, a1, -1
+; RV32I-NEXT:    addi a1, zero, -1
 ; RV32I-NEXT:    sh a1, 0(a0)
 ; RV32I-NEXT:    ret
 ;
 ; RV64I-LABEL: imm_store_i16_neg1:
 ; RV64I:       # %bb.0:
-; RV64I-NEXT:    lui a1, 16
-; RV64I-NEXT:    addiw a1, a1, -1
+; RV64I-NEXT:    addi a1, zero, -1
 ; RV64I-NEXT:    sh a1, 0(a0)
 ; RV64I-NEXT:    ret
   store i16 -1, i16* %p
   ret void
 }
 
 ; FIXME: This should use a single ADDI for the immediate.
 define void @imm_store_i32_neg1(i32* %p) nounwind {
 ; RV32I-LABEL: imm_store_i32_neg1:
 ; RV32I:       # %bb.0:
 ; RV32I-NEXT:    addi a1, zero, -1
 ; RV32I-NEXT:    sw a1, 0(a0)
 ; RV32I-NEXT:    ret
 ;
 ; RV64I-LABEL: imm_store_i32_neg1:
 ; RV64I:       # %bb.0:
 ; RV64I-NEXT:    addi a1, zero, -1
-; RV64I-NEXT:    srli a1, a1, 32
 ; RV64I-NEXT:    sw a1, 0(a0)
 ; RV64I-NEXT:    ret
   store i32 -1, i32* %p
   ret void
 }