[TableGen] Allow identical MnemonicAliases with no predicate

[foad]

Summary

My use case for this is illustrated in the test case: I want to define
the same instruction twice with different (disjoint) predicates, because
the instruction has different operands on different subtargets. It's
convenient to do this with a multiclass that also defines an alias for
the instruction.

Previously tablegen would complain if this alias was defined twice with
no predicate. One way to fix this would be to add a predicate on each
definition of the alias, matching the predicate on the instruction. But
this (a) is slightly awkward to do in the real world use case I had, and
(b) leads to an inefficient matcher that will do something like this:

if (Mnemonic == "foo_alias") {
  if (Features.test(Feature_Subtarget1Bit))
    Mnemonic == "foo";
  else if (Features.test(Feature_Subtarget2Bit))
    Mnemonic == "foo";
  return;
}

It would be more efficient to skip the feature tests and return "foo"
unconditionally.

Overall it seems better to allow multiple definitions of the identical
alias with no predicate.

[nlguillemot]

In the code sample in the description, is it supposed to be Feature_Subtarget2Bit for the second "if"?

One thing I'm wondering is, how about defining the MnemonicAlias just once outside the multiclass? Like this:

defm FooInst1 : DefInstruction<"foo", (outs), (ins), Subtarget1>;
defm FooInst2 : DefInstruction<"foo", (outs), (ins), Subtarget2>;
def : MnemonicAlias<"foo_alias", "foo">;

Otherwise if the MnemonicAlias is in the multiclass then there are two virtually identical MnemonicAliases being defined right?

I think that generally speaking it makes sense to avoid letting users use duplicate inputs since that might help catch errors in some cases. On the other hand I can see the practical utility in allowing duplicates when it comes to working with multiclasses. I'm not sure, thoughts?

[foad]

In D105033#2844486, @nlguillemot wrote:

In the code sample in the description, is it supposed to be Feature_Subtarget2Bit for the second "if"?

Yes, sorry about that. Fixed.

One thing I'm wondering is, how about defining the MnemonicAlias just once outside the multiclass?

Well, yes. The problem with simplifying the example enough to fit in a commit message, is that it makes it look simple to fix by rewriting the td file. The real world example is more complicated. And yes I'm sure I could fix it by changing the td file, but I was hoping you all would take my word for it that it's pretty awkward to do that.

To show you the complete real world example I would have to spend some time making it open-source-able. But the multiclass I'm really interested in is analogous to:

multiclass VOP1Inst <string opName, VOPProfile P,
                     SDPatternOperator node = null_frag> {
  // We only want to set this on the basic, non-SDWA or DPP forms.
  defvar should_mov_imm = !eq(opName, "v_mov_b32");

  let isMoveImm = should_mov_imm in {
    def _e32 : VOP1_Pseudo <opName, P>;
    def _e64 : VOP3_Pseudo <opName, P, getVOP1Pat64<node, P>.ret>;
  }

  foreach _ = BoolToList<P.HasExtSDWA>.ret in
    def _sdwa : VOP1_SDWA_Pseudo <opName, P>;

  foreach _ = BoolToList<P.HasExtDPP>.ret in
    def _dpp : VOP1_DPP_Pseudo <opName, P>;

  def : MnemonicAlias<opName#"_e32", opName>, LetDummies;
  def : MnemonicAlias<opName#"_e64", opName>, LetDummies;

  foreach _ = BoolToList<P.HasExtSDWA>.ret in
    def : MnemonicAlias<opName#"_sdwa", opName>, LetDummies;

  foreach _ = BoolToList<P.HasExtDPP>.ret in
    def : MnemonicAlias<opName#"_dpp", opName>, LetDummies;
}

And it's used something like this:

let SubtargetPredicate = isGFX7GFX8GFX9 in {
  let TRANS = 1, SchedRW = [WriteTrans32] in {
    defm V_LOG_LEGACY_F32 : VOP1Inst<"v_log_legacy_f32", VOP_F32_F32>;
    defm V_EXP_LEGACY_F32 : VOP1Inst<"v_exp_legacy_f32", VOP_F32_F32>;
  } // End TRANS = 1, SchedRW = [WriteTrans32]
} // End SubtargetPredicate = isGFX7GFX8GFX9

And note that the way the predicate is set on the instructions with let SubtargetPredicate = (where SubtargetPredicate is used by a class called PredicateControl which is a superclass of VOP1_Pseudo, VOP3_Pseudo et al) does not automatically set a corresponding predicate on the MnemonicAliases defined by the multiclass.

[nlguillemot]

LGTM!

llvm/test/TableGen/MnemonicAlias.td

+// RUN: llvm-tblgen -gen-asm-matcher -I %p/../../include %s | FileCheck %s
+
+include "llvm/Target/Target.td"
+
+def ArchInstrInfo : InstrInfo { }
+
+def Arch : Target {
+  let InstructionSet = ArchInstrInfo;
+}
+
+def Reg : Register<"reg">;
+def RegClass : RegisterClass<"foo", [i32], 0, (add Reg)>;
+
+def AsmCond1 : SubtargetFeature<"cond1", "cond1", "true", "">;
+def AsmCond2 : SubtargetFeature<"cond2", "cond2", "true", "">;
+
+def Subtarget1 : Predicate<"Pred1">, AssemblerPredicate<(all_of AsmCond1)>;
+def Subtarget2 : Predicate<"Pred2">, AssemblerPredicate<(all_of AsmCond2)>;
+
+multiclass DefInstruction<string name, dag outs, dag ins, Predicate pred> {
+  def "" : Instruction {
+    let Size = 2;
+    let OutOperandList = outs;
+    let InOperandList = ins;
+    let AsmString = name;
+    let Predicates = [pred];
+  }
+  def : MnemonicAlias<name # "_alias", name>;
+}
+
+defm FooInst1 : DefInstruction<"foo", (outs), (ins), Subtarget1>;
+
+defm FooInst2 : DefInstruction<"foo", (outs), (ins), Subtarget2>;
+
+// Check that applyMnemonicAliases maps "foo_alias" to "foo" once only and
+// without checking any predicates.
+
+// CHECK:      if (memcmp(Mnemonic.data()+0, "foo_alias", 9) != 0)
+// CHECK-NEXT:   break;
+// CHECK-NEXT: Mnemonic = "foo";        // "foo_alias"
+// CHECK-NEXT: return;

llvm/utils/TableGen/AsmMatcherEmitter.cpp

for (const auto &AliasEntry : AliasesFromMnemonic) {
 
     for (unsigned i = 0, e = ToVec.size(); i != e; ++i) {
       Record *R = ToVec[i];
       std::string FeatureMask = GetAliasRequiredFeatures(R, Info);
 
       // If this unconditionally matches, remember it for later and diagnose
       // duplicates.
       if (FeatureMask.empty()) {
-        if (AliasWithNoPredicate != -1) {
-          // We can't have two aliases from the same mnemonic with no predicate.
-          PrintError(ToVec[AliasWithNoPredicate]->getLoc(),
-                     "two MnemonicAliases with the same 'from' mnemonic!");
+        if (AliasWithNoPredicate != -1 &&
+            R->getValueAsString("ToMnemonic") !=
+                ToVec[AliasWithNoPredicate]->getValueAsString("ToMnemonic")) {
+          // We can't have two different aliases from the same mnemonic with no
+          // predicate.
+          PrintError(
+              ToVec[AliasWithNoPredicate]->getLoc(),
+              "two different MnemonicAliases with the same 'from' mnemonic!");
           PrintFatalError(R->getLoc(), "this is the other MnemonicAlias.");
         }
 
         AliasWithNoPredicate = i;
         continue;
       }
       if (R->getValueAsString("ToMnemonic") == AliasEntry.first)
         PrintFatalError(R->getLoc(), "MnemonicAlias to the same string");