[GlobalISel][NFC] Gardening: Factor out code for simple unary intrinsics

[paquette]

Summary

I noticed that there was a lot of duplicated code for unary intrinsics in translateKnownIntrinsic, and thought it would be nice to factor it out into its own function.

This adds a function and a lookup table which contains the currently supported simple unary intrinsics. I figured the lookup table would be nice, because if we add one of these, then all we have to do is add the intrinsic and its generic opcode to the mapping.

Also, it would get us out of writing code like this, which seems a little redundant IMO:

case intrinsicA:
    translateFooIntrinsic(OpcodeForA...);
case intrinsicB:
    translateFooIntrinsic(OpcodeForB...);
case intrinsicC:
    translateFooIntrinsic(OpcodeForC...);

[arsenm]

include/llvm/CodeGen/GlobalISel/IRTranslator.h

166–168

Why a map instead of a simple switch function?

[paquette]

include/llvm/CodeGen/GlobalISel/IRTranslator.h

166–168

Purely personal preference; I don't feel strongly either way so I'm happy to change it.

[paquette]

Use a function containing a switch instead of a map, and move it all into IRTranslator.cpp.

[arsenm]

LGTM with nit

lib/CodeGen/GlobalISel/IRTranslator.cpp

791

I don't think it's necessary to use Optional here, since you can use Intrinsic::not_intrinsic and that's what places usually do

[paquette]

Update diff to address nit before committing

include/llvm/CodeGen/GlobalISel/IRTranslator.h

private:
   // in once all MachineBasicBlocks have been created.
   SmallVector<std::pair<const PHINode *, SmallVector<MachineInstr *, 1>>, 4>
       PendingPHIs;
 
   /// Record of what frame index has been allocated to specified allocas for
   /// this function.
   DenseMap<const AllocaInst *, int> FrameIndices;
 
+  /// Static map of opcodes which can be translated using
+  /// translateSimpleUnaryIntrinsic.
+  const DenseMap<unsigned, unsigned> SimpleUnaryIntrinsicOpcodes = {
+      {Intrinsic::ceil, TargetOpcode::G_FCEIL},
+      {Intrinsic::cos, TargetOpcode::G_FCOS},
+      {Intrinsic::ctpop, TargetOpcode::G_CTPOP},
+      {Intrinsic::exp, TargetOpcode::G_FEXP},
+      {Intrinsic::exp2, TargetOpcode::G_FEXP2},
+      {Intrinsic::fabs, TargetOpcode::G_FABS},
+      {Intrinsic::log, TargetOpcode::G_FLOG},
+      {Intrinsic::log2, TargetOpcode::G_FLOG2},
+      {Intrinsic::log10, TargetOpcode::G_FLOG10},
+      {Intrinsic::round, TargetOpcode::G_INTRINSIC_ROUND},
+      {Intrinsic::sin, TargetOpcode::G_FSIN},
+      {Intrinsic::sqrt, TargetOpcode::G_FSQRT},
+      {Intrinsic::trunc, TargetOpcode::G_INTRINSIC_TRUNC}};
+
   /// \name Methods for translating form LLVM IR to MachineInstr.
   /// \see ::translate for general information on the translate methods.
   /// @{
 
   /// Translate \p Inst into its corresponding MachineInstr instruction(s).
   /// Insert the newly translated instruction(s) right where the CurBuilder
   /// is set.
   ///
private:
   bool translateMemfunc(const CallInst &CI, MachineIRBuilder &MIRBuilder,
                         unsigned ID);
 
   void getStackGuard(unsigned DstReg, MachineIRBuilder &MIRBuilder);
 
   bool translateOverflowIntrinsic(const CallInst &CI, unsigned Op,
                                   MachineIRBuilder &MIRBuilder);
 
+  /// Translates the intrinsics found in SimpleUnaryIntrinsicOpcodes.
+  /// \return true if the translation succeeded.
+  bool translateSimpleUnaryIntrinsic(const CallInst &CI, Intrinsic::ID ID,
+                                     MachineIRBuilder &MIRBuilder);
+
   bool translateKnownIntrinsic(const CallInst &CI, Intrinsic::ID ID,
                                MachineIRBuilder &MIRBuilder);
 
   bool translateInlineAsm(const CallInst &CI, MachineIRBuilder &MIRBuilder);
 
   // FIXME: temporary function to expose previous interface to call lowering
   // until it is refactored.
   /// Combines all component registers of \p V into a single scalar with size

lib/CodeGen/GlobalISel/IRTranslator.cpp

MIRBuilder.buildInstr(Op)
       .addDef(ResRegs[0])
       .addDef(ResRegs[1])
       .addUse(getOrCreateVReg(*CI.getOperand(0)))
       .addUse(getOrCreateVReg(*CI.getOperand(1)));
 
   return true;
 }
 
+bool IRTranslator::translateSimpleUnaryIntrinsic(
+    const CallInst &CI, Intrinsic::ID ID, MachineIRBuilder &MIRBuilder) {
+
+  auto Op = SimpleUnaryIntrinsicOpcodes.find(ID);
+
+  // Is this a simple unary intrinsic?
+  if (Op == SimpleUnaryIntrinsicOpcodes.end())
+    return false;
+
+  // Yes. Let's translate it.
+  auto Inst = MIRBuilder.buildInstr(Op->second)
+                  .addDef(getOrCreateVReg(CI))
+                  .addUse(getOrCreateVReg(*CI.getArgOperand(0)));
+  Inst->copyIRFlags(CI);
+  return true;
+}
+
 bool IRTranslator::translateKnownIntrinsic(const CallInst &CI, Intrinsic::ID ID,
                                            MachineIRBuilder &MIRBuilder) {
+
+  // If this is a simple unary intrinsic (that is, we just need to add a def of
+  // a vreg, and a use of a vreg, then translate it.
+  if (translateSimpleUnaryIntrinsic(CI, ID, MIRBuilder))
+    return true;
+
   switch (ID) {
   default:
     break;
   case Intrinsic::lifetime_start:
   case Intrinsic::lifetime_end: {
     // No stack colouring in O0, discard region information.
     if (MF->getTarget().getOptLevel() == CodeGenOpt::None)
       return true;
bool IRTranslator::translateKnownIntrinsic(const CallInst &CI, Intrinsic::ID ID,
   case Intrinsic::pow: {
     auto Pow = MIRBuilder.buildInstr(TargetOpcode::G_FPOW)
         .addDef(getOrCreateVReg(CI))
         .addUse(getOrCreateVReg(*CI.getArgOperand(0)))
         .addUse(getOrCreateVReg(*CI.getArgOperand(1)));
     Pow->copyIRFlags(CI);
     return true;
   }
-  case Intrinsic::exp: {
-    auto Exp = MIRBuilder.buildInstr(TargetOpcode::G_FEXP)
-        .addDef(getOrCreateVReg(CI))
-        .addUse(getOrCreateVReg(*CI.getArgOperand(0)));
-    Exp->copyIRFlags(CI);
-    return true;
-  }
-  case Intrinsic::exp2: {
-    auto Exp2 = MIRBuilder.buildInstr(TargetOpcode::G_FEXP2)
-        .addDef(getOrCreateVReg(CI))
-        .addUse(getOrCreateVReg(*CI.getArgOperand(0)));
-    Exp2->copyIRFlags(CI);
-    return true;
-  }
-  case Intrinsic::log: {
-    auto Log = MIRBuilder.buildInstr(TargetOpcode::G_FLOG)
-        .addDef(getOrCreateVReg(CI))
-        .addUse(getOrCreateVReg(*CI.getArgOperand(0)));
-    Log->copyIRFlags(CI);
-    return true;
-  }
-  case Intrinsic::log2: {
-    auto Log2 = MIRBuilder.buildInstr(TargetOpcode::G_FLOG2)
-        .addDef(getOrCreateVReg(CI))
-        .addUse(getOrCreateVReg(*CI.getArgOperand(0)));
-    Log2->copyIRFlags(CI);
-    return true;
-  }
-  case Intrinsic::log10: {
-    auto Log10 = MIRBuilder.buildInstr(TargetOpcode::G_FLOG10)
-        .addDef(getOrCreateVReg(CI))
-        .addUse(getOrCreateVReg(*CI.getArgOperand(0)));
-    Log10->copyIRFlags(CI);
-    return true;
-  }
-  case Intrinsic::fabs: {
-    auto Fabs = MIRBuilder.buildInstr(TargetOpcode::G_FABS)
-        .addDef(getOrCreateVReg(CI))
-        .addUse(getOrCreateVReg(*CI.getArgOperand(0)));
-    Fabs->copyIRFlags(CI);
-    return true;
-  }
-  case Intrinsic::trunc:
-    MIRBuilder.buildInstr(TargetOpcode::G_INTRINSIC_TRUNC)
-        .addDef(getOrCreateVReg(CI))
-        .addUse(getOrCreateVReg(*CI.getArgOperand(0)));
-    return true;
-  case Intrinsic::round:
-    MIRBuilder.buildInstr(TargetOpcode::G_INTRINSIC_ROUND)
-        .addDef(getOrCreateVReg(CI))
-        .addUse(getOrCreateVReg(*CI.getArgOperand(0)));
-    return true;
   case Intrinsic::fma: {
     auto FMA = MIRBuilder.buildInstr(TargetOpcode::G_FMA)
         .addDef(getOrCreateVReg(CI))
         .addUse(getOrCreateVReg(*CI.getArgOperand(0)))
         .addUse(getOrCreateVReg(*CI.getArgOperand(1)))
         .addUse(getOrCreateVReg(*CI.getArgOperand(2)));
     FMA->copyIRFlags(CI);
     return true;
unsigned Opcode = isTrailing
                                           : TargetOpcode::G_CTTZ_ZERO_UNDEF
                           : Cst->isZero() ? TargetOpcode::G_CTLZ
                                           : TargetOpcode::G_CTLZ_ZERO_UNDEF;
     MIRBuilder.buildInstr(Opcode)
         .addDef(getOrCreateVReg(CI))
         .addUse(getOrCreateVReg(*CI.getArgOperand(0)));
     return true;
   }
-  case Intrinsic::ctpop: {
-    MIRBuilder.buildInstr(TargetOpcode::G_CTPOP)
-        .addDef(getOrCreateVReg(CI))
-        .addUse(getOrCreateVReg(*CI.getArgOperand(0)));
-    return true;
-  }
   case Intrinsic::invariant_start: {
     LLT PtrTy = getLLTForType(*CI.getArgOperand(0)->getType(), *DL);
     unsigned Undef = MRI->createGenericVirtualRegister(PtrTy);
     MIRBuilder.buildUndef(Undef);
     return true;
   }
   case Intrinsic::invariant_end:
     return true;
-  case Intrinsic::ceil:
-    MIRBuilder.buildInstr(TargetOpcode::G_FCEIL)
-        .addDef(getOrCreateVReg(CI))
-        .addUse(getOrCreateVReg(*CI.getArgOperand(0)));
-    return true;
-  case Intrinsic::cos:
-    MIRBuilder.buildInstr(TargetOpcode::G_FCOS)
-        .addDef(getOrCreateVReg(CI))
-        .addUse(getOrCreateVReg(*CI.getArgOperand(0)));
-    return true;
-  case Intrinsic::sin:
-    MIRBuilder.buildInstr(TargetOpcode::G_FSIN)
-        .addDef(getOrCreateVReg(CI))
-        .addUse(getOrCreateVReg(*CI.getArgOperand(0)));
-    return true;
-  case Intrinsic::sqrt:
-    MIRBuilder.buildInstr(TargetOpcode::G_FSQRT)
-        .addDef(getOrCreateVReg(CI))
-        .addUse(getOrCreateVReg(*CI.getArgOperand(0)));
-    return true;
   }
   return false;
 }
 
 bool IRTranslator::translateInlineAsm(const CallInst &CI,
                                       MachineIRBuilder &MIRBuilder) {
   const InlineAsm &IA = cast<InlineAsm>(*CI.getCalledValue());
   if (!IA.getConstraintString().empty())