[SelectionDAGBuilder] Respect getBooleanContents in copies to VRegs

[tlively]

Summary

Fixes https://bugs.llvm.org/show_bug.cgi?id=40172. See
test/CodeGen/WebAssembly/bug-40172.ll for an explanation. The changes
to X86 codegen are unfortunate if they are unnecessary, and I'd be
happy to accept feedback on how to make this fix less disruptive.

[craig.topper]

Is the zext/cmp optimization in WebAssemblyFastISel::zeroExtendToI32 really valid? Do other targets do that? If that had instead emitted an And with i1 like X86 does. The any_extend wouldn't have been an issue.

[tlively]

In D56428#1349304, @craig.topper wrote:

Is the zext/cmp optimization in WebAssemblyFastISel::zeroExtendToI32 really valid? Do other targets do that? If that had instead emitted an And with i1 like X86 does. The any_extend wouldn't have been an issue.

The FastISel optimization is correct as long as the CmpInst is also lowered by FastISel. I will have to think more about whether that is a valid assumption. But that FastISel logic does not actually affect the bug in question. See the continued discussion on the bug itself.

[tlively]

One question I have about this is whether BooleanContents should apply to *all* i1 values, which is what this patch assumes, or whether it should only apply to certain i1 values that are logically Boolean because they are the result of a setcc or similar. If BooleanContents should only apply to some i1s, this fix is overly conservative.

[tlively]

This is superseded by D56457.

lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

SelectionDAGBuilder::CopyValueToVirtualRegister(const Value *V, unsigned Reg) {
   const TargetLowering &TLI = DAG.getTargetLoweringInfo();
   // If this is an InlineAsm we have to match the registers required, not the
   // notional registers required by the type.
 
   RegsForValue RFV(V->getContext(), TLI, DAG.getDataLayout(), Reg, V->getType(),
                    None); // This is not an ABI copy.
   SDValue Chain = DAG.getEntryNode();
 
-  ISD::NodeType ExtendType = (FuncInfo.PreferredExtendType.find(V) ==
-                              FuncInfo.PreferredExtendType.end())
-                                 ? ISD::ANY_EXTEND
-                                 : FuncInfo.PreferredExtendType[V];
+  ISD::NodeType ExtendType = ISD::ANY_EXTEND;
+  if (V->getType()->isIntegerTy(1)) {
+    // Respect getBooleanContents while extending i1 values
+    switch (TLI.getBooleanContents(/*isVec=*/false, /*isFloat=*/false)) {
+    case TargetLowering::ZeroOrOneBooleanContent:
+      ExtendType = ISD::ZERO_EXTEND;
+      break;
+    case TargetLowering::ZeroOrNegativeOneBooleanContent:
+      ExtendType = ISD::SIGN_EXTEND;
+      break;
+    case TargetLowering::UndefinedBooleanContent:
+      break;
+    }
+  } else if (FuncInfo.PreferredExtendType.find(V) !=
+             FuncInfo.PreferredExtendType.end()) {
+    ExtendType = FuncInfo.PreferredExtendType[V];
+  }
+
   RFV.getCopyToRegs(Op, DAG, getCurSDLoc(), Chain, nullptr, V, ExtendType);
   PendingExports.push_back(Chain);
 }
 
 #include "llvm/CodeGen/SelectionDAGISel.h"
 
 /// isOnlyUsedInEntryBlock - If the specified argument is only used in the
 /// entry block, return true.  This includes arguments used by switches, since

test/CodeGen/WebAssembly/bug-40172.ll

+; RUN: llc < %s -O0 -debug 2>&1 | FileCheck %s
+
+; Regression test for bug 40172. The problem was that when FastISel
+; fell back to DAG ISel for the code below, the DAG was combined down
+; to a fragment that looked like
+;
+;   t2: i32,ch = CopyFromReg t0, Register:i32 %0
+;     t24: i1 = truncate t2
+;        t8: i32 = any_extend t24
+;          t10: ch = CopyToReg t0, Register:i32 %6, t8
+;
+; Where the icmp yielding %x5 has been combined away into the
+; truncate-extend pair. However, in the next step the truncate-extend
+; pair is combined out, yielding
+;
+;   t2: i32,ch = CopyFromReg t0, Register:i32 %0
+;     t10: ch = CopyToReg t0, Register:i32 %6, t2
+;
+; Which no longer produces a bool that respects
+; getBooleanContents. The extend should have been a zero_extend to
+; start out with so that getBooleanContents would be respected.
+
+target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
+target triple = "wasm32-unknown-unknown"
+
+; CHECK:      t0: ch = EntryToken
+; CHECK-NEXT:   t2: i32,ch = CopyFromReg t0, Register:i32 %0
+; CHECK-NEXT:     t3: i8 = truncate t2
+; CHECK-NEXT:       t5: i8 = and t3, Constant:i8<1>
+; CHECK-NEXT:         t7: i1 = setcc t5, Constant:i8<1>, seteq:ch
+; CHECK-NEXT:           t8: i32 = zero_extend t7
+; CHECK-NEXT:             t10: ch = CopyToReg t0, Register:i32 %5, t8
+
+define void @test(i8 %byte) {
+  %t = alloca { i8, i8 }, align 1
+  %x4 = and i8 %byte, 1
+  %x5 = icmp eq i8 %x4, 1
+  %x6 = and i8 %byte, 2
+  %x7 = icmp eq i8 %x6, 2
+  %x8 = bitcast { i8, i8 }* %t to i8*
+  %x9 = zext i1 %x5 to i8
+  store i8 %x9, i8* %x8, align 1
+  %x10 = getelementptr inbounds { i8, i8 }, { i8, i8 }* %t, i32 0, i32 1
+  %x11 = zext i1 %x7 to i8
+  store i8 %x11, i8* %x10, align 1
+  ret void
+}

test/CodeGen/X86/and-sink.ll

 define i32 @and_sink2(i32 %a, i1 %c, i1 %c2) {
 ; CHECK-LABEL: and_sink2:
 ; CHECK:       # %bb.0:
 ; CHECK-NEXT:    movl $0, A
 ; CHECK-NEXT:    testb $1, {{[0-9]+}}(%esp)
 ; CHECK-NEXT:    je .LBB1_5
 ; CHECK-NEXT:  # %bb.1: # %bb0.preheader
 ; CHECK-NEXT:    movb {{[0-9]+}}(%esp), %al
+; CHECK-NEXT:    andb $1, %al
 ; CHECK-NEXT:    movl {{[0-9]+}}(%esp), %ecx
 ; CHECK-NEXT:    .p2align 4, 0x90
 ; CHECK-NEXT:  .LBB1_2: # %bb0
 ; CHECK-NEXT:    # =>This Inner Loop Header: Depth=1
 ; CHECK-NEXT:    movl $0, B
-; CHECK-NEXT:    testb $1, %al
+; CHECK-NEXT:    testb %al, %al
 ; CHECK-NEXT:    je .LBB1_5
 ; CHECK-NEXT:  # %bb.3: # %bb1
 ; CHECK-NEXT:    # in Loop: Header=BB1_2 Depth=1
 ; CHECK-NEXT:    testb $4, %cl
 ; CHECK-NEXT:    movl $0, C
 ; CHECK-NEXT:    jne .LBB1_2
 ; CHECK-NEXT:  # %bb.4: # %bb2
 ; CHECK-NEXT:    movl $1, %eax

test/CodeGen/X86/fast-isel-select.ll

 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
 ; RUN: llc < %s -mtriple=x86_64-apple-darwin -O0 | FileCheck %s
 
 ; Make sure we only use the less significant bit of the value that feeds the
 ; select. Otherwise, we may account for a non-zero value whereas the
 ; lsb is zero.
 ; <rdar://problem/15651765>
 
 define i32 @fastisel_select(i1 %exchSub2211_, i1 %trunc_8766) {
 ; CHECK-LABEL: fastisel_select:
 ; CHECK:       ## %bb.0:
 ; CHECK-NEXT:    movb %sil, %al
+; CHECK-NEXT:    andb $1, %al
 ; CHECK-NEXT:    movb %dil, %cl
+; CHECK-NEXT:    andb $1, %cl
 ; CHECK-NEXT:    xorl %esi, %esi
 ; CHECK-NEXT:    subb %al, %cl
+; CHECK-NEXT:    andb $1, %cl
 ; CHECK-NEXT:    testb $1, %cl
 ; CHECK-NEXT:    movl $1204476887, %edi ## imm = 0x47CADBD7
 ; CHECK-NEXT:    cmovnel %edi, %esi
 ; CHECK-NEXT:    movl %esi, %eax
 ; CHECK-NEXT:    retq
   %shuffleInternal15257_8932 = sub i1 %exchSub2211_, %trunc_8766
   %counter_diff1345 = select i1 %shuffleInternal15257_8932, i32 1204476887, i32 0
   ret i32 %counter_diff1345
 }
 

test/CodeGen/X86/gpr-to-mask.ll

 define void @test_load_add(i1 %cond, float* %fptr, i1* %iptr1, i1* %iptr2, float %f1, float %f2)  {
 ; X86-64-LABEL: test_load_add:
 ; X86-64:       # %bb.0: # %entry
 ; X86-64-NEXT:    testb $1, %dil
 ; X86-64-NEXT:    je .LBB2_2
 ; X86-64-NEXT:  # %bb.1: # %if
 ; X86-64-NEXT:    movb (%rdx), %al
 ; X86-64-NEXT:    addb (%rcx), %al
+; X86-64-NEXT:    andb $1, %al
 ; X86-64-NEXT:    jmp .LBB2_3
 ; X86-64-NEXT:  .LBB2_2: # %else
 ; X86-64-NEXT:    movb (%rcx), %al
 ; X86-64-NEXT:  .LBB2_3: # %exit
 ; X86-64-NEXT:    kmovd %eax, %k1
 ; X86-64-NEXT:    vmovss %xmm0, %xmm0, %xmm1 {%k1}
 ; X86-64-NEXT:    vmovss %xmm1, (%rsi)
 ; X86-64-NEXT:    retq
 ;
 ; X86-32-LABEL: test_load_add:
 ; X86-32:       # %bb.0: # %entry
 ; X86-32-NEXT:    vmovss {{.*#+}} xmm0 = mem[0],zero,zero,zero
 ; X86-32-NEXT:    vmovss {{.*#+}} xmm1 = mem[0],zero,zero,zero
-; X86-32-NEXT:    movl {{[0-9]+}}(%esp), %ecx
+; X86-32-NEXT:    movl {{[0-9]+}}(%esp), %edx
 ; X86-32-NEXT:    movl {{[0-9]+}}(%esp), %eax
 ; X86-32-NEXT:    testb $1, {{[0-9]+}}(%esp)
 ; X86-32-NEXT:    je .LBB2_2
 ; X86-32-NEXT:  # %bb.1: # %if
-; X86-32-NEXT:    movl {{[0-9]+}}(%esp), %edx
-; X86-32-NEXT:    movb (%edx), %dl
-; X86-32-NEXT:    addb (%ecx), %dl
+; X86-32-NEXT:    movl {{[0-9]+}}(%esp), %ecx
+; X86-32-NEXT:    movb (%ecx), %cl
+; X86-32-NEXT:    addb (%edx), %cl
+; X86-32-NEXT:    andb $1, %cl
 ; X86-32-NEXT:    jmp .LBB2_3
 ; X86-32-NEXT:  .LBB2_2: # %else
-; X86-32-NEXT:    movb (%ecx), %dl
+; X86-32-NEXT:    movb (%edx), %cl
 ; X86-32-NEXT:  .LBB2_3: # %exit
-; X86-32-NEXT:    kmovd %edx, %k1
+; X86-32-NEXT:    kmovd %ecx, %k1
 ; X86-32-NEXT:    vmovss %xmm1, %xmm0, %xmm0 {%k1}
 ; X86-32-NEXT:    vmovss %xmm0, (%eax)
 ; X86-32-NEXT:    retl
 entry:
   br i1 %cond, label %if, label %else
 
 if:
   %loaded1 = load i1, i1* %iptr1

 
 define void @test_loadi1_storei1(i1 %cond, i1* %iptr1, i1* %iptr2, i1* %iptr3)  {
 ; X86-64-LABEL: test_loadi1_storei1:
 ; X86-64:       # %bb.0: # %entry
 ; X86-64-NEXT:    testb $1, %dil
 ; X86-64-NEXT:    je .LBB4_2
 ; X86-64-NEXT:  # %bb.1: # %if
 ; X86-64-NEXT:    movb (%rsi), %al
-; X86-64-NEXT:    jmp .LBB4_3
+; X86-64-NEXT:    movb %al, (%rcx)
+; X86-64-NEXT:    retq
 ; X86-64-NEXT:  .LBB4_2: # %else
 ; X86-64-NEXT:    movb (%rdx), %al
-; X86-64-NEXT:  .LBB4_3: # %exit
-; X86-64-NEXT:    andb $1, %al
 ; X86-64-NEXT:    movb %al, (%rcx)
 ; X86-64-NEXT:    retq
 ;
 ; X86-32-LABEL: test_loadi1_storei1:
 ; X86-32:       # %bb.0: # %entry
 ; X86-32-NEXT:    movl {{[0-9]+}}(%esp), %eax
 ; X86-32-NEXT:    testb $1, {{[0-9]+}}(%esp)
 ; X86-32-NEXT:    je .LBB4_2
 ; X86-32-NEXT:  # %bb.1: # %if
 ; X86-32-NEXT:    movl {{[0-9]+}}(%esp), %ecx
 ; X86-32-NEXT:    jmp .LBB4_3
 ; X86-32-NEXT:  .LBB4_2: # %else
 ; X86-32-NEXT:    movl {{[0-9]+}}(%esp), %ecx
 ; X86-32-NEXT:  .LBB4_3: # %exit
 ; X86-32-NEXT:    movb (%ecx), %cl
-; X86-32-NEXT:    andb $1, %cl
 ; X86-32-NEXT:    movb %cl, (%eax)
 ; X86-32-NEXT:    retl
 entry:
   br i1 %cond, label %if, label %else
 
 if:
   %loaded1 = load i1, i1* %iptr1
   br label %exit

test/CodeGen/X86/test-shrink-bug.ll

 ; CHECK-X64-NEXT:    testl $263, %edi # imm = 0x107
 ; CHECK-X64-NEXT:    je .LBB1_2
 ; CHECK-X64-NEXT:  # %bb.1:
 ; CHECK-X64-NEXT:    pand {{.*}}(%rip), %xmm0
 ; CHECK-X64-NEXT:    pcmpeqd {{.*}}(%rip), %xmm0
 ; CHECK-X64-NEXT:    pshufd {{.*#+}} xmm1 = xmm0[1,0,3,2]
 ; CHECK-X64-NEXT:    pand %xmm0, %xmm1
 ; CHECK-X64-NEXT:    pextrw $4, %xmm1, %eax
-; CHECK-X64-NEXT:    testb $1, %al
+; CHECK-X64-NEXT:    andb $1, %al
 ; CHECK-X64-NEXT:    jne .LBB1_2
 ; CHECK-X64-NEXT:  # %bb.3: # %no
 ; CHECK-X64-NEXT:    callq bar
 ; CHECK-X64-NEXT:    popq %rax
 ; CHECK-X64-NEXT:    .cfi_def_cfa_offset 8
 ; CHECK-X64-NEXT:    retq
 ; CHECK-X64-NEXT:  .LBB1_2: # %yes
 ; CHECK-X64-NEXT:    .cfi_def_cfa_offset 16

test/CodeGen/X86/xmulo.ll

 ; SDAG-NEXT:    mulq %rcx
 ; SDAG-NEXT:    seto %al
 ; SDAG-NEXT:    orb %sil, %al
 ; SDAG-NEXT:    retq
 ;
 ; FAST-LABEL: bug27873:
 ; FAST:       ## %bb.0:
 ; FAST-NEXT:    movq %rdi, %rax
+; FAST-NEXT:    andb $1, %sil
 ; FAST-NEXT:    movl $160, %ecx
 ; FAST-NEXT:    mulq %rcx
 ; FAST-NEXT:    seto %al
 ; FAST-NEXT:    orb %sil, %al
 ; FAST-NEXT:    retq
   %mul = call { i64, i1 } @llvm.umul.with.overflow.i64(i64 %c1, i64 160)
   %mul.overflow = extractvalue { i64, i1 } %mul, 1
   %x1 = or i1 %c2, %mul.overflow