[X86] Teach PostprocessISelDAG to fold ANDrm+TESTrr when chain result is used.

[craig.topper]

Summary

The isOnlyUserOf prevented the fold if the chain result had any
users. What we really care about is the the data result from the
AND is only used by the TEST, and the flags results from the ANDs
aren't used at all. It's ok if the chain has users, we just need
to replace those users with the chain from the TESTrm.

[LuoYuanke]

llvm/lib/Target/X86/X86ISelDAGToDAG.cpp

1490

AND32rm has 3 output. This check 1st output, should we also check 2nd output (EFLAGS) and only leave the 3rd (chain) output be used freely?

[craig.topper]

llvm/lib/Target/X86/X86ISelDAGToDAG.cpp

1490

See line 1496 and 1507. We have to check the opcode before we can tell if output 1 exists.

[LuoYuanke]

llvm/lib/Target/X86/X86ISelDAGToDAG.cpp

1507

Is the EFLAGS of AND32rm the same to the EFLAGS produced by TEST32mr?

[craig.topper]

llvm/lib/Target/X86/X86ISelDAGToDAG.cpp

1507

That's not quite the right question to ask because the EFLAGS of AND32rm were never used.

The question to ask is does TEST32mr have the same EFLAGs as a TEST32rr of the AND32rm result register. Remember that the TEST32rr is doing an AND of the input with itself.

They both internally do an AND and discard the result, but keep the flags which are calculated form the result. For TEST32mr this result will computed by ANDing the two inputs from the original AND32rm. For the TEST32rr it will AND a value with itself so the resulting values is just whatever original AND32rm computed. So for both TEST instruction this result value is the same.

Let's look at each of the 6 flags
C - both set to 0
O - both set to 0
S - set to the sign bit of the result of the internal AND. Both produce the same result so the flag is the same
P - set based on the parity of the lower 8 bits of the result of the internal AND. Both produce the same result so the flag is the same
A - undefined and the compiler never uses it anyway
Z - set if the result of the internal AND is zero. Both produce the same result so the flag is the same

[LuoYuanke]

llvm/lib/Target/X86/X86ISelDAGToDAG.cpp

1507

Thank Craig for explaining. If And->hasAnyUseOfValue(1) is true. Can Value 1 be replaced by SDValue(Test, 0)?

[LuoYuanke]

LGTM except some question that not relevant to this patch.

llvm/lib/Target/X86/X86ISelDAGToDAG.cpp

while (Position != CurDAG->allnodes_begin()) {
     }
 
     // Look for a TESTrr+ANDrr pattern where both operands of the test are
     // the same. Rewrite to remove the AND.
     unsigned Opc = N->getMachineOpcode();
     if ((Opc == X86::TEST8rr || Opc == X86::TEST16rr ||
          Opc == X86::TEST32rr || Opc == X86::TEST64rr) &&
         N->getOperand(0) == N->getOperand(1) &&
-        N->isOnlyUserOf(N->getOperand(0).getNode()) &&
+        N->getOperand(0)->hasNUsesOfValue(2, N->getOperand(0).getResNo()) &&
         N->getOperand(0).isMachineOpcode()) {
       SDValue And = N->getOperand(0);
       unsigned N0Opc = And.getMachineOpcode();
-      if (N0Opc == X86::AND8rr || N0Opc == X86::AND16rr ||
-          N0Opc == X86::AND32rr || N0Opc == X86::AND64rr) {
+      if ((N0Opc == X86::AND8rr || N0Opc == X86::AND16rr ||
+           N0Opc == X86::AND32rr || N0Opc == X86::AND64rr) &&
+          !And->hasAnyUseOfValue(1)) {
         MachineSDNode *Test = CurDAG->getMachineNode(Opc, SDLoc(N),
                                                      MVT::i32,
                                                      And.getOperand(0),
                                                      And.getOperand(1));
         ReplaceUses(N, Test);
         MadeChange = true;
         continue;
       }
-      if (N0Opc == X86::AND8rm || N0Opc == X86::AND16rm ||
-          N0Opc == X86::AND32rm || N0Opc == X86::AND64rm) {
+      if ((N0Opc == X86::AND8rm || N0Opc == X86::AND16rm ||
+           N0Opc == X86::AND32rm || N0Opc == X86::AND64rm) &&
+          !And->hasAnyUseOfValue(1)) {
         unsigned NewOpc;
         switch (N0Opc) {
         case X86::AND8rm:  NewOpc = X86::TEST8mr; break;
         case X86::AND16rm: NewOpc = X86::TEST16mr; break;
         case X86::AND32rm: NewOpc = X86::TEST32mr; break;
         case X86::AND64rm: NewOpc = X86::TEST64mr; break;
         }
 
         // Need to swap the memory and register operand.
         SDValue Ops[] = { And.getOperand(1),
                           And.getOperand(2),
                           And.getOperand(3),
                           And.getOperand(4),
                           And.getOperand(5),
                           And.getOperand(0),
                           And.getOperand(6)  /* Chain */ };
         MachineSDNode *Test = CurDAG->getMachineNode(NewOpc, SDLoc(N),
                                                      MVT::i32, MVT::Other, Ops);
         CurDAG->setNodeMemRefs(
             Test, cast<MachineSDNode>(And.getNode())->memoperands());
-        ReplaceUses(N, Test);
+        ReplaceUses(And.getValue(2), SDValue(Test, 1));
+        ReplaceUses(SDValue(N, 0), SDValue(Test, 0));
         MadeChange = true;
         continue;
       }
     }
 
     // Look for a KAND+KORTEST and turn it into KTEST if only the zero flag is
     // used. We're doing this late so we can prefer to fold the AND into masked
     // comparisons. Doing that can be better for the live range of the mask

llvm/test/CodeGen/X86/cmp.ll

 return:                                           ; preds = %if.end, %if.then
   %retval.0 = phi i32 [ %call, %if.then ], [ %call2, %if.end ]
   ret i32 %retval.0
 }
 
 declare i32 @g()
 declare i32 @f()
 
-; FIXME: We should use a test from memory here instead of a load+and.i
-; The store makes sure the chain result of the load is used which prevents the
-; post isel peephole from catching this.
+; Make sure we fold the load+and into a test from memory.
+; The store makes sure the chain result of the load is used which used to
+; prevent the post isel peephole from catching this.
 define i1 @fold_test_and_with_chain(i32* %x, i32* %y, i32 %z) {
 ; CHECK-LABEL: fold_test_and_with_chain:
 ; CHECK:       # %bb.0:
-; CHECK-NEXT:    movl (%rdi), %eax # encoding: [0x8b,0x07]
-; CHECK-NEXT:    andl %edx, %eax # encoding: [0x21,0xd0]
+; CHECK-NEXT:    testl %edx, (%rdi) # encoding: [0x85,0x17]
 ; CHECK-NEXT:    sete %al # encoding: [0x0f,0x94,0xc0]
 ; CHECK-NEXT:    movl %edx, (%rsi) # encoding: [0x89,0x16]
 ; CHECK-NEXT:    retq # encoding: [0xc3]
   %a = load i32, i32* %x
   %b = and i32 %z, %a
   %c = icmp eq i32 %b, 0
   store i32 %z, i32* %y
   ret i1 %c
 }