Revert r372285 "GlobalISel: Don't materialize immarg arguments to intrinsics"

This broke the Chromium build, causing it to fail with e.g.

  fatal error: error in backend: Cannot select: t362: v4i32 = X86ISD::VSHLI t392, Constant:i8<15>

See llvm-commits thread of r372285 for details.

This also reverts r372286, r372287, r372288, r372289, r372290, r372291,
r372292, r372293, r372296, and r372297, which seemed to depend on the
main commit.

> Encode them directly as an imm argument to G_INTRINSIC*.
>
> Since now intrinsics can now define what parameters are required to be
> immediates, avoid using registers for them. Intrinsics could
> potentially want a constant that isn't a legal register type. Also,
> since G_CONSTANT is subject to CSE and legalization, transforms could
> potentially obscure the value (and create extra work for the
> selector). The register bank of a G_CONSTANT is also meaningful, so
> this could throw off future folding and legalization logic for AMDGPU.
>
> This will be much more convenient to work with than needing to call
> getConstantVRegVal and checking if it may have failed for every
> constant intrinsic parameter. AMDGPU has quite a lot of intrinsics wth
> immarg operands, many of which need inspection during lowering. Having
> to find the value in a register is going to add a lot of boilerplate
> and waste compile time.
>
> SelectionDAG has always provided TargetConstant for constants which
> should not be legalized or materialized in a register. The distinction
> between Constant and TargetConstant was somewhat fuzzy, and there was
> no automatic way to force usage of TargetConstant for certain
> intrinsic parameters. They were both ultimately ConstantSDNode, and it
> was inconsistently used. It was quite easy to mis-select an
> instruction requiring an immediate. For SelectionDAG, start emitting
> TargetConstant for these arguments, and using timm to match them.
>
> Most of the work here is to cleanup target handling of constants. Some
> targets process intrinsics through intermediate custom nodes, which
> need to preserve TargetConstant usage to match the intrinsic
> expectation. Pattern inputs now need to distinguish whether a constant
> is merely compatible with an operand or whether it is mandatory.
>
> The GlobalISelEmitter needs to treat timm as a special case of a leaf
> node, simlar to MachineBasicBlock operands. This should also enable
> handling of patterns for some G_* instructions with immediates, like
> G_FENCE or G_EXTRACT.
>
> This does include a workaround for a crash in GlobalISelEmitter when
> ARM tries to uses "imm" in an output with a "timm" pattern source.

llvm-svn: 372314

Author: zmodem

llvm/include/llvm/CodeGen/GlobalISel/InstructionSelector.h

@@ -220,11 +220,6 @@ enum {
   /// - OpIdx - Operand index
   GIM_CheckIsMBB,
 
-  /// Check the specified operand is an Imm
-  /// - InsnID - Instruction ID
-  /// - OpIdx - Operand index
-  GIM_CheckIsImm,
-
   /// Check if the specified operand is safe to fold into the current
   /// instruction.
   /// - InsnID - Instruction ID

llvm/include/llvm/CodeGen/GlobalISel/InstructionSelectorImpl.h

@@ -690,19 +690,7 @@ bool InstructionSelector::executeMatchTable(
       }
       break;
     }
-    case GIM_CheckIsImm: {
-      int64_t InsnID = MatchTable[CurrentIdx++];
-      int64_t OpIdx = MatchTable[CurrentIdx++];
-      DEBUG_WITH_TYPE(TgtInstructionSelector::getName(),
-                      dbgs() << CurrentIdx << ": GIM_CheckIsImm(MIs[" << InsnID
-                             << "]->getOperand(" << OpIdx << "))\n");
-      assert(State.MIs[InsnID] != nullptr && "Used insn before defined");
-      if (!State.MIs[InsnID]->getOperand(OpIdx).isImm()) {
-        if (handleReject() == RejectAndGiveUp)
-          return false;
-      }
-      break;
-    }
+
     case GIM_CheckIsSafeToFold: {
       int64_t InsnID = MatchTable[CurrentIdx++];
       DEBUG_WITH_TYPE(TgtInstructionSelector::getName(),

llvm/include/llvm/CodeGen/ScheduleDAGInstrs.h

@@ -374,9 +374,6 @@ namespace llvm {
     /// Returns a mask for which lanes get read/written by the given (register)
     /// machine operand.
     LaneBitmask getLaneMaskForMO(const MachineOperand &MO) const;
-
-    /// Returns true if the def register in \p MO has no uses.
-    bool deadDefHasNoUse(const MachineOperand &MO);
   };
 
   /// Creates a new SUnit and return a ptr to it.

llvm/include/llvm/Target/TargetSelectionDAG.td

@@ -848,11 +848,6 @@ class ImmLeaf<ValueType vt, code pred, SDNodeXForm xform = NOOP_SDNodeXForm,
   bit IsAPFloat = 0;
 }
 
-// Convenience wrapper for ImmLeaf to use timm/TargetConstant instead
-// of imm/Constant.
-class TImmLeaf<ValueType vt, code pred, SDNodeXForm xform = NOOP_SDNodeXForm,
-  SDNode ImmNode = timm> : ImmLeaf<vt, pred, xform, ImmNode>;
-
 // An ImmLeaf except that Imm is an APInt. This is useful when you need to
 // zero-extend the immediate instead of sign-extend it.
 //

llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp

@@ -1617,29 +1617,14 @@ bool IRTranslator::translateCall(const User &U, MachineIRBuilder &MIRBuilder) {
   if (isa<FPMathOperator>(CI))
     MIB->copyIRFlags(CI);
 
-  for (auto &Arg : enumerate(CI.arg_operands())) {
+  for (auto &Arg : CI.arg_operands()) {
     // Some intrinsics take metadata parameters. Reject them.
-    if (isa<MetadataAsValue>(Arg.value()))
+    if (isa<MetadataAsValue>(Arg))
       return false;
-
-    // If this is required to be an immediate, don't materialize it in a
-    // register.
-    if (CI.paramHasAttr(Arg.index(), Attribute::ImmArg)) {
-      if (ConstantInt *CI = dyn_cast<ConstantInt>(Arg.value())) {
-        // imm arguments are more convenient than cimm (and realistically
-        // probably sufficient), so use them.
-        assert(CI->getBitWidth() <= 64 &&
-               "large intrinsic immediates not handled");
-        MIB.addImm(CI->getSExtValue());
-      } else {
-        MIB.addFPImm(cast<ConstantFP>(Arg.value()));
-      }
-    } else {
-      ArrayRef<Register> VRegs = getOrCreateVRegs(*Arg.value());
-      if (VRegs.size() > 1)
-        return false;
-      MIB.addUse(VRegs[0]);
-    }
+    ArrayRef<Register> VRegs = getOrCreateVRegs(*Arg);
+    if (VRegs.size() > 1)
+      return false;
+    MIB.addUse(VRegs[0]);
   }
 
   // Add a MachineMemOperand if it is a target mem intrinsic.

llvm/lib/CodeGen/ScheduleDAGInstrs.cpp

@@ -373,13 +373,6 @@ LaneBitmask ScheduleDAGInstrs::getLaneMaskForMO(const MachineOperand &MO) const
   return TRI->getSubRegIndexLaneMask(SubReg);
 }
 
-bool ScheduleDAGInstrs::deadDefHasNoUse(const MachineOperand &MO) {
-  auto RegUse = CurrentVRegUses.find(MO.getReg());
-  if (RegUse == CurrentVRegUses.end())
-    return true;
-  return (RegUse->LaneMask & getLaneMaskForMO(MO)).none();
-}
-
 /// Adds register output and data dependencies from this SUnit to instructions
 /// that occur later in the same scheduling region if they read from or write to
 /// the virtual register defined at OperIdx.
@@ -409,7 +402,8 @@ void ScheduleDAGInstrs::addVRegDefDeps(SUnit *SU, unsigned OperIdx) {
   }
 
   if (MO.isDead()) {
-    assert(deadDefHasNoUse(MO) && "Dead defs should have no uses");
+    assert(CurrentVRegUses.find(Reg) == CurrentVRegUses.end() &&
+           "Dead defs should have no uses");
   } else {
     // Add data dependence to all uses we found so far.
     const TargetSubtargetInfo &ST = MF.getSubtarget();

llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

@@ -4768,22 +4768,8 @@ void SelectionDAGBuilder::visitTargetIntrinsic(const CallInst &I,
 
   // Add all operands of the call to the operand list.
   for (unsigned i = 0, e = I.getNumArgOperands(); i != e; ++i) {
-    const Value *Arg = I.getArgOperand(i);
-    if (!I.paramHasAttr(i, Attribute::ImmArg)) {
-      Ops.push_back(getValue(Arg));
-      continue;
-    }
-
-    // Use TargetConstant instead of a regular constant for immarg.
-    EVT VT = TLI.getValueType(*DL, Arg->getType(), true);
-    if (const ConstantInt *CI = dyn_cast<ConstantInt>(Arg)) {
-      assert(CI->getBitWidth() <= 64 &&
-             "large intrinsic immediates not handled");
-      Ops.push_back(DAG.getTargetConstant(*CI, SDLoc(), VT));
-    } else {
-      Ops.push_back(
-          DAG.getTargetConstantFP(*cast<ConstantFP>(Arg), SDLoc(), VT));
-    }
+    SDValue Op = getValue(I.getArgOperand(i));
+    Ops.push_back(Op);
   }
 
   SmallVector<EVT, 4> ValueVTs;

llvm/lib/Target/AArch64/AArch64InstrFormats.td

@@ -685,11 +685,11 @@ def logical_imm64_not : Operand<i64> {
 
 // iXX_imm0_65535 predicates - True if the immediate is in the range [0,65535].
 let ParserMatchClass = AsmImmRange<0, 65535>, PrintMethod = "printImmHex" in {
-def i32_imm0_65535 : Operand<i32>, TImmLeaf<i32, [{
+def i32_imm0_65535 : Operand<i32>, ImmLeaf<i32, [{
   return ((uint32_t)Imm) < 65536;
 }]>;
 
-def i64_imm0_65535 : Operand<i64>, TImmLeaf<i64, [{
+def i64_imm0_65535 : Operand<i64>, ImmLeaf<i64, [{
   return ((uint64_t)Imm) < 65536;
 }]>;
 }

llvm/lib/Target/AArch64/AArch64InstrInfo.td

@@ -798,7 +798,7 @@ def MOVbaseTLS : Pseudo<(outs GPR64:$dst), (ins),
 let Uses = [ X9 ], Defs = [ X16, X17, LR, NZCV ] in {
 def HWASAN_CHECK_MEMACCESS : Pseudo<
   (outs), (ins GPR64noip:$ptr, i32imm:$accessinfo),
-  [(int_hwasan_check_memaccess X9, GPR64noip:$ptr, (i32 timm:$accessinfo))]>,
+  [(int_hwasan_check_memaccess X9, GPR64noip:$ptr, (i32 imm:$accessinfo))]>,
   Sched<[]>;
 }