[llvm] r222038 - Change order of tablegen generated fast-isel instruction code to be

NAKAMURA Takumi geek4civic at gmail.com
Sat Nov 15 13:47:07 PST 2014


Bill, thanks for the information. FYI, it made my builders orange.
They don't use -Werror, but buildsteps are picking up warnings.

http://bb.pgr.jp/waterfall

2014-11-15 23:28 GMT+09:00 Bill Schmidt <wschmidt at linux.vnet.ibm.com>:
> On Sat, 2014-11-15 at 21:27 +0900, NAKAMURA Takumi wrote:
>> It causes warnings in X86GenFastISel.inc.
>
> Yes, it does.  That's because there's a bug in the X86 back end that was
> discovered by this work.  See
> http://llvm.org/bugs/show_bug.cgi?id=21575.
>
>>
>> [15/37] cd lib/Target/X86 && bin/llvm-tblgen -gen-fast-isel -I
>> llvm-project/llvm/lib/Target/X86 -I llvm-project/llvm/lib/Target -I
>> llvm-project/llvm/include llvm-project/llvm/lib/Target/X86/X86.td -o
>> lib/Target/X86/X86GenFastISel.inc.tmp
>> warning:Multiple instructions match and one with no predicate came
>> before one with a predicate!  name:VBROADCASTSSZr  predicate:
>> (Subtarget->hasAVX512())
>> warning:Multiple instructions match and one with no predicate came
>> before one with a predicate!  name:VBROADCASTSDZr  predicate:
>> (Subtarget->hasAVX512())
>>
>> 2014-11-15 6:05 GMT+09:00 Bill Schmidt <wschmidt at linux.vnet.ibm.com>:
>> > Author: wschmidt
>> > Date: Fri Nov 14 15:05:45 2014
>> > New Revision: 222038
>> >
>> > URL: http://llvm.org/viewvc/llvm-project?rev=222038&view=rev
>> > Log:
>> > Change order of tablegen generated fast-isel instruction code to be
>> > based on instruction complexity
>> >
>> > The order that tablegen fast-isel instruction code is generated is
>> > currently based on the text of the predicate (using string
>> > less-than). This patch changes this to instead use the instruction
>> > complexity. Because the complexities are not unique a C++ multimap is
>> > used instead of a map.
>> >
>> > This fixes the problem where code with no predicate always comes out
>> > first (the empty string always compares as less than all other
>> > strings) thus making the code with predicates dead code. See the FMUL
>> > code in PPCFastISel.cpp for an example. It also more closely matches
>> > the normal codegen ordering. Some error checking in the tablegen
>> > fast-isel code is fixed as well.
>> >
>> > Patch by Bill Seurer.
>> >
>> >
>> > Modified:
>> >     llvm/trunk/utils/TableGen/FastISelEmitter.cpp
>> >
>> > Modified: llvm/trunk/utils/TableGen/FastISelEmitter.cpp
>> > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/utils/TableGen/FastISelEmitter.cpp?rev=222038&r1=222037&r2=222038&view=diff
>> > ==============================================================================
>> > --- llvm/trunk/utils/TableGen/FastISelEmitter.cpp (original)
>> > +++ llvm/trunk/utils/TableGen/FastISelEmitter.cpp Fri Nov 14 15:05:45 2014
>> > @@ -37,6 +37,7 @@ struct InstructionMemo {
>> >    const CodeGenRegisterClass *RC;
>> >    std::string SubRegNo;
>> >    std::vector<std::string>* PhysRegs;
>> > +  std::string PredicateCheck;
>> >  };
>> >  } // End anonymous namespace
>> >
>> > @@ -365,7 +366,9 @@ struct OperandsSignature {
>> >
>> >  namespace {
>> >  class FastISelMap {
>> > -  typedef std::map<std::string, InstructionMemo> PredMap;
>> > +  // A multimap is needed instead of a "plain" map because the key is
>> > +  // the instruction's complexity (an int) and they are not unique.
>> > +  typedef std::multimap<int, InstructionMemo> PredMap;
>> >    typedef std::map<MVT::SimpleValueType, PredMap> RetPredMap;
>> >    typedef std::map<MVT::SimpleValueType, RetPredMap> TypeRetPredMap;
>> >    typedef std::map<std::string, TypeRetPredMap> OpcodeTypeRetPredMap;
>> > @@ -374,6 +377,16 @@ class FastISelMap {
>> >
>> >    OperandsOpcodeTypeRetPredMap SimplePatterns;
>> >
>> > +  // This is used to check that there are no duplicate predicates
>> > +  typedef std::multimap<std::string, bool> PredCheckMap;
>> > +  typedef std::map<MVT::SimpleValueType, PredCheckMap> RetPredCheckMap;
>> > +  typedef std::map<MVT::SimpleValueType, RetPredCheckMap> TypeRetPredCheckMap;
>> > +  typedef std::map<std::string, TypeRetPredCheckMap> OpcodeTypeRetPredCheckMap;
>> > +  typedef std::map<OperandsSignature, OpcodeTypeRetPredCheckMap>
>> > +            OperandsOpcodeTypeRetPredCheckMap;
>> > +
>> > +  OperandsOpcodeTypeRetPredCheckMap SimplePatternsCheck;
>> > +
>> >    std::map<OperandsSignature, std::vector<OperandsSignature> >
>> >      SignaturesWithConstantForms;
>> >
>> > @@ -385,6 +398,11 @@ public:
>> >    void collectPatterns(CodeGenDAGPatterns &CGP);
>> >    void printImmediatePredicates(raw_ostream &OS);
>> >    void printFunctionDefinitions(raw_ostream &OS);
>> > +private:
>> > +  void emitInstructionCode(raw_ostream &OS,
>> > +                           const OperandsSignature &Operands,
>> > +                           const PredMap &PM,
>> > +                           const std::string &RetVTName);
>> >  };
>> >  } // End anonymous namespace
>> >
>> > @@ -561,14 +579,24 @@ void FastISelMap::collectPatterns(CodeGe
>> >        Pattern.getDstPattern()->getOperator()->getName(),
>> >        DstRC,
>> >        SubRegNo,
>> > -      PhysRegInputs
>> > +      PhysRegInputs,
>> > +      PredicateCheck
>> >      };
>> > +
>> > +    int complexity = Pattern.getPatternComplexity(CGP);
>> >
>> > -    if (SimplePatterns[Operands][OpcodeName][VT][RetVT].count(PredicateCheck))
>> > +    if (SimplePatternsCheck[Operands][OpcodeName][VT]
>> > +         [RetVT].count(PredicateCheck)) {
>> >        PrintFatalError(Pattern.getSrcRecord()->getLoc(),
>> > -                    "Duplicate record in FastISel table!");
>> > +                    "Duplicate predicate in FastISel table!");
>> > +    }
>> > +    SimplePatternsCheck[Operands][OpcodeName][VT][RetVT].insert(
>> > +            std::make_pair(PredicateCheck, true));
>> >
>> > -    SimplePatterns[Operands][OpcodeName][VT][RetVT][PredicateCheck] = Memo;
>> > +       // Note: Instructions with the same complexity will appear in the order
>> > +          // that they are encountered.
>> > +    SimplePatterns[Operands][OpcodeName][VT][RetVT].insert(
>> > +      std::make_pair(complexity, Memo));
>> >
>> >      // If any of the operands were immediates with predicates on them, strip
>> >      // them down to a signature that doesn't have predicates so that we can
>> > @@ -594,6 +622,72 @@ void FastISelMap::printImmediatePredicat
>> >    OS << "\n\n";
>> >  }
>> >
>> > +void FastISelMap::emitInstructionCode(raw_ostream &OS,
>> > +                                      const OperandsSignature &Operands,
>> > +                                      const PredMap &PM,
>> > +                                      const std::string &RetVTName) {
>> > +  // Emit code for each possible instruction. There may be
>> > +  // multiple if there are subtarget concerns.  A reverse iterator
>> > +  // is used to produce the ones with highest complexity first.
>> > +
>> > +  bool OneHadNoPredicate = false;
>> > +  for (PredMap::const_reverse_iterator PI = PM.rbegin(), PE = PM.rend();
>> > +       PI != PE; ++PI) {
>> > +    const InstructionMemo &Memo = PI->second;
>> > +    std::string PredicateCheck = Memo.PredicateCheck;
>> > +
>> > +    if (PredicateCheck.empty()) {
>> > +      assert(!OneHadNoPredicate &&
>> > +             "Multiple instructions match and more than one had "
>> > +             "no predicate!");
>> > +      OneHadNoPredicate = true;
>> > +    } else {
>> > +      if (OneHadNoPredicate) {
>> > +        // FIXME: This should be a PrintError once the x86 target
>> > +        // fixes PR21575.
>> > +        PrintWarning("Multiple instructions match and one with no "
>> > +                     "predicate came before one with a predicate!  "
>> > +                     "name:" + Memo.Name + "  predicate: " +
>> > +                     PredicateCheck);
>> > +      }
>> > +      OS << "  if (" + PredicateCheck + ") {\n";
>> > +      OS << "  ";
>> > +    }
>> > +
>> > +    for (unsigned i = 0; i < Memo.PhysRegs->size(); ++i) {
>> > +      if ((*Memo.PhysRegs)[i] != "")
>> > +        OS << "  BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DbgLoc, "
>> > +           << "TII.get(TargetOpcode::COPY), "
>> > +           << (*Memo.PhysRegs)[i] << ").addReg(Op" << i << ");\n";
>> > +    }
>> > +
>> > +    OS << "  return fastEmitInst_";
>> > +    if (Memo.SubRegNo.empty()) {
>> > +      Operands.PrintManglingSuffix(OS, *Memo.PhysRegs,
>> > +     ImmediatePredicates, true);
>> > +      OS << "(" << InstNS << Memo.Name << ", ";
>> > +      OS << "&" << InstNS << Memo.RC->getName() << "RegClass";
>> > +      if (!Operands.empty())
>> > +        OS << ", ";
>> > +      Operands.PrintArguments(OS, *Memo.PhysRegs);
>> > +      OS << ");\n";
>> > +    } else {
>> > +      OS << "extractsubreg(" << RetVTName
>> > +         << ", Op0, Op0IsKill, " << Memo.SubRegNo << ");\n";
>> > +    }
>> > +
>> > +    if (!PredicateCheck.empty()) {
>> > +      OS << "  }\n";
>> > +    }
>> > +  }
>> > +  // Return 0 if all of the possibilities had predicates but none
>> > +  // were satisfied.
>> > +  if (!OneHadNoPredicate)
>> > +    OS << "  return 0;\n";
>> > +  OS << "}\n";
>> > +  OS << "\n";
>> > +}
>> > +
>> >
>> >  void FastISelMap::printFunctionDefinitions(raw_ostream &OS) {
>> >    // Now emit code for all the patterns that we collected.
>> > @@ -620,7 +714,6 @@ void FastISelMap::printFunctionDefinitio
>> >                 RI != RE; ++RI) {
>> >              MVT::SimpleValueType RetVT = RI->first;
>> >              const PredMap &PM = RI->second;
>> > -            bool HasPred = false;
>> >
>> >              OS << "unsigned fastEmit_"
>> >                 << getLegalCName(Opcode)
>> > @@ -631,54 +724,7 @@ void FastISelMap::printFunctionDefinitio
>> >              Operands.PrintParameters(OS);
>> >              OS << ") {\n";
>> >
>> > -            // Emit code for each possible instruction. There may be
>> > -            // multiple if there are subtarget concerns.
>> > -            for (PredMap::const_iterator PI = PM.begin(), PE = PM.end();
>> > -                 PI != PE; ++PI) {
>> > -              std::string PredicateCheck = PI->first;
>> > -              const InstructionMemo &Memo = PI->second;
>> > -
>> > -              if (PredicateCheck.empty()) {
>> > -                assert(!HasPred &&
>> > -                       "Multiple instructions match, at least one has "
>> > -                       "a predicate and at least one doesn't!");
>> > -              } else {
>> > -                OS << "  if (" + PredicateCheck + ") {\n";
>> > -                OS << "  ";
>> > -                HasPred = true;
>> > -              }
>> > -
>> > -              for (unsigned i = 0; i < Memo.PhysRegs->size(); ++i) {
>> > -                if ((*Memo.PhysRegs)[i] != "")
>> > -                  OS << "  BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DbgLoc, "
>> > -                     << "TII.get(TargetOpcode::COPY), "
>> > -                     << (*Memo.PhysRegs)[i] << ").addReg(Op" << i << ");\n";
>> > -              }
>> > -
>> > -              OS << "  return fastEmitInst_";
>> > -              if (Memo.SubRegNo.empty()) {
>> > -                Operands.PrintManglingSuffix(OS, *Memo.PhysRegs,
>> > -                                             ImmediatePredicates, true);
>> > -                OS << "(" << InstNS << Memo.Name << ", ";
>> > -                OS << "&" << InstNS << Memo.RC->getName() << "RegClass";
>> > -                if (!Operands.empty())
>> > -                  OS << ", ";
>> > -                Operands.PrintArguments(OS, *Memo.PhysRegs);
>> > -                OS << ");\n";
>> > -              } else {
>> > -                OS << "extractsubreg(" << getName(RetVT);
>> > -                OS << ", Op0, Op0IsKill, " << Memo.SubRegNo << ");\n";
>> > -              }
>> > -
>> > -              if (HasPred)
>> > -                OS << "  }\n";
>> > -
>> > -            }
>> > -            // Return 0 if none of the predicates were satisfied.
>> > -            if (HasPred)
>> > -              OS << "  return 0;\n";
>> > -            OS << "}\n";
>> > -            OS << "\n";
>> > +            emitInstructionCode(OS, Operands, PM, getName(RetVT));
>> >            }
>> >
>> >            // Emit one function for the type that demultiplexes on return type.
>> > @@ -720,58 +766,8 @@ void FastISelMap::printFunctionDefinitio
>> >               << ")\n    return 0;\n";
>> >
>> >            const PredMap &PM = RM.begin()->second;
>> > -          bool HasPred = false;
>> > -
>> > -          // Emit code for each possible instruction. There may be
>> > -          // multiple if there are subtarget concerns.
>> > -          for (PredMap::const_iterator PI = PM.begin(), PE = PM.end(); PI != PE;
>> > -               ++PI) {
>> > -            std::string PredicateCheck = PI->first;
>> > -            const InstructionMemo &Memo = PI->second;
>> > -
>> > -            if (PredicateCheck.empty()) {
>> > -              assert(!HasPred &&
>> > -                     "Multiple instructions match, at least one has "
>> > -                     "a predicate and at least one doesn't!");
>> > -            } else {
>> > -              OS << "  if (" + PredicateCheck + ") {\n";
>> > -              OS << "  ";
>> > -              HasPred = true;
>> > -            }
>> > -
>> > -            for (unsigned i = 0; i < Memo.PhysRegs->size(); ++i) {
>> > -              if ((*Memo.PhysRegs)[i] != "")
>> > -                OS << "  BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DbgLoc, "
>> > -                   << "TII.get(TargetOpcode::COPY), "
>> > -                   << (*Memo.PhysRegs)[i] << ").addReg(Op" << i << ");\n";
>> > -            }
>> > -
>> > -            OS << "  return fastEmitInst_";
>> > -
>> > -            if (Memo.SubRegNo.empty()) {
>> > -              Operands.PrintManglingSuffix(OS, *Memo.PhysRegs,
>> > -                                           ImmediatePredicates, true);
>> > -              OS << "(" << InstNS << Memo.Name << ", ";
>> > -              OS << "&" << InstNS << Memo.RC->getName() << "RegClass";
>> > -              if (!Operands.empty())
>> > -                OS << ", ";
>> > -              Operands.PrintArguments(OS, *Memo.PhysRegs);
>> > -              OS << ");\n";
>> > -            } else {
>> > -              OS << "extractsubreg(RetVT, Op0, Op0IsKill, ";
>> > -              OS << Memo.SubRegNo;
>> > -              OS << ");\n";
>> > -            }
>> > -
>> > -             if (HasPred)
>> > -               OS << "  }\n";
>> > -          }
>> >
>> > -          // Return 0 if none of the predicates were satisfied.
>> > -          if (HasPred)
>> > -            OS << "  return 0;\n";
>> > -          OS << "}\n";
>> > -          OS << "\n";
>> > +          emitInstructionCode(OS, Operands, PM, "RetVT");
>> >          }
>> >        }
>> >
>> >
>> >
>> > _______________________________________________
>> > llvm-commits mailing list
>> > llvm-commits at cs.uiuc.edu
>> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>
>



More information about the llvm-commits mailing list