[GlobalISel] Extend booleans based on the target's BooleanContent

[mgrang]

Summary

Booleans should be extended based on the target's BooleanContent otherwise
always sign extending a 1 bit boolean results in making its value negative.

[mgrang]

Here's the failing test case:

cat a.mir
--- |
  target datalayout = "e-m:o-i64:64-i128:128-n32:64-S128"
  target triple = "aarch64--"
  define void @test_constant() {
  entry:
    ret void
  }
...
---
name:            test_constant
registers:
  - { id: 0, class: _ }
body: |
  bb.0.entry:
    %0:_(s1) = G_CONSTANT i1 1
    %1:_(s32) = COPY %0(s1)

llc -O0 -run-pass=legalizer a.mir -o -

bb.0.entry:
    %2:_(s32) = G_CONSTANT i32 -1
    %0:_(s1) = G_TRUNC %2(s32)
    %1:_(s32) = COPY %0(s1)

MachineIRBuilder.cpp was the only place I could find to intercept the boolean val and zext it. Please let me know if this isn't the proper place to fix this.

[arsenm]

lib/CodeGen/GlobalISel/MachineIRBuilder.cpp

268–271

This depends on the TargetBooleanContents

[mgrang]

Addressed comment.

[efriedma]

This doesn't seem right.

getBooleanContents specifically describes the results and operands for a certain set of DAG operations which require values that are either true or false, but are not represented using i1 after legalization. So we're talking about the result of ISD::SETCC, the condition operand of ISD::SELECT, etc. In SelectionDAG, this set of operations are legalized using PromoteTargetBoolean. (I'm assuming GlobalISel follows similar rules, although I can't find any documentation.)

This general rule does not apply to arbitrary i1 values. When an i1 is stored to memory, it is always zero-extended. And for other operations (logic operations, PHI nodes, etc), the high bits are undefined.

Given that, messing with MachineIRBuilder::buildConstant does nothing useful: you're just hiding a bug that could be exposed by producing a boolean some other way. For example, "trunc i8 %z to i1" gets legalized to a no-op with both SelectionDAG isel and GlobalISel.

[mgrang]

This is fixed in D47425. Abandoning this patch.

lib/CodeGen/GlobalISel/MachineIRBuilder.cpp

 
 MachineInstrBuilder MachineIRBuilder::buildConstant(unsigned Res,
                                                     const ConstantInt &Val) {
   LLT Ty = MRI->getType(Res);
 
   assert((Ty.isScalar() || Ty.isPointer()) && "invalid operand type");
 
   const ConstantInt *NewVal = &Val;
-  if (Ty.getSizeInBits() != Val.getBitWidth())
-    NewVal = ConstantInt::get(MF->getFunction().getContext(),
-                              Val.getValue().sextOrTrunc(Ty.getSizeInBits()));
-
+  if (Ty.getSizeInBits() != Val.getBitWidth()) {
+    APInt V;
+    // Booleans should be zero extended.
+    if (Val.getBitWidth() == 1)
+      V = Val.getValue().zext(Ty.getSizeInBits());
+    else
+      V = Val.getValue().sextOrTrunc(Ty.getSizeInBits());
+    NewVal = ConstantInt::get(MF->getFunction().getContext(), V);
+  }
   return buildInstr(TargetOpcode::G_CONSTANT).addDef(Res).addCImm(NewVal);
 }
 
 MachineInstrBuilder MachineIRBuilder::buildConstant(unsigned Res,
                                                     int64_t Val) {
   auto IntN = IntegerType::get(MF->getFunction().getContext(),
                                MRI->getType(Res).getSizeInBits());
   ConstantInt *CI = ConstantInt::get(IntN, Val, true);

test/CodeGen/AArch64/GlobalISel/legalize-constant.mir

bb.0.entry:
     ; CHECK: [[COPY2:%[0-9]+]]:_(s32) = COPY [[C2]](s32)
     ; CHECK: %w0 = COPY [[COPY2]](s32)
     ; CHECK: [[C3:%[0-9]+]]:_(s32) = G_CONSTANT i32 -1
     ; CHECK: %w0 = COPY [[C3]](s32)
     ; CHECK: [[C4:%[0-9]+]]:_(s64) = G_CONSTANT i64 1
     ; CHECK: %x0 = COPY [[C4]](s64)
     ; CHECK: [[C5:%[0-9]+]]:_(s64) = G_CONSTANT i64 0
     ; CHECK: %x0 = COPY [[C5]](s64)
+    ; CHECK: [[C:%[0-9]+]]:_(s32) = G_CONSTANT i32 1
     %0(s1) = G_CONSTANT i1 0
     %6:_(s32) = G_ANYEXT %0
     %w0 = COPY %6
     %1(s8) = G_CONSTANT i8 42
     %7:_(s32) = G_ANYEXT %1
     %w0 = COPY %7
     %2(s16) = G_CONSTANT i16 65535
     %8:_(s32) = G_ANYEXT %2
     %w0 = COPY %8
     %3(s32) = G_CONSTANT i32 -1
     %w0 = COPY %3
     %4(s64) = G_CONSTANT i64 1
     %x0 = COPY %4
     %5(s64) = G_CONSTANT i64 0
     %x0 = COPY %5
+    %9:_(s1) = G_CONSTANT i1 1
+    %10:_(s32) = COPY %9
 ...
 
 ---
 name:            test_fconstant
 registers:
   - { id: 0, class: _ }
   - { id: 1, class: _ }
   - { id: 2, class: _ }

test/CodeGen/ARM/GlobalISel/arm-legalize-fp.mir

bb.0:
     BX_RET 14, %noreg, implicit %r0
     ; HARD-DAG: [[X:%[0-9]+]]:_(s32) = COPY %r0
     ; HARD-DAG: [[Y:%[0-9]+]]:_(s32) = COPY %r1
     ; HARD: [[R:%[0-9]+]]:_(s1) = G_FCMP floatpred(true), [[X]](s32), [[Y]]
     ; HARD: [[REXT:%[0-9]+]]:_(s32) = G_ZEXT [[R]](s1)
     ; SOFT-NOT: G_FCMP
     ; For soft float we just need to return a '-1' constant, but the truncation
     ; to 1 bit is converted by the combiner to the following masking sequence.
-    ; SOFT: [[R:%[0-9]+]]:_(s32) = G_CONSTANT i32 -1
+    ; SOFT: [[R:%[0-9]+]]:_(s32) = G_CONSTANT i32 1
     ; SOFT: [[MASK:%[0-9]+]]:_(s32) = G_CONSTANT i32 1
     ; SOFT: [[RCOPY:%[0-9]+]]:_(s32) = COPY [[R]](s32)
     ; SOFT: [[REXT:%[0-9]+]]:_(s32) = G_AND [[RCOPY]], [[MASK]]
     ; SOFT-NOT: G_FCMP
     ; CHECK: %r0 = COPY [[REXT]]
 ...
 ---
 name:            test_fcmp_false_s32
bb.0:
     %4(s64) = G_MERGE_VALUES %0(s32), %1
     %5(s64) = G_MERGE_VALUES %2(s32), %3
     ; HARD-DAG: [[X:%[0-9]+]]:_(s64) = G_MERGE_VALUES [[X0]](s32), [[X1]](s32)
     ; HARD-DAG: [[Y:%[0-9]+]]:_(s64) = G_MERGE_VALUES [[Y0]](s32), [[Y1]](s32)
     %6(s1) = G_FCMP floatpred(true), %4(s64), %5
     ; HARD: [[R:%[0-9]+]]:_(s1) = G_FCMP floatpred(true), [[X]](s64), [[Y]]
     ; HARD: [[REXT:%[0-9]+]]:_(s32) = G_ZEXT [[R]](s1)
     ; SOFT-NOT: G_FCMP
-    ; SOFT: [[R:%[0-9]+]]:_(s32) = G_CONSTANT i32 -1
+    ; SOFT: [[R:%[0-9]+]]:_(s32) = G_CONSTANT i32 1
     ; The result needs to be truncated, and the combiner turns the truncation
     ; into the following masking sequence.
     ; SOFT: [[MASK:%[0-9]+]]:_(s32) = G_CONSTANT i32 1
     ; SOFT: [[RCOPY:%[0-9]+]]:_(s32) = COPY [[R]]
     ; SOFT: [[REXT:%[0-9]+]]:_(s32) = G_AND [[RCOPY]], [[MASK]]
     ; SOFT-NOT: G_FCMP
     %7(s32) = G_ZEXT %6(s1)
     %r0 = COPY %7(s32)

test/CodeGen/ARM/GlobalISel/arm-legalizer.mir

bb.0:
     ; CHECK-NOT: G_CONSTANT i8
     ; CHECK: [[EXT:%[0-9]+]]:_(s32) = G_CONSTANT i32 10
     ; CHECK: {{%[0-9]+}}:_(s8) = G_TRUNC [[EXT]](s32)
     ; CHECK-NOT: G_CONSTANT i8
 
     %3(s1) = G_CONSTANT i1 1
     G_STORE %3(s1), %4(p0) :: (store 1)
     ; CHECK-NOT: G_CONSTANT i1
-    ; CHECK: [[EXT:%[0-9]+]]:_(s32) = G_CONSTANT i32 -1
+    ; CHECK: [[EXT:%[0-9]+]]:_(s32) = G_CONSTANT i32 1
     ; CHECK: {{%[0-9]+}}:_(s1) = G_TRUNC [[EXT]](s32)
     ; CHECK-NOT: G_CONSTANT i1
 
     %5(p0) = G_CONSTANT 0
     G_STORE %5(p0), %4(p0) :: (store 4)
     ; CHECK: {{%[0-9]+}}:_(p0) = G_CONSTANT 0
 
     %r0 = COPY %0(s32)