[TargetLowering] try to create -1 constant operand for math ops via demanded bits

[spatel]

Summary

This reverses instcombine's demanded bits' transform which always tries to clear bits in constants.

As noted in PR35792 and shown in the test diffs:
https://bugs.llvm.org/show_bug.cgi?id=35792
...we can do better in codegen by trying to form -1. The x86 sub test shows a missed opportunity. We also don't get the shift case from the bug report, but I thought it'd be best to post this as-is to make sure it looks ok.

I did investigate changing instcombine's behavior, but it would be more work to change canonicalization in IR. Clearing bits / shrinking constants can allow killing instructions, so we'd have to figure out how to not regress those cases.

[craig.topper]

lib/CodeGen/SelectionDAG/TargetLowering.cpp

1246 ↗

(On Diff #133079)

Do we need to do anything with NSW/NUW flags?

[spatel]

lib/CodeGen/SelectionDAG/TargetLowering.cpp

1246 ↗

(On Diff #133079)

Good question. I saw the block above this and was wondering why it needed to explicitly disable those flags. It's because if we don't manually disable those flags, getNode() may return an existing node that has the flags set. That's scary...

The new math op in this case can't guarantee nsw/nuw, so yes, I think we need to explicitly disable those.

[spatel]

Patch updated:
Explicitly clear nsw/nuw.

[RKSimon]

LGTM

llvm/trunk/lib/CodeGen/SelectionDAG/TargetLowering.cpp

if (SimplifyDemandedBits(Op0, LoMask, Known2, TLO, Depth + 1) ||
         Flags.setNoSignedWrap(false);
         Flags.setNoUnsignedWrap(false);
         SDValue NewOp = TLO.DAG.getNode(Op.getOpcode(), dl, VT, Op0, Op1,
                                         Flags);
         return TLO.CombineTo(Op, NewOp);
       }
       return true;
     }
+
+    // If we have a constant operand, we may be able to turn it into -1 if we
+    // do not demand the high bits. This can make the constant smaller to
+    // encode, allow more general folding, or match specialized instruction
+    // patterns (eg, 'blsr' on x86). Don't bother changing 1 to -1 because that
+    // is probably not useful (and could be detrimental).
+    ConstantSDNode *C = isConstOrConstSplat(Op1);
+    APInt HighMask = APInt::getHighBitsSet(NewMask.getBitWidth(), NewMaskLZ);
+    if (C && !C->isAllOnesValue() && !C->isOne() &&
+        (C->getAPIntValue() | HighMask).isAllOnesValue()) {
+      SDValue Neg1 = TLO.DAG.getAllOnesConstant(dl, VT);
+      // We can't guarantee that the new math op doesn't wrap, so explicitly
+      // clear those flags to prevent folding with a potential existing node
+      // that has those flags set.
+      SDNodeFlags Flags;
+      Flags.setNoSignedWrap(false);
+      Flags.setNoUnsignedWrap(false);
+      SDValue NewOp = TLO.DAG.getNode(Op.getOpcode(), dl, VT, Op0, Neg1, Flags);
+      return TLO.CombineTo(Op, NewOp);
+    }
+
     LLVM_FALLTHROUGH;
   }
   default:
     // Just use computeKnownBits to compute output bits.
     TLO.DAG.computeKnownBits(Op, Known, Depth);
     break;
   }
 

llvm/trunk/test/CodeGen/AArch64/sub1.ll

 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
 ; RUN: llc < %s -mtriple=aarch64-unknown-unknown | FileCheck %s
 
 define i64 @sub1_disguised_constant(i64 %x) {
 ; CHECK-LABEL: sub1_disguised_constant:
 ; CHECK:       // %bb.0:
-; CHECK-NEXT:    orr w8, wzr, #0xffff
-; CHECK-NEXT:    add w8, w0, w8
+; CHECK-NEXT:    sub w8, w0, #1 // =1
 ; CHECK-NEXT:    and w8, w0, w8
 ; CHECK-NEXT:    and x0, x8, #0xffff
 ; CHECK-NEXT:    ret
   %a1 = and i64 %x, 65535
   %a2 = add i64 %x, 65535
   %r = and i64 %a1, %a2
   ret i64 %r
 }
 

llvm/trunk/test/CodeGen/X86/bmi.ll

; CHECK-NEXT:    retq
   ret i64 %tmp2
 }
 
 ; PR35792 - https://bugs.llvm.org/show_bug.cgi?id=35792
 
 define i64 @blsr_disguised_constant(i64 %x) {
 ; CHECK-LABEL: blsr_disguised_constant:
 ; CHECK:       # %bb.0:
-; CHECK-NEXT:    leal 65535(%rdi), %eax
-; CHECK-NEXT:    andl %edi, %eax
+; CHECK-NEXT:    blsrl %edi, %eax
 ; CHECK-NEXT:    movzwl %ax, %eax
 ; CHECK-NEXT:    retq
   %a1 = and i64 %x, 65535
   %a2 = add i64 %x, 65535
   %r = and i64 %a1, %a2
   ret i64 %r
 }
 

llvm/trunk/test/CodeGen/X86/zext-demanded.ll

; CHECK-NEXT:    retq
   %y = and i32 %x, 65535
   %z = or i32 %y, 1
   ret i32 %z
 }
 
 define i64 @add_neg_one(i64 %x) {
 ; CHECK-LABEL: add_neg_one:
 ; CHECK:       # %bb.0:
-; CHECK-NEXT:    leal 65535(%rdi), %eax
+; CHECK-NEXT:    leal -1(%rdi), %eax
 ; CHECK-NEXT:    andl %edi, %eax
 ; CHECK-NEXT:    movzwl %ax, %eax
 ; CHECK-NEXT:    retq
   %a1 = and i64 %x, 65535
   %a2 = add i64 %x, 65535
   %r = and i64 %a1, %a2
   ret i64 %r
 }
; CHECK-NEXT:    retq
   %r = and i64 %a1, %a2
   ret i64 %r
 }
 
 define i64 @mul_neg_one(i64 %x) {
 ; CHECK-LABEL: mul_neg_one:
 ; CHECK:       # %bb.0:
 ; CHECK-NEXT:    movl %edi, %eax
-; CHECK-NEXT:    shll $16, %eax
-; CHECK-NEXT:    subl %edi, %eax
+; CHECK-NEXT:    negl %eax
 ; CHECK-NEXT:    andl %edi, %eax
 ; CHECK-NEXT:    movzwl %ax, %eax
 ; CHECK-NEXT:    retq
   %a1 = and i64 %x, 65535
   %a2 = mul i64 %x, 65535
   %r = and i64 %a1, %a2
   ret i64 %r
 }