[PATCH] D55089: [TableGen] Fix negation of simple predicates

[Andrea Di Biagio via Phabricator via llvm-commits]

andreadb added a comment.

Hi Evandro.

The two new methods added by your patch are mostly equivalent to `expandCheckImmOperand` and `expandCheckRegOperand`.
Rather than introducing those new methods, I think a better fix is to add checks for null/empty operands in `expandCheckImmOperand` and `expandCheckRegOperand`.

Basically, I am thinking about something like this:

  Index: TableGen/PredicateExpander.cpp
  ===================================================================
  --- TableGen/PredicateExpander.cpp      (revision 347858)
  +++ TableGen/PredicateExpander.cpp      (working copy)
  @@ -31,14 +31,16 @@
   }
  
   void PredicateExpander::expandCheckImmOperand(raw_ostream &OS, int OpIndex,
                                                 StringRef ImmVal,
                                                 StringRef FunctionMapper) {
     if (!FunctionMapper.empty())
       OS << FunctionMapper << "(";
  +  if (ImmVal.empty() && shouldNegate())
  +    OS << '!';
     OS << "MI" << (isByRef() ? "." : "->") << "getOperand(" << OpIndex
        << ").getImm()";
  
     OS << (FunctionMapper.empty() ? "" : ")");
     if (ImmVal.empty())
       return;
     OS << (shouldNegate() ? " != " : " == ") << ImmVal;
  @@ -47,14 +49,16 @@
   void PredicateExpander::expandCheckRegOperand(raw_ostream &OS, int OpIndex,
                                                 const Record *Reg,
                                                 StringRef FunctionMapper) {
     assert(Reg->isSubClassOf("Register") && "Expected a register Record!");
  
     if (!FunctionMapper.empty())
       OS << FunctionMapper << "(";
  +  if (!Reg && shouldNegate())
  +    OS << '!';
     OS << "MI" << (isByRef() ? "." : "->") << "getOperand(" << OpIndex
        << ").getReg()";
     OS << (FunctionMapper.empty() ? "" : ")");
     if (!Reg)
       return;
     OS << (shouldNegate() ? " != " : " == ");
     const StringRef Str = Reg->getValueAsString("Namespace");

As a side note: your patch made me realize that the declarations of `expandCheckRegOperandSimple` and `expandCheckImmOperandSimple` in the header file are pretty much dead. In case, (if you agree with this alternative approach), your patch should be able to remove them from the header file.

-Andrea


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55089/new/

https://reviews.llvm.org/D55089