[GISel]: Add missing opcodes for overflow intrinsics

[aditya_nandakumar]

Summary

Currently, IRTranslator (and GISel) seems to be arbitrarily picking which overflow intrinsics get mapped into opcodes which either have a carry as an input or not.
For intrinsics such as Intrinsic::uadd_with_overflow, translate it to an opcode (G_UADDO) which doesn't have any carry inputs (similar to LLVM IR).

This patch adds 4 missing opcodes for completeness - G_UADDO, G_USUBO, G_SSUBE and G_SADDE.

As there is already some code that's selecting G_UADDE (X86) - I haven't changed X86's legalizerinfo. G_UADDEs can still be produced through legalization/combines etc.

[aemerson]

x86 is still using UADDE, and it's generated only by IRTranslator, so if you do this then x86 will lose support for compiling llvm.uadd.with.overflow intrinsics. @igorb what do you think?

Which is exposing a hole in our general GISel testing, where individual changes in passes can result in overall support for some input IR to be lost.

[igorb]

In D51197#1215499, @aemerson wrote:

x86 is still using UADDE, and it's generated only by IRTranslator, so if you do this then x86 will lose support for compiling llvm.uadd.with.overflow intrinsics. @igorb what do you think?

Which is exposing a hole in our general GISel testing, where individual changes in passes can result in overall support for some input IR to be lost.

Hi,
I think currently X86 use G_UADDE generated by Legalizer to support 64bit add on 32bit architecture.

Regards,
Igor

[aemerson]

In D51197#1215516, @igorb wrote:

In D51197#1215499, @aemerson wrote:

x86 is still using UADDE, and it's generated only by IRTranslator, so if you do this then x86 will lose support for compiling llvm.uadd.with.overflow intrinsics. @igorb what do you think?

Which is exposing a hole in our general GISel testing, where individual changes in passes can result in overall support for some input IR to be lost.

Hi,
I think currently X86 use G_UADDE generated by Legalizer to support 64bit add on 32bit architecture.

Regards,
Igor

Ah yes, I see it now.

[aditya_nandakumar]

Submitted in r340865. Thanks.

include/llvm/Support/TargetOpcodes.def

 HANDLE_TARGET_OPCODE(G_ICMP)
 
 /// Generic floating-point comparison, also applicable to vectors.
 HANDLE_TARGET_OPCODE(G_FCMP)
 
 /// Generic select.
 HANDLE_TARGET_OPCODE(G_SELECT)
 
+/// Generic unsigned add instruction, consuming the normal operands and
+/// producing the result and a carry flag.
+HANDLE_TARGET_OPCODE(G_UADDO)
+
 /// Generic unsigned add instruction, consuming the normal operands plus a carry
 /// flag, and similarly producing the result and a carry flag.
 HANDLE_TARGET_OPCODE(G_UADDE)
 
+/// Generic unsigned sub instruction, consuming the normal operands and
+/// producing the result and a carry flag.
+HANDLE_TARGET_OPCODE(G_USUBO)
+
 /// Generic unsigned subtract instruction, consuming the normal operands plus a
 /// carry flag, and similarly producing the result and a carry flag.
 HANDLE_TARGET_OPCODE(G_USUBE)
 
 /// Generic signed add instruction, producing the result and a signed overflow
 /// flag.
 HANDLE_TARGET_OPCODE(G_SADDO)
 
+/// Generic signed add instruction, consuming the normal operands plus a carry
+/// flag, and similarly producing the result and a carry flag.
+HANDLE_TARGET_OPCODE(G_SADDE)
+
 /// Generic signed subtract instruction, producing the result and a signed
 /// overflow flag.
 HANDLE_TARGET_OPCODE(G_SSUBO)
 
+/// Generic signed sub instruction, consuming the normal operands plus a carry
+/// flag, and similarly producing the result and a carry flag.
+HANDLE_TARGET_OPCODE(G_SSUBE)
+
 /// Generic unsigned multiply instruction, producing the result and a signed
 /// overflow flag.
 HANDLE_TARGET_OPCODE(G_UMULO)
 
 /// Generic signed multiply instruction, producing the result and a signed
 /// overflow flag.
 HANDLE_TARGET_OPCODE(G_SMULO)
 

include/llvm/Target/GenericOpcodes.td

def G_PTR_MASK : GenericInstruction {
   let InOperandList = (ins type0:$src, unknown:$bits);
   let hasSideEffects = 0;
 }
 
 //------------------------------------------------------------------------------
 // Overflow ops
 //------------------------------------------------------------------------------
 
+// Generic unsigned addition producing a carry flag.
+def G_UADDO : GenericInstruction {
+  let OutOperandList = (outs type0:$dst, type1:$carry_out);
+  let InOperandList = (ins type0:$src1, type0:$src2);
+  let hasSideEffects = 0;
+  let isCommutable = 1;
+}
+
 // Generic unsigned addition consuming and producing a carry flag.
 def G_UADDE : GenericInstruction {
   let OutOperandList = (outs type0:$dst, type1:$carry_out);
   let InOperandList = (ins type0:$src1, type0:$src2, type1:$carry_in);
   let hasSideEffects = 0;
 }
 
 // Generic signed addition producing a carry flag.
 def G_SADDO : GenericInstruction {
   let OutOperandList = (outs type0:$dst, type1:$carry_out);
   let InOperandList = (ins type0:$src1, type0:$src2);
   let hasSideEffects = 0;
   let isCommutable = 1;
 }
 
+// Generic signed addition consuming and producing a carry flag.
+def G_SADDE : GenericInstruction {
+  let OutOperandList = (outs type0:$dst, type1:$carry_out);
+  let InOperandList = (ins type0:$src1, type0:$src2, type1:$carry_in);
+  let hasSideEffects = 0;
+}
+
+// Generic unsigned subtraction producing a carry flag.
+def G_USUBO : GenericInstruction {
+  let OutOperandList = (outs type0:$dst, type1:$carry_out);
+  let InOperandList = (ins type0:$src1, type0:$src2);
+  let hasSideEffects = 0;
+}
 // Generic unsigned subtraction consuming and producing a carry flag.
 def G_USUBE : GenericInstruction {
   let OutOperandList = (outs type0:$dst, type1:$carry_out);
   let InOperandList = (ins type0:$src1, type0:$src2, type1:$carry_in);
   let hasSideEffects = 0;
 }
 
-// Generic unsigned subtraction producing a carry flag.
+// Generic signed subtraction producing a carry flag.
 def G_SSUBO : GenericInstruction {
   let OutOperandList = (outs type0:$dst, type1:$carry_out);
   let InOperandList = (ins type0:$src1, type0:$src2);
   let hasSideEffects = 0;
 }
 
+// Generic signed subtraction consuming and producing a carry flag.
+def G_SSUBE : GenericInstruction {
+  let OutOperandList = (outs type0:$dst, type1:$carry_out);
+  let InOperandList = (ins type0:$src1, type0:$src2, type1:$carry_in);
+  let hasSideEffects = 0;
+}
+
 // Generic unsigned multiplication producing a carry flag.
 def G_UMULO : GenericInstruction {
   let OutOperandList = (outs type0:$dst, type1:$carry_out);
   let InOperandList = (ins type0:$src1, type0:$src2);
   let hasSideEffects = 0;
   let isCommutable = 1;
 }
 

lib/CodeGen/GlobalISel/IRTranslator.cpp

MachineMemOperand *MemRef =
       MF->getMachineMemOperand(MPInfo, Flags, DL->getPointerSizeInBits() / 8,
                                DL->getPointerABIAlignment(0));
   MIB.setMemRefs({MemRef});
 }
 
 bool IRTranslator::translateOverflowIntrinsic(const CallInst &CI, unsigned Op,
                                               MachineIRBuilder &MIRBuilder) {
   ArrayRef<unsigned> ResRegs = getOrCreateVRegs(CI);
-  auto MIB = MIRBuilder.buildInstr(Op)
-                 .addDef(ResRegs[0])
-                 .addDef(ResRegs[1])
-                 .addUse(getOrCreateVReg(*CI.getOperand(0)))
-                 .addUse(getOrCreateVReg(*CI.getOperand(1)));
+  MIRBuilder.buildInstr(Op)
 
-  if (Op == TargetOpcode::G_UADDE || Op == TargetOpcode::G_USUBE) {
-    unsigned Zero = getOrCreateVReg(
-        *Constant::getNullValue(Type::getInt1Ty(CI.getContext())));
-    MIB.addUse(Zero);
-  }
-
   return true;
 }
 
 bool IRTranslator::translateKnownIntrinsic(const CallInst &CI, Intrinsic::ID ID,
                                            MachineIRBuilder &MIRBuilder) {
   switch (ID) {
   default:
     break;
if (!V) {
       // direct/indirect thing shouldn't really be handled by something as
       // implicit as reg+noreg vs reg+imm in the first palce, but it seems
       // pretty baked in right now.
       MIRBuilder.buildDirectDbgValue(Reg, DI.getVariable(), DI.getExpression());
     }
     return true;
   }
   case Intrinsic::uadd_with_overflow:
-    return translateOverflowIntrinsic(CI, TargetOpcode::G_UADDE, MIRBuilder);
+    return translateOverflowIntrinsic(CI, TargetOpcode::G_UADDO, MIRBuilder);
   case Intrinsic::sadd_with_overflow:
     return translateOverflowIntrinsic(CI, TargetOpcode::G_SADDO, MIRBuilder);
   case Intrinsic::usub_with_overflow:
-    return translateOverflowIntrinsic(CI, TargetOpcode::G_USUBE, MIRBuilder);
+    return translateOverflowIntrinsic(CI, TargetOpcode::G_USUBO, MIRBuilder);
   case Intrinsic::ssub_with_overflow:
     return translateOverflowIntrinsic(CI, TargetOpcode::G_SSUBO, MIRBuilder);
   case Intrinsic::umul_with_overflow:
     return translateOverflowIntrinsic(CI, TargetOpcode::G_UMULO, MIRBuilder);
   case Intrinsic::smul_with_overflow:
     return translateOverflowIntrinsic(CI, TargetOpcode::G_SMULO, MIRBuilder);
   case Intrinsic::pow:
     MIRBuilder.buildInstr(TargetOpcode::G_FPOW)

test/CodeGen/AArch64/GlobalISel/arm64-irtranslator.ll

define void @test_sadd_overflow(i32 %lhs, i32 %rhs, { i32, i1 }* %addr) {
   store { i32, i1 } %res, { i32, i1 }* %addr
   ret void
 }
 
 ; CHECK-LABEL: name: test_uadd_overflow
 ; CHECK: [[LHS:%[0-9]+]]:_(s32) = COPY $w0
 ; CHECK: [[RHS:%[0-9]+]]:_(s32) = COPY $w1
 ; CHECK: [[ADDR:%[0-9]+]]:_(p0) = COPY $x2
-; CHECK: [[ZERO:%[0-9]+]]:_(s1) = G_CONSTANT i1 false
-; CHECK: [[VAL:%[0-9]+]]:_(s32), [[OVERFLOW:%[0-9]+]]:_(s1) = G_UADDE [[LHS]], [[RHS]], [[ZERO]]
+; CHECK: [[VAL:%[0-9]+]]:_(s32), [[OVERFLOW:%[0-9]+]]:_(s1) = G_UADDO [[LHS]], [[RHS]]
 ; CHECK: G_STORE [[VAL]](s32), [[ADDR]](p0) :: (store 4 into %ir.addr)
 ; CHECK: [[CST:%[0-9]+]]:_(s64) = G_CONSTANT i64 4
 ; CHECK: [[GEP:%[0-9]+]]:_(p0) = G_GEP [[ADDR]], [[CST]](s64)
 ; CHECK: G_STORE [[OVERFLOW]](s1), [[GEP]](p0) :: (store 1 into %ir.addr + 4, align 4)
 declare { i32, i1 } @llvm.uadd.with.overflow.i32(i32, i32)
 define void @test_uadd_overflow(i32 %lhs, i32 %rhs, { i32, i1 }* %addr) {
   %res = call { i32, i1 } @llvm.uadd.with.overflow.i32(i32 %lhs, i32 %rhs)
   store { i32, i1 } %res, { i32, i1 }* %addr
define void @test_ssub_overflow(i32 %lhs, i32 %rhs, { i32, i1 }* %subr) {
   store { i32, i1 } %res, { i32, i1 }* %subr
   ret void
 }
 
 ; CHECK-LABEL: name: test_usub_overflow
 ; CHECK: [[LHS:%[0-9]+]]:_(s32) = COPY $w0
 ; CHECK: [[RHS:%[0-9]+]]:_(s32) = COPY $w1
 ; CHECK: [[ADDR:%[0-9]+]]:_(p0) = COPY $x2
-; CHECK: [[ZERO:%[0-9]+]]:_(s1) = G_CONSTANT i1 false
-; CHECK: [[VAL:%[0-9]+]]:_(s32), [[OVERFLOW:%[0-9]+]]:_(s1) = G_USUBE [[LHS]], [[RHS]], [[ZERO]]
+; CHECK: [[VAL:%[0-9]+]]:_(s32), [[OVERFLOW:%[0-9]+]]:_(s1) = G_USUBO [[LHS]], [[RHS]]
 ; CHECK: G_STORE [[VAL]](s32), [[ADDR]](p0) :: (store 4 into %ir.subr)
 ; CHECK: [[CST:%[0-9]+]]:_(s64) = G_CONSTANT i64 4
 ; CHECK: [[GEP:%[0-9]+]]:_(p0) = G_GEP [[ADDR]], [[CST]](s64)
 ; CHECK: G_STORE [[OVERFLOW]](s1), [[GEP]](p0) :: (store 1 into %ir.subr + 4, align 4)
 declare { i32, i1 } @llvm.usub.with.overflow.i32(i32, i32)
 define void @test_usub_overflow(i32 %lhs, i32 %rhs, { i32, i1 }* %subr) {
   %res = call { i32, i1 } @llvm.usub.with.overflow.i32(i32 %lhs, i32 %rhs)
   store { i32, i1 } %res, { i32, i1 }* %subr

test/CodeGen/AArch64/GlobalISel/legalizer-info-validation.mir

 # DEBUG:      .. the first uncovered type index: 2, OK
 #
 # DEBUG-NEXT: G_FCMP (opcode {{[0-9]+}}): 2 type indices
 # DEBUG:      .. the first uncovered type index: 2, OK
 #
 # DEBUG-NEXT: G_SELECT (opcode {{[0-9]+}}): 2 type indices
 # DEBUG:      .. the first uncovered type index: 2, OK
 #
+# DEBUG-NEXT: G_UADDO (opcode {{[0-9]+}}): 2 type indices
+# DEBUG:      .. type index coverage check SKIPPED: no rules defined
+#
 # DEBUG-NEXT: G_UADDE (opcode {{[0-9]+}}): 2 type indices
 # DEBUG:      .. the first uncovered type index: 2, OK
 #
+# DEBUG-NEXT: G_USUBO (opcode {{[0-9]+}}): 2 type indices
+# DEBUG:      .. type index coverage check SKIPPED: no rules defined
+#
 # DEBUG-NEXT: G_USUBE (opcode {{[0-9]+}}): 2 type indices
 # DEBUG:      .. the first uncovered type index: 2, OK
 #
 # DEBUG-NEXT: G_SADDO (opcode {{[0-9]+}}): 2 type indices
 # DEBUG:      .. the first uncovered type index: 2, OK
 #
+# DEBUG-NEXT: G_SADDE (opcode {{[0-9]+}}): 2 type indices
+# DEBUG:      .. type index coverage check SKIPPED: no rules defined
+#
 # DEBUG-NEXT: G_SSUBO (opcode {{[0-9]+}}): 2 type indices
 # DEBUG:      .. the first uncovered type index: 2, OK
 #
+# DEBUG-NEXT: G_SSUBE (opcode {{[0-9]+}}): 2 type indices
+# DEBUG:      .. type index coverage check SKIPPED: no rules defined
+#
 # DEBUG-NEXT: G_UMULO (opcode {{[0-9]+}}): 2 type indices
 # DEBUG:      .. the first uncovered type index: 2, OK
 #
 # DEBUG-NEXT: G_SMULO (opcode {{[0-9]+}}): 2 type indices
 # DEBUG:      .. the first uncovered type index: 2, OK
 #
 # DEBUG-NEXT: G_UMULH (opcode {{[0-9]+}}): 1 type index
 # DEBUG:      .. the first uncovered type index: 1, OK