lflow: Correct register definitions to use subfields for overlaps.

OVN expressions need to know what fields overlap or alias one another.
This is supposed to be done via subfields: if two fields overlap, then the
smaller one should be defined as a subfield of the larger one.  For
example, reg0 should be defined as xxreg0[96..127].  The symbol table in
lflow didn't do this, so it's possible for confusion to result.  (I don't
have evidence of this actually happening, because it would only occur
in a case where the same bits of a field were referred to with different
names.)

This commit fixes the problem.  It deserves a test, but that's somewhat
difficult at this point, so it will actually happen in a future commit.

Signed-off-by: Ben Pfaff <blp@ovn.org>
Acked-by: Ryan Moats <rmoats@us.ibm.com>

1 files changed, 45 insertions, 5 deletions

diff --git a/ovn/controller/lflow.c b/ovn/controller/lflow.c
index 9eb92c838..71d42a73c 100644
--- a/ovn/controller/lflow.c
+++ b/ovn/controller/lflow.c
@@ -52,6 +52,20 @@ lflow_reset_processing(void)
     physical_reset_processing();
 }
 
+static void
+add_subregister(const char *name,
+                const char *parent_name, int parent_idx,
+                int width, int idx,
+                struct shash *symtab)
+{
+    int lsb = width * idx;
+    int msb = lsb + (width - 1);
+    char *expansion = xasprintf("%s%d[%d..%d]",
+                                parent_name, parent_idx, lsb, msb);
+    expr_symtab_add_subfield(symtab, name, NULL, expansion);
+    free(expansion);
+}
+
 void
 lflow_init(void)
 {
@@ -64,16 +78,42 @@ lflow_init(void)
     expr_symtab_add_string(&symtab, "inport", MFF_LOG_INPORT, NULL);
     expr_symtab_add_string(&symtab, "outport", MFF_LOG_OUTPORT, NULL);
 
-    /* Logical registers. */
+    /* Logical registers:
+     *     128-bit xxregs
+     *     64-bit xregs
+     *     32-bit regs
+     *
+     * The expression language doesn't handle overlapping fields properly
+     * unless they're formally defined as subfields.  It's a little awkward. */
+    for (int xxi = 0; xxi < MFF_N_LOG_REGS / 4; xxi++) {
+        char *xxname = xasprintf("xxreg%d", xxi);
+        expr_symtab_add_field(&symtab, xxname, MFF_XXREG0 + xxi, NULL, false);
+        free(xxname);
+    }
+    for (int xi = 0; xi < MFF_N_LOG_REGS / 2; xi++) {
+        char *xname = xasprintf("xreg%d", xi);
+        int xxi = xi / 2;
+        if (xxi < MFF_N_LOG_REGS / 4) {
+            add_subregister(xname, "xxreg", xxi, 64, 1 - xi % 2, &symtab);
+        } else {
+            expr_symtab_add_field(&symtab, xname, MFF_XREG0 + xi, NULL, false);
+        }
+        free(xname);
+    }
     for (int i = 0; i < MFF_N_LOG_REGS; i++) {
         char *name = xasprintf("reg%d", i);
-        expr_symtab_add_field(&symtab, name, MFF_LOG_REG0 + i, NULL, false);
+        int xxi = i / 4;
+        int xi = i / 2;
+        if (xxi < MFF_N_LOG_REGS / 4) {
+            add_subregister(name, "xxreg", xxi, 32, 3 - i % 4, &symtab);
+        } else if (xi < MFF_N_LOG_REGS / 2) {
+            add_subregister(name, "xreg", xi, 32, 1 - i % 2, &symtab);
+        } else {
+            expr_symtab_add_field(&symtab, name, MFF_REG0 + i, NULL, false);
+        }
         free(name);
     }
 
-    expr_symtab_add_field(&symtab, "xxreg0", MFF_XXREG0, NULL, false);
-    expr_symtab_add_field(&symtab, "xxreg1", MFF_XXREG1, NULL, false);
-
     /* Flags used in logical to physical transformation. */
     expr_symtab_add_field(&symtab, "flags", MFF_LOG_FLAGS, NULL, false);
     char flags_str[16];