[TableGen] Prevent invalid code generation when emitting AssemblerPredicate conditions.

[tomatabacu]

Summary

The loop which emits AssemblerPredicate conditions also links them together by emitting a '&&'.
If the 1st predicate is not an AssemblerPredicate, while the 2nd one is, nothing gets emitted for the 1st one, but we still emit the '&&' because of the 2nd predicate.
This generated code looks like "( && Cond2)" and is invalid.

[dsanders]

I don't think the commit message explains this change very well. If we hadn't already discussed the problem off-list I'm not sure I would have understood what you meant. I'd have started with a subject like:

[tablegen] Fix invalid code generation when the first predicate is not an AssemblerMatcherPredicate.

and then talked about the way it currently produces 'if ( && B)' in the body of the message.

test/TableGen/PredicateOrderDisasm.td

3–5

3–11

This is a bit strange to read. Maybe simply:

Check that we don't accidentally generate code of the form 'if ( && B)'.

is enough.

37

It's better to check that the correct thing happens rather than check that the wrong thing doesn't happen.

[tomatabacu]

Rewrote comments.
Switched to CHECK-ing for correct behavior.
Renamed test file to a more appropriate name.

[tomatabacu]

Ping.

[dsanders]

LGTM

test/TableGen/PredicateOrderDisasm.td

+// RUN: llvm-tblgen -gen-disassembler -I %p/../../include %s | FileCheck %s
+
+// Check that we don't try to logically link together the 1st and 2nd generated
+// Predicate conditions if for the 1st Predicate we didn't actually emit
+// anything.
+
+// In the example below, Pred1 is not an AssemblerPredicate, while Pred2 is.
+// When doing -gen-disassembler, we only care about AssemblerPredicate's, so
+// we don't emit the condition for Pred1, but we would still try to link the
+// condition for Pred2 with the (inexistent) one for Pred1 by emitting "&&".
+// This would result in invalid code being generated.
+
+include "llvm/Target/Target.td"
+
+def archInstrInfo : InstrInfo { }
+
+def arch : Target {
+  let InstructionSet = archInstrInfo;
+}
+
+def Pred1 : Predicate<"Condition1">;
+def Pred2 : Predicate<"Condition2">,
+            AssemblerPredicate<"AssemblerCondition2">;
+
+def foo : Instruction {
+  let Size = 2;
+  let OutOperandList = (outs);
+  let InOperandList = (ins);
+  field bits<16> Inst;
+  let Inst = 0xAAAA;
+  let AsmString = "foo";
+  field bits<16> SoftFail = 0;
+  // This is the important bit:
+  let Predicates = [Pred1, Pred2];
+}
+
+// CHECK-NOT: ( && (Bits & arch::AssemblerCondition2))

utils/TableGen/FixedLenDecoderEmitter.cpp

static void emitSinglePredicateMatch(raw_ostream &o, StringRef str,
   else
     o << "(Bits & " << PredicateNamespace << "::" << str << ")";
 }
 
 bool FilterChooser::emitPredicateMatch(raw_ostream &o, unsigned &Indentation,
                                        unsigned Opc) const {
   ListInit *Predicates =
     AllInstructions[Opc]->TheDef->getValueAsListInit("Predicates");
+  bool IsFirstEmission = true;
   for (unsigned i = 0; i < Predicates->getSize(); ++i) {
     Record *Pred = Predicates->getElementAsRecord(i);
     if (!Pred->getValue("AssemblerMatcherPredicate"))
       continue;
 
     std::string P = Pred->getValueAsString("AssemblerCondString");
 
     if (!P.length())
       continue;
 
-    if (i != 0)
+    if (!IsFirstEmission)
       o << " && ";
 
     StringRef SR(P);
     std::pair<StringRef, StringRef> pairs = SR.split(',');
     while (pairs.second.size()) {
       emitSinglePredicateMatch(o, pairs.first, Emitter->PredicateNamespace);
       o << " && ";
       pairs = pairs.second.split(',');
     }
     emitSinglePredicateMatch(o, pairs.first, Emitter->PredicateNamespace);
+    IsFirstEmission = false;
   }
   return Predicates->getSize() > 0;
 }
 
 bool FilterChooser::doesOpcodeNeedPredicate(unsigned Opc) const {
   ListInit *Predicates =
     AllInstructions[Opc]->TheDef->getValueAsListInit("Predicates");
   for (unsigned i = 0; i < Predicates->getSize(); ++i) {