[RISCV] Teach targetShrinkDemandedConstant to preserve (and X, neg)

[liaolucy]

Summary

Our CI caught regression from e1aa91b36325
that breaks the ANDI by turning -4 into 0x7ffffffc,
but I see that the commmit make sense, try to fix the regression.
Regarding ShrinkDemandedConstant, I am still learning
and willing to learn if there is a better solution.

[liaolucy]

This patch relies on the D153934.
Otherwise, I tried another method:

diff --git a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
index ef9e96b6cca4..9a46455017c6 100644
--- a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
+++ b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
@@ -13020,7 +13020,7 @@ bool RISCVTargetLowering::targetShrinkDemandedConstant(

   // For the remaining optimizations, we need to be able to make a negative
   // number through a combination of mask and undemanded bits.
-  if (!ExpandedMask.isNegative())
+  if (!ExpandedMask.isNegative() && Opcode != ISD::AND)
     return false;

   // What is the fewest number of bits we need to represent the negative number.
@@ -13034,8 +13034,13 @@ bool RISCVTargetLowering::targetShrinkDemandedConstant(
     NewMask.setBitsFrom(11);
   else if (!C->isOpaque() && MinSignedBits <= 32 && !ShrunkMask.isSignedIntN(32))
     NewMask.setBitsFrom(31);
-  else
-    return false;
+  else {
+    NewMask.setBitsFrom(31);
+    if (Opcode == ISD::AND && !C->isOpaque() && MinSignedBits <= 32 && !ShrunkMask.isSignedIntN(32) && NewMask.isSignedIntN(12))
+     return UseMask(NewMask);
+    else
+     return false;
+  }

   // Check that our new mask is a subset of the demanded mask.
   assert(IsLegalMask(NewMask));

[frasercrmck]

Just because I saw the word "regression" in my inbox and worried we were generating wrong code, it's probably best to explicitly say you're talking about fixing a performance regression. Or is it a code size regression? I couldn't tell from the link.

[craig.topper]

llvm/lib/Target/RISCV/RISCVISelLowering.cpp

9378

I’m not sure this is correct. We used ANY_EXTEND for the value to be shifted. That creates a new unknown sign bit. We can no longer be sure that wrap does not occur relative to that unknown sign.

[liaolucy]

I don't find a better way to implement it, I'll create new patch if I have a better idea, thanks for the review.

llvm/lib/Target/RISCV/RISCVISelLowering.cpp

if (N->getOperand(1).getOpcode() != ISD::Constant) {
       break;
     }
 
     // Custom legalize ISD::SHL by placing a SIGN_EXTEND_INREG after. This is
     // similar to customLegalizeToWOpWithSExt, but we must zero_extend the
     // shift amount.
     if (N->getOpcode() == ISD::SHL) {
       SDLoc DL(N);
+      SDNodeFlags Flags = N->getFlags();
       SDValue NewOp0 =
           DAG.getNode(ISD::ANY_EXTEND, DL, MVT::i64, N->getOperand(0));
       SDValue NewOp1 =
           DAG.getNode(ISD::ZERO_EXTEND, DL, MVT::i64, N->getOperand(1));
-      SDValue NewWOp = DAG.getNode(ISD::SHL, DL, MVT::i64, NewOp0, NewOp1);
+      SDValue NewWOp =
+          DAG.getNode(ISD::SHL, DL, MVT::i64, NewOp0, NewOp1, Flags);
       SDValue NewRes = DAG.getNode(ISD::SIGN_EXTEND_INREG, DL, MVT::i64, NewWOp,
                                    DAG.getValueType(MVT::i32));
       Results.push_back(DAG.getNode(ISD::TRUNCATE, DL, MVT::i32, NewRes));
     }
 
     break;
   case ISD::ROTL:
   case ISD::ROTR:
bool RISCVTargetLowering::targetShrinkDemandedConstant(
 
   // For the remaining optimizations, we need to be able to make a negative
   // number through a combination of mask and undemanded bits.
   if (!ExpandedMask.isNegative())
     return false;
 
   // What is the fewest number of bits we need to represent the negative number.
   unsigned MinSignedBits = ExpandedMask.getSignificantBits();
+  APInt NewMask = ShrunkMask;
+  NewMask.setBitsFrom(31);
+  if (Opcode == ISD::AND && MinSignedBits == 64 && NewMask.isSignedIntN(12) &&
+      ShrunkMask.isSignedIntN(32)) {
+    return UseMask(NewMask);
+  }
 
   // Try to make a 12 bit negative immediate. If that fails try to make a 32
   // bit negative immediate unless the shrunk immediate already fits in 32 bits.
   // If we can't create a simm12, we shouldn't change opaque constants.
-  APInt NewMask = ShrunkMask;
+  NewMask = ShrunkMask;
   if (MinSignedBits <= 12)
     NewMask.setBitsFrom(11);
   else if (!C->isOpaque() && MinSignedBits <= 32 && !ShrunkMask.isSignedIntN(32))
     NewMask.setBitsFrom(31);
   else
     return false;
 
   // Check that our new mask is a subset of the demanded mask.

llvm/test/CodeGen/RISCV/branch-on-zero.ll

 ; RV64:       # %bb.0: # %entry
 ; RV64-NEXT:    sext.w a1, a0
 ; RV64-NEXT:    beqz a1, .LBB2_3
 ; RV64-NEXT:  # %bb.1: # %for.body.preheader
 ; RV64-NEXT:    li a1, 0
 ; RV64-NEXT:  .LBB2_2: # %for.body
 ; RV64-NEXT:    # =>This Inner Loop Header: Depth=1
 ; RV64-NEXT:    andi a2, a0, 1
-; RV64-NEXT:    srliw a0, a0, 1
+; RV64-NEXT:    srli a0, a0, 1
 ; RV64-NEXT:    addw a1, a1, a2
 ; RV64-NEXT:    bnez a0, .LBB2_2
 ; RV64-NEXT:  .LBB2_3: # %for.end
 ; RV64-NEXT:    mv a0, a1
 ; RV64-NEXT:    ret
 entry:
   %tobool.not4 = icmp eq i32 %v, 0
   br i1 %tobool.not4, label %for.end, label %for.body

llvm/test/CodeGen/RISCV/rv64i-demanded-bits.ll

 ; CHECK-NEXT:    slli a0, a0, 48
 ; CHECK-NEXT:    srli a0, a0, 48
 ; CHECK-NEXT:    ret
 entry:
   %and = and i32 %x, 65280
   %or = or i32 %and, 255
   ret i32 %or
 }
+
+; https://alive2.llvm.org/ce/z/RizyWW
+define i32 @shl_add_neg4(i32 noundef %x) {
+; CHECK-LABEL: shl_add_neg4:
+; CHECK:       # %bb.0:
+; CHECK-NEXT:    slli a0, a0, 1
+; CHECK-NEXT:    andi a0, a0, -4
+; CHECK-NEXT:    ret
+  %y = shl nuw nsw i32 %x, 1
+  %z = and i32 %y, 2147483644
+  ret i32 %z
+}