bmv2 "array index must be a constant" error #2007

hesingh · 2019-07-20T23:00:00Z

I am using a header stack in v1model p4-16 code. The index into the header stack array is obtained from a header in the packet and index copied to metadata. Obviously, the header is parsed in the parser. p4c-bm2-ss fails because the index is not a compile-time constant. So, how is such code supposed to work? The p4test backend does not care and compiles the code fine.

./testdata/p4_16_samples/ipv6-switch-ml-bmv2.p4(111): [--Werror=invalid] error: meta._ml_idx4: Invalid all array indexes must be constant
             hdr.vector[0].e = pool[meta.ml_idx].val;
                                    ^^^^

The text was updated successfully, but these errors were encountered:

jafingerhut · 2019-07-20T23:36:47Z

Section 8.15 "Operations on header stacks" of the P4_16 language spec: https://p4.org/p4-spec/docs/P4-16-v1.1.0-spec.html#sec-expr-hs

says this: "Some architectures may impose the constraint that the index expression evaluates to a compile-time known value"

v1model is such an architecture that imposes this constraint.

If you want to access one of a handful of different header stack elements in actions depending upon a run-time varying value, one workaround for the v1model restriction is to include the run-time variable index as a table search key, which selects between several different actions that are identical, except for the constant array index values they use.

hesingh · 2019-07-20T23:50:35Z

Right - I do understand why bmv2 imposes the restriction because there is no JSON one can generate for a non-compile-time index. Sorry, I forgot about using tables - make sense, thanks!

hesingh · 2019-07-21T00:21:23Z

It's not a handful of elements. I have 128 and each can be used a index for the header stack array.

hesingh · 2019-07-21T00:24:10Z

Both the issues I filed today are related to switch Machine Learning (ML) where array elements are added and array indices are used for aggregating ML data into different aggregators. I think the switch ML on Tofino may be using p4-14. I am writing p4-16 and v1model for the IETF hackathon this weekend.

jafingerhut · 2019-07-21T00:26:22Z

The approach of multiple actions works for an arbitrary finite number of elements. I only mentioned a handful since I was guessing that might be the case, but apparently it is not for what you want. Again, auto-generation/printing of repetitive code might be your friend here.

I agree that it would be nice if all P4 targets supported run-time variable indexes of arrays. The fact is that the statement is in the language spec precisely because someone wanted to caution people about expecting that, at least at the time it was written, and I doubt that the highest speed implementations have changed significantly since that writing.

hesingh · 2019-07-21T00:29:59Z

Agree on auto-generation - thanks. Maybe, we keep this issue open and explore what's it take for the compiler to support run-time array index. Also, in April 2019, using p4c of that time, I could use if-statement inside action. Now, I can't.

jafingerhut · 2019-07-21T00:48:05Z

Some if statements inside of actions should still work just fine. If you have an example of one that used to work with older p4c and doesn't with latest p4c, I'd be interested in seeing an example.

It is not all if statements that used to work -- if the compiler can figure out how to auto-translate them into using a ternary operator on the right-hand side of an assignment statement, it should work, e.g. (expression) ? (true_expr) : (false_expr). If the compiler cannot figure out how to auto-translate it into that form, I do not believe p4c has ever supported it.

See here for some details: https://github.com/p4lang/behavioral-model/blob/master/docs/simple_switch.md#restrictions-on-code-within-actions

hesingh · 2019-07-21T01:00:24Z

This is what worked inside an action during April 2019, but did not later. Don't have exact dates handy.

        if (hdr.ipv6.isValid() || hdr.inner_ipv6.isValid()) {
	    hdr.vlan_tag_[0].etherType = 16w0x86dd;
	} else {
	    hdr.vlan_tag_[0].etherType = 16w0x800;
	}

jafingerhut · 2019-07-21T01:46:39Z

I just tried making that the body of an action in a program where those headers were defined, and it compiled fine with the latest p4c --target bmv2 --arch v1model myprog.p4 command, and write a BMv2 JSON file.

Are you getting a compilation error when trying to compile a program with such an if statement inside of an action? Or it produces a BMv2 JSON file that doesn't seem to be working correctly? Something else?

hesingh · 2019-07-21T09:35:02Z

I too tried ./p4c-bm2-ss --std p4-16 foo.p4 with the if-statements above in action body and p4c behaves fine.

Now I remember what transpired. When I moved my code to run on another target hardware, the action body with if-statement was not accepted.

jnfoster · 2019-07-21T11:39:04Z

This is not an appropriate forum for discussing the capabilities or restrictions of backends for proprietary P4_16 targets.

hesingh · 2019-07-21T12:02:47Z

@jnfoster Sorry, my bad. I edited the comment.

hesingh · 2019-12-16T14:21:25Z

@jafingerhut I wonder if the v1model.p4 could be changed to allow run-time indices. If not, would the PSA model consider making the change to the bmv2 backend?

mihaibudiu · 2019-12-16T16:23:40Z

How would you change v1model?

hesingh · 2019-12-16T16:30:34Z

Sorry, I used incorrect terminology.

It's the v1model.p4 architecture being used by the bmv2 simple_switch backend that imposes the constraint. The backend would need a change.

jafingerhut · 2019-12-16T16:32:03Z

It seems that modifying the open source tools p4c and behavioral-model to support this would be some changes to each of them. e.g. I think this might cover it, but haven't gone very far in examining the details:

add a way to the BMv2 JSON data file to provide a run-time expression to use as the index, when accessing a header stack
generate that new BMv2 JSON construct from the p4c BMv2 back end
verify that the p4c front end can handle run-time variables as header stack index expressions, without violating any constraints or assumptions made when its internals were developed, enhancing any parts where that assumption was formerly baked into the code.

All of that could be done for v1model and/or PSA.

I suspect that more accurate than the current statement in the spec "Some architectures may impose the constraint that the index expression evaluates to a compile-time known value" is this: "Some targets may ...". Making all of the changes above would encourage target developers to consider supporting run-time index values, but it would not magically change a target that physically could not do it, restricted by its design, into one that could.

hesingh · 2019-12-16T17:58:41Z

I always start with p4c frontend to see if a new feature requires any change. The Stack's next, last, push, pop can only be used in the P4 parser. The ML paper has algorithm 1 on page 4 using run-time index.

https://arxiv.org/pdf/1903.06701.pdf

We may have to use the run-time index outside of the P4 parser in an ingress control. Also, I think, the Stack IR may change to incorporate a run-time index.

Thereafter, the next crucial issue is to define JSON for the new run-time index.

Another operative question is for asic/FPGA vendors if they can support such a feature in their hardware. Intel asic and FPGA/Mellanox/Cisco/Xilinx should weigh in. If that's not possible without signing NDA, never mind.

jafingerhut · 2019-12-16T18:12:43Z

Minor correction to your previous comment: The P4_16 language spec restricts next and last on header stacks to within parsers.

It makes no such restrictions on push_front and pop_front operations, and the current p4c and behavioral-model implementations allow those operations to be invoked during control execution. In fact, for those, it is not obvious to me whether they work if invoked inside of a parser -- they might, but I doubt there is much reason a P4 program author would want to perform those operations in a parser, so it seems fairly low priority to find out what the current implementation does there.

In general, anything that can be done in software could be done in an FPGA, without changing the FPGA hardware design.

ASICs are the targets that might have designs preventing them from doing this. If you expect a public answer to this Github comment on this topic, I expect you will be disappointed (but I am happy to be proved wrong on that).

hesingh · 2019-12-16T18:22:29Z

See this line of code in that restricts push and pop of Stack to be allowed only in parser.

https://github.com/p4lang/p4c/blob/master/frontends/p4/typeChecking/typeChecker.cpp#L2642

Regarding asics, even I was pessimistic about supporting a run-time index. If asics can't support this feature, why bother changing p4c and bmv2 backend? After all switch ML has to run on an asic.

hesingh · 2019-12-16T18:23:55Z

What I call as "Stack" is Type_Stack in p4c code. Type_Stack represents a header stack.

antoninbas · 2019-12-16T18:24:25Z

A correction to Andy's previous comment: bmv2 already supports dynamic indices for header stacks. This is done using the dereference_header_stack binary operand in the bmv2 JSON (see the doc), which takes as parameters the name of the header stack and an expression evaluating to a valid index for that header tack.

jafingerhut · 2019-12-16T18:34:15Z

@hesingh I do not know what the code in the typeChecker.cpp source file that you link to actually does. I do know that I wrote, checked in, and every commit to p4c code tests, the test P4_16 program linked below, which has an STF test to send packets through bmv2 simple_switch, that uses push_front and pop_front operations on header stacks, invoked from a control, not a parser.

https://github.com/p4lang/p4c/blob/master/testdata/p4_16_samples/header-stack-ops-bmv2.p4
https://github.com/p4lang/p4c/blob/master/testdata/p4_16_samples/header-stack-ops-bmv2.stf

jafingerhut · 2019-12-16T18:36:12Z

Thanks for the correction, Antonin. Either I never saw that before, or had forgotten about it.

jafingerhut · 2019-12-16T18:54:14Z

Regarding this comment: "Regarding asics, even I was pessimistic about supporting a run-time index. If asics can't support this feature, why bother changing p4c and bmv2 backend? After all switch ML has to run on an asic."

I was not saying that I know ASICs cannot do it. I was trying to say that getting public answers about existing ASICs that they can/cannot do it seems unlikely. Not impossible, just unlikely.

Also, a potential reason to implement it in the open source tools is that those targets that can do it, can take advantage of the implementation in p4c. If you take the long view, it might encourage future ASICs to support it that might not otherwise do so.

hesingh · 2019-12-16T18:58:31Z

@hesingh I do not know what the code in the typeChecker.cpp source file that you link to actually does. I do know that I wrote, checked in, and every commit to p4c code tests, the test P4_16 program linked below, which has an STF test to send packets through bmv2 simple_switch, that uses push_front and pop_front operations on header stacks, invoked from a control, not a parser.

https://github.com/p4lang/p4c/blob/master/testdata/p4_16_samples/header-stack-ops-bmv2.p4
https://github.com/p4lang/p4c/blob/master/testdata/p4_16_samples/header-stack-ops-bmv2.stf

Sorry, my bad. This is the code I pointed to:

if (parser != nullptr)
                typeError("%1%: '%2%' and '%3%' for stacks cannot be used in a parser",
                          expression, IR::Type_Stack::push_front, IR::Type_Stack::pop_front);

push and pop are NOT allowed in parser. So, indeed, you are correct push and pop are allowed inside a control.

hesingh · 2019-12-16T19:01:15Z

Regarding this comment: "Regarding asics, even I was pessimistic about supporting a run-time index. If asics can't support this feature, why bother changing p4c and bmv2 backend? After all switch ML has to run on an asic."

I was not saying that I know ASICs cannot do it. I was trying to say that getting public answers about existing ASICs that they can/cannot do it seems unlikely. Not impossible, just unlikely.

Also, a potential reason to implement it in the open source tools is that those targets that can do it, can take advantage of the implementation in p4c. If you take the long view, it might encourage future ASICs to support it that might not otherwise do so.

Ok, then, let's investigate the feature for a p4c and bmv2 implementation - thanks. Thanks also to @antoninbas for the reference to bmv2 code.

jafingerhut · 2019-12-16T20:35:30Z

@hesingh Antonin's comment was that bmv2 simple_switch has a way to do this, if p4c could only generate the correct BMv2 JSON file.

Your comment shows that p4c does not generate the correct BMv2 JSON file. No argument there.

hesingh · 2019-12-16T21:11:50Z

@hesingh Antonin's comment was that bmv2 simple_switch has a way to do this, if p4c could only generate the correct BMv2 JSON file.

Your comment shows that p4c does not generate the correct BMv2 JSON file. No argument there.

I see, thanks, @jafingerhut. Then, let's see if p4c can fix the issue.

hesingh · 2019-12-16T21:18:20Z

Do note, simple_switch injests JSON output from p4c. This is the reason it makes sense to fix p4c-bm2-ss for this issue.

hesingh · 2019-12-20T20:24:41Z

WIth latest p4c from today, I see the left-hand side of a statement causing an error which was not seen by p4c few days back. Few days back, the right-hand side of a statement caused error with header stack index not being a constant.

hemant@ubuntu:~/p4c/build$ ./p4c-bm2-ss --std p4-16 ~/p4-ipv6-switch-ml/ipv6-switch-ml-bmv2.p4 
/home/hemant/p4-ipv6-switch-ml/ipv6-switch-ml-bmv2.p4(106): [--Wwarn=uninitialized_use] warning: [].val may be uninitialized
         pool[hdr.ml.idx].val = pool[hdr.ml.idx].val + hdr.vector[0].e;
                                ^^^^^^^^^^^^^^^^^^^^
/home/hemant/p4-ipv6-switch-ml/ipv6-switch-ml-bmv2.p4(107): [--Wwarn=uninitialized_use] warning: [].cnt may be uninitialized
         count[hdr.ml.idx].cnt = count[hdr.ml.idx].cnt + 1;
                                 ^^^^^^^^^^^^^^^^^^^^^
In file: /home/hemant/p4c/backends/bmv2/common/expression.cpp:97
Compiler Bug: /home/hemant/p4-ipv6-switch-ml/ipv6-switch-ml-bmv2.p4(109): hdr.vector[0];: could not convert to Json
             hdr.vector[0].e = pool[hdr.ml.idx].val;
             ^^^^^^^^^^^^^

hemant@ubuntu:~/p4c/build$

The error is from this line of code in p4c.

https://github.com/p4lang/p4c/blob/master/backends/bmv2/common/expression.cpp#L97

hesingh · 2019-12-20T20:33:20Z

The P4 code used can be found at https://github.com/IETF-Hackathon/p4-ipv6-switch-ml

jafingerhut · 2019-12-20T20:53:03Z

I know the change in behavior you are seeing is related to this issue, but it might be easier to focus on the change in behavior if you create a separate issue from this one, since the fix for each is likely different.

mihaibudiu · 2019-12-20T21:03:01Z

I cannot reproduce this issue with the master branch of p4c.

ChrisDodd · 2019-12-20T21:04:44Z

When I compile the linked code with latest p4c-bmv2-ss, I just get the "array indexes must be constant" errors:

ipv6-switch-ml-bmv2.p4(106): [--Wwarn=uninitialized_use] warning: [].val may be uninitialized
         pool[hdr.ml.idx].val = pool[hdr.ml.idx].val + hdr.vector[0].e;
                                ^^^^^^^^^^^^^^^^^^^^
ipv6-switch-ml-bmv2.p4(107): [--Wwarn=uninitialized_use] warning: [].cnt may be uninitialized
         count[hdr.ml.idx].cnt = count[hdr.ml.idx].cnt + 1;
                                 ^^^^^^^^^^^^^^^^^^^^^
ipv6-switch-ml-bmv2.p4(109): [--Werror=invalid] error: hdr.ml.idx: Invalid all array indexes must be constant
             hdr.vector[0].e = pool[hdr.ml.idx].val;
                                    ^^^^^^^^^^
ipv6-switch-ml-bmv2.p4(110): [--Werror=invalid] error: hdr.ml.idx: Invalid all array indexes must be constant
             pool[hdr.ml.idx] = {0};
                  ^^^^^^^^^^
ipv6-switch-ml-bmv2.p4(111): [--Werror=invalid] error: hdr.ml.idx: Invalid all array indexes must be constant
             count[hdr.ml.idx] = {0};
                   ^^^^^^^^^^
ipv6-switch-ml-bmv2.p4(106): [--Werror=invalid] error: hdr.ml.idx: Invalid all array indexes must be constant
         pool[hdr.ml.idx].val = pool[hdr.ml.idx].val + hdr.vector[0].e;
              ^^^^^^^^^^
ipv6-switch-ml-bmv2.p4(106): [--Werror=invalid] error: hdr.ml.idx: Invalid all array indexes must be constant
         pool[hdr.ml.idx].val = pool[hdr.ml.idx].val + hdr.vector[0].e;
                                     ^^^^^^^^^^
ipv6-switch-ml-bmv2.p4(107): [--Werror=invalid] error: hdr.ml.idx: Invalid all array indexes must be constant
         count[hdr.ml.idx].cnt = count[hdr.ml.idx].cnt + 1;
               ^^^^^^^^^^
ipv6-switch-ml-bmv2.p4(107): [--Werror=invalid] error: hdr.ml.idx: Invalid all array indexes must be constant
         count[hdr.ml.idx].cnt = count[hdr.ml.idx].cnt + 1;
                                       ^^^^^^^^^^

hesingh · 2019-12-20T21:05:51Z

Sorry, I had changed p4c/backends/bm2/common.cpp in my build and that caused the different error.
Once I backed out the change, p4c is just throws the "Invalid all array indexes must be constant" error.

hesingh · 2019-12-20T21:53:44Z

In the behavioral-model, which supports header stack with array index at run-time, the following op is defined in code:

https://github.com/p4lang/behavioral-model/blob/7192f7550b20859e9f5f1fb8759f2d07e912b6ae/include/bm/bm_sim/expressions.h#L52

For p4c to support dereference_header_stack, should we define a new Operation_deref_hdr_stack in p4c/ir/expression.def?

ChrisDodd · 2019-12-20T22:00:23Z

Why define a new operation, instead of just using the existing IR::ArrayIndex? What is the gain? While you could do so, it seems that the effect is the same as ArrrayIndex, so you'd need to duplicate all code for handling ArrayIndex to also deal with the new node (at least if you need to run the pass in question after introducing the new node type)

ChrisDodd · 2019-12-20T22:03:44Z

IMO the obvious way of handling this is in ExpressionConverter::postorder(const IR::ArrayIndex* expression), if the right operand is not a constant, you create this DEREFERENCE_HEADER_STACK json node instead of giving an error.

hesingh · 2019-12-20T22:04:29Z

Thanks, @ChrisDodd

hesingh · 2019-12-20T22:59:43Z

@antoninbas or @ChrisDodd For this P4 statement:

pool[hdr.ml.idx].val = pool[hdr.ml.idx].val + hdr.vector[0].e;

should the ArrayIndex in JSON for right-hand side be

"value" : ["pool_0[hdr.ml.idx]", "val"]

or

"value" : ["pool_0[DEREFERENCE_STACK]", "val"]

or anything else so that the JSON can interoperate with the behavioral-model.

hesingh · 2019-12-20T23:44:22Z

I do not see any P4 code in the behavioral-model which is using non-constant array index. This is the issue with trying to figure out what to write for JSON object for dereference_stack in p4c.
Looking at the behavioral-model test code, I see DEREFERENCE_HEADER_STACK being used below.

https://github.com/p4lang/behavioral-model/blob/fff3171f2a7e3fcbb2ae8dec9aa425e282025b10/tests/test_conditionals.cpp#L680

However, I am not sure what name and index are.

This is what I have changed for the bmv2 backend so far merely to test but it's not final code.

diff --git a/backends/bmv2/common/expression.cpp b/backends/bmv2/common/expression.cpp
index 5246aeef..48f2a558 100644
--- a/backends/bmv2/common/expression.cpp
+++ b/backends/bmv2/common/expression.cpp
@@ -204,7 +204,9 @@ void ExpressionConverter::postorder(const IR::ArrayIndex* expression)  {
     }
 
     if (!expression->right->is<IR::Constant>()) {
-        ::error(ErrorType::ERR_INVALID, "all array indexes must be constant", expression->right);
+        // create a DEREFERENCE_HEADER_STACK json node
+        auto str = expression->right->toString();
+        elementAccess += "[" + str + "]";
     } else {
         int index = expression->right->to<IR::Constant>()->asInt();
         elementAccess += "[" + Util::toString(index) + "]";

ChrisDodd · 2019-12-20T23:56:42Z

I would imagine it would be more like:

  "value" : [ {
    "type" : "expression",
    "value" : {
      "op" : "DEREFERENCE_STACK",
      "left" : { "type" : "stack", "value" : "pool_0" },
      "right" : { "type" : "field", "value" : [ "hdr", "ml", "idx" ] } } },
    "val" ]

but I'm not familiar with the exact syntax supported by bmv2 in these json expressions.

antoninbas · 2019-12-21T00:06:52Z

I think the documentation is quite clear about how it works. It is not different from other binary operands. And the proof is that @ChrisDodd was able to figure it out pretty fast without being familiar with the JSON format :)

hesingh · 2019-12-21T19:44:09Z

I changed the p4c bmv2 backend. This is the line of P4 code I am generating JSON for:

hdr.vector[0].e = pool[hdr.ml.idx].val;

But the JSON causes Compiler Bug: 0x55b016900f60: Unexpected json/. The error is not too helpful to a new user working with p4c's JSON.

{
  "value" : {
    "value" : {
      "op" : "DEREFERENCE_STACK",
      "left" : {
        "type" : "stack",
        "value" : "pool_0"
      },
      "right" : {
        "type" : "field",
        "value" : ["hdr", "ml", "idx"]
      }
    },
    "type" : "expression"
  }
}
In file: /home/hemant/p4c/backends/bmv2/common/expression.cpp:447
Compiler Bug: 0x55c254de2cc0: Unexpected json

jafingerhut · 2019-12-21T21:25:37Z

I am pretty sure that modifying p4c is not a "new user experience" kind of thing. Power users only, who can do lots of debugging on their own :-)

That said, I do not know whether anyone is willing to look at and help debug your changes, but if you publish them, it is possible, whereas it is impossible if you do not.

hesingh · 2019-12-21T22:07:19Z

Actually, writing the p4c code to generate new JSON is pretty trivial for me.

I am saying I did what the JSON was generally asked to be generated but there is still something missing with JSON.

jafingerhut · 2019-12-21T23:57:05Z

The error message is coming from p4c code, yes? Not from simple_switch?

hesingh · 2019-12-21T23:59:47Z

The error message is coming from p4c code, yes? Not from simple_switch?

Yes, it's from p4c because I am developing p4c code to support JSON output for run-time array index.

hesingh · 2019-12-22T00:03:26Z

I am getting there. In the p4c bmv2 backend code the "Unexpected json" error is from an "else" block because now the ArrayIndex is now a JSON object. I added code to deal with this case and write more code.

hesingh · 2019-12-23T18:32:25Z

A correction to Andy's previous comment: bmv2 already supports dynamic indices for header stacks. This is done using the dereference_header_stack binary operand in the bmv2 JSON (see the doc), which takes as parameters the name of the header stack and an expression evaluating to a valid index for that header stack.

Now that I changed p4c bmv2 backend to support dynamic indices, the expression is a JsonObject. If you see this behavioral-model code,

https://github.com/p4lang/behavioral-model/blob/7192f7550b20859e9f5f1fb8759f2d07e912b6ae/src/bm_sim/P4Objects.cpp#L476

the code is incomplete to handle a expression if the expression type is objectValue. asString() fails on this line of code:

https://github.com/p4lang/behavioral-model/blob/7192f7550b20859e9f5f1fb8759f2d07e912b6ae/third_party/jsoncpp/src/jsoncpp.cpp#L3111

with type_ == 7 which is objectValue. So, even this json code is incomplete.

jafingerhut · 2019-12-23T19:16:01Z

If you believe you have a BMv2 JSON file that contains what you've been told is correct for variable array indices, and it is leading to some kind of misbehavior in simple_switch, I would recommend creating a separate issue for the behavioral-model repo, attach the BMv2 JSON file you think should work, and details about what is going wrong.

hesingh · 2019-12-23T19:27:14Z

I do think the JSON file from new p4c code with expression as a JsonObject is correct and the beharioral-model should support the expression being a JsonObject. I have filed this issue.

p4lang/behavioral-model#835

hesingh closed this as completed Jul 20, 2019

hesingh reopened this Jul 21, 2019

hesingh closed this as completed Jul 22, 2019

hesingh reopened this Dec 16, 2019

mihaibudiu added the enhancement This topic discusses an improvement to existing compiler code. label Dec 16, 2019

hesingh mentioned this issue Dec 22, 2019

Change p4c bmv2 backend to support runtime array index #2118

Closed

jafingerhut mentioned this issue Jan 16, 2020

Assignment to out-of-range element of header stack array p4lang/p4-spec#805

Closed

hesingh closed this as completed Jan 28, 2020

bmv2 "array index must be a constant" error #2007

bmv2 "array index must be a constant" error #2007

Comments

hesingh commented Jul 20, 2019

jafingerhut commented Jul 20, 2019

hesingh commented Jul 20, 2019

hesingh commented Jul 21, 2019

hesingh commented Jul 21, 2019

jafingerhut commented Jul 21, 2019

hesingh commented Jul 21, 2019

jafingerhut commented Jul 21, 2019

hesingh commented Jul 21, 2019

jafingerhut commented Jul 21, 2019

hesingh commented Jul 21, 2019 • edited Loading

jnfoster commented Jul 21, 2019

hesingh commented Jul 21, 2019

hesingh commented Dec 16, 2019

mihaibudiu commented Dec 16, 2019

hesingh commented Dec 16, 2019

jafingerhut commented Dec 16, 2019

hesingh commented Dec 16, 2019

jafingerhut commented Dec 16, 2019

hesingh commented Dec 16, 2019

hesingh commented Dec 16, 2019

antoninbas commented Dec 16, 2019

jafingerhut commented Dec 16, 2019

jafingerhut commented Dec 16, 2019

jafingerhut commented Dec 16, 2019

hesingh commented Dec 16, 2019

hesingh commented Dec 16, 2019

jafingerhut commented Dec 16, 2019

hesingh commented Dec 16, 2019

hesingh commented Dec 16, 2019

hesingh commented Dec 20, 2019

hesingh commented Dec 20, 2019

jafingerhut commented Dec 20, 2019

mihaibudiu commented Dec 20, 2019

ChrisDodd commented Dec 20, 2019

hesingh commented Dec 20, 2019 • edited Loading

hesingh commented Dec 20, 2019

ChrisDodd commented Dec 20, 2019

ChrisDodd commented Dec 20, 2019

hesingh commented Dec 20, 2019

hesingh commented Dec 20, 2019

hesingh commented Dec 20, 2019

ChrisDodd commented Dec 20, 2019 • edited Loading

antoninbas commented Dec 21, 2019

hesingh commented Dec 21, 2019

jafingerhut commented Dec 21, 2019

hesingh commented Dec 21, 2019

jafingerhut commented Dec 21, 2019

hesingh commented Dec 21, 2019

hesingh commented Dec 22, 2019

hesingh commented Dec 23, 2019

jafingerhut commented Dec 23, 2019

hesingh commented Dec 23, 2019

hesingh commented Jul 21, 2019 •

edited

Loading

hesingh commented Dec 20, 2019 •

edited

Loading

ChrisDodd commented Dec 20, 2019 •

edited

Loading