[DAG] SimplifyDemandedBits - ISD::OR - attempt to simplify Op1 again once we have KnownBits from Op0

[RKSimon]

Summary

We already use the KnownBits of Op1 (which is more likely to be a constant) to improve the demanded bits of Op0. But we can repeat the process back on Op1 again if the demanded bits have further improved.

Something else I noticed while investigating D125836 and D77804....

[craig.topper]

Digging back through the history here, it looks like the two test cases here were explicitly broken by D116270 which prevented the OR from reassociating the constant to the outer OR.

I'm not necessarily opposed to this patch. I'm just not sure these test changes are the right demonstration of it. Does it help with other cases after the patches you mentioned?

llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp

1421

Needs to be indented 1 more space

llvm/test/CodeGen/AMDGPU/permute.ll

194

This doesn't match what comes out of instcombine, and with the instcombine version we already get v_perm

[RKSimon]

No it doesn't specifically help the UXTB regression that I'm still chasing - it was just one of the things I tried.

I'll leave this on hold for now.

[foad]

Digging back through the history here, it looks like the two test cases here were explicitly broken by D116270 which prevented the OR from reassociating the constant to the outer OR.

@alex-t FYI.

llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp

bool TargetLowering::SimplifyDemandedBits(
   case ISD::OR: {
     SDValue Op0 = Op.getOperand(0);
     SDValue Op1 = Op.getOperand(1);
 
     if (SimplifyDemandedBits(Op1, DemandedBits, DemandedElts, Known, TLO,
                              Depth + 1))
       return true;
     assert(!Known.hasConflict() && "Bits known to be one AND zero?");
-    if (SimplifyDemandedBits(Op0, ~Known.One & DemandedBits, DemandedElts,
-                             Known2, TLO, Depth + 1))
+    APInt DemandedBits0 = ~Known.One & DemandedBits;
+    if (SimplifyDemandedBits(Op0, DemandedBits0, DemandedElts, Known2, TLO,
+                             Depth + 1))
       return true;
     assert(!Known2.hasConflict() && "Bits known to be one AND zero?");
 
+   // Try Op1 again now we have the KnownBits of Op0.
+    APInt DemandedBits1 = ~(Known2.One & DemandedBits0) & DemandedBits;
+    if (DemandedBits1 != DemandedBits)
+      if (SimplifyDemandedBits(Op1, DemandedBits1, DemandedElts, Known, TLO,
+                               Depth + 1))
+        return true;
+    assert(!Known.hasConflict() && "Bits known to be one AND zero?");
+
     // Attempt to avoid multi-use ops if we don't need anything from them.
     if (!DemandedBits.isAllOnes() || !DemandedElts.isAllOnes()) {
       SDValue DemandedOp0 = SimplifyMultipleUseDemandedBits(
           Op0, DemandedBits, DemandedElts, TLO.DAG, Depth + 1);
       SDValue DemandedOp1 = SimplifyMultipleUseDemandedBits(
           Op1, DemandedBits, DemandedElts, TLO.DAG, Depth + 1);
       if (DemandedOp0 || DemandedOp1) {
         Op0 = DemandedOp0 ? DemandedOp0 : Op0;

llvm/test/CodeGen/AMDGPU/permute.ll

bb:
   %tmp = load i32, i32 addrspace(1)* %gep, align 4
   %tmp2 = and i32 %tmp, -16776961
   %tmp3 = and i32 %arg1, 16776960
   %tmp4 = xor i32 %tmp2, %tmp3
   store i32 %tmp4, i32 addrspace(1)* %gep, align 4
   ret void
 }
 
-; FIXME here should have been "v_perm_b32" with 0xffff0500 mask.
 define amdgpu_kernel void @and_or_or_and(i32 addrspace(1)* nocapture %arg, i32 %arg1) {
 ; GCN-LABEL: and_or_or_and:
 ; GCN:       ; %bb.0: ; %bb
 ; GCN-NEXT:    s_load_dwordx2 s[2:3], s[0:1], 0x24
 ; GCN-NEXT:    s_load_dword s0, s[0:1], 0x2c
 ; GCN-NEXT:    v_lshlrev_b32_e32 v0, 2, v0
+; GCN-NEXT:    v_mov_b32_e32 v3, 0xffff0500
 ; GCN-NEXT:    s_waitcnt lgkmcnt(0)
 ; GCN-NEXT:    v_mov_b32_e32 v1, s3
 ; GCN-NEXT:    v_add_u32_e32 v0, vcc, s2, v0
 ; GCN-NEXT:    v_addc_u32_e32 v1, vcc, 0, v1, vcc
 ; GCN-NEXT:    flat_load_dword v2, v[0:1]
-; GCN-NEXT:    s_and_b32 s0, s0, 0xff00
-; GCN-NEXT:    s_or_b32 s0, s0, 0xffff0000
 ; GCN-NEXT:    s_waitcnt vmcnt(0)
-; GCN-NEXT:    v_and_b32_e32 v2, 0xff00ff, v2
-; GCN-NEXT:    v_or_b32_e32 v2, s0, v2
+; GCN-NEXT:    v_perm_b32 v2, s0, v2, v3
 ; GCN-NEXT:    flat_store_dword v[0:1], v2
 ; GCN-NEXT:    s_endpgm
 bb:
   %id = tail call i32 @llvm.amdgcn.workitem.id.x()
   %gep = getelementptr i32, i32 addrspace(1)* %arg, i32 %id
   %tmp = load i32, i32 addrspace(1)* %gep, align 4
   %and = and i32 %tmp, 16711935     ; 0x00ff00ff
   %tmp1 = and i32 %arg1, 4294967040 ; 0xffffff00
bb:
   %tmp = load i32, i32 addrspace(1)* %gep, align 4
   %or1 = or i32 %tmp, 16776960    ; 0x00ffff00
   %or2 = or i32 %arg1, 4278190335 ; 0xff0000ff
   %and = and i32 %or1, %or2
   store i32 %and, i32 addrspace(1)* %gep, align 4
   ret void
 }
 
-; FIXME here should have been "v_perm_b32" with 0xffff0500 mask.
 define amdgpu_kernel void @known_ffff0500(i32 addrspace(1)* nocapture %arg, i32 %arg1) {
 ; GCN-LABEL: known_ffff0500:
 ; GCN:       ; %bb.0: ; %bb
 ; GCN-NEXT:    s_load_dwordx2 s[2:3], s[0:1], 0x24
 ; GCN-NEXT:    s_load_dword s0, s[0:1], 0x2c
 ; GCN-NEXT:    v_lshlrev_b32_e32 v0, 2, v0
-; GCN-NEXT:    v_mov_b32_e32 v5, 0xffff8004
+; GCN-NEXT:    v_mov_b32_e32 v5, 0xffff0500
+; GCN-NEXT:    v_mov_b32_e32 v6, 0xffff8004
 ; GCN-NEXT:    s_waitcnt lgkmcnt(0)
 ; GCN-NEXT:    v_mov_b32_e32 v1, s3
 ; GCN-NEXT:    v_add_u32_e32 v0, vcc, s2, v0
 ; GCN-NEXT:    v_addc_u32_e32 v1, vcc, 0, v1, vcc
 ; GCN-NEXT:    flat_load_dword v4, v[0:1]
 ; GCN-NEXT:    s_bitset1_b32 s0, 15
-; GCN-NEXT:    s_and_b32 s0, s0, 0xff00
-; GCN-NEXT:    s_or_b32 s0, s0, 0xffff0000
 ; GCN-NEXT:    v_mov_b32_e32 v2, s2
 ; GCN-NEXT:    v_mov_b32_e32 v3, s3
 ; GCN-NEXT:    s_waitcnt vmcnt(0)
 ; GCN-NEXT:    v_or_b32_e32 v4, 4, v4
-; GCN-NEXT:    v_and_b32_e32 v4, 0xff00ff, v4
-; GCN-NEXT:    v_or_b32_e32 v4, s0, v4
+; GCN-NEXT:    v_perm_b32 v4, s0, v4, v5
 ; GCN-NEXT:    flat_store_dword v[0:1], v4
-; GCN-NEXT:    flat_store_dword v[2:3], v5
+; GCN-NEXT:    flat_store_dword v[2:3], v6
 ; GCN-NEXT:    s_endpgm
 bb:
   %id = tail call i32 @llvm.amdgcn.workitem.id.x()
   %gep = getelementptr i32, i32 addrspace(1)* %arg, i32 %id
   %load = load i32, i32 addrspace(1)* %gep, align 4
   %mask1 = or i32 %arg1, 32768 ; 0x8000
   %mask2 = or i32 %load, 4
   %and = and i32 %mask2, 16711935     ; 0x00ff00ff