[PATCH] D67968: [TableGen] Introduce a generic automaton (DFA) backend

Mon Sep 30 11:08:39 PDT 2019

dsanders added a comment.

In D67968#1682549 <https://reviews.llvm.org/D67968#1682549>, @jmolloy wrote:

> Perhaps you might have some thoughts on this patch? It's a step towards slightly easier bundle packing and similar problems for VLIWs.

Hi James,

Sorry for the slow reply. Our target isn't a VLIW, our instructions words are just very long :-). We don't have the usual VLIW issues, it just takes a lot of bits to describe some operations.

I haven't dug into the implementation code but FWIW, the definition makes sense to me. It took me a little while to pick up on how State bits being 1 prevented transitions that has NewState be 1 in that same bit and it might be worth commenting on that in the tests.

================
Comment at: llvm/unittests/TableGen/AutomataTest.cpp:101-117
+TEST(Automata, BinPackerAutomatonAccepts) {
+  Automaton<BinPackerAutomatonAction> A(makeArrayRef(BinPackerAutomatonTransitions));
+
+  // Expect that we can pack two double-bins in 0-4, then no more in 0-4.
+  A.reset();
+  EXPECT_TRUE(A.add(BRK_0_to_4_dbl));
+  EXPECT_TRUE(A.add(BRK_0_to_4_dbl));
----------------
I notice that there isn't a test where one of the two-bin constraints is trying to pack into a state where only one of the two bins are available.
Shouldn't there be one that does something like pack BRK_0_to_6 five times and then checks that packing BRK_0_to_6_dbl fails?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67968