Propose design for packed conditional fields

[jasongraffius]

See #191

[reventlov]

Nit: Rerference => Reference

[reventlov]

The lookback stack and some of the condition checking are optimizations that, I think, make this section harder to understand.

I think the new (unoptimized) implementation of $next would just be:

1. For each structure:
   1. Initialize next to 0.
   2. For each physical field:
      1. Replace $next in appropriate contexts with the current value of next.
      2. Update the definition of next to field_condition ? field_start + field_size : (previous_next).

It may even be better to use the unoptimized version in the desugaring stage, then let a later optimization pass strip out the unreachable branches -- I have some related code that I was working on recently that is intended to optimize the $size_in_bytes expression, which has many of the same problems.

[jasongraffius]

That's fair, you're right that is an optimization now that I think about it, and the next value can be propagated forward through non-present fields. I'll update this to make it simpler.

I still think there would be value to doing the optimization in the actual code though, I know that every C++ compiler out there would optimize away chained conditionals with constant values or repeated conditions (with only non-volatile and non-aliased components), but it wouldn't be optimized for something like a potential future Python backend, for instance, which would make accessing the nth field of a struct an O(n) operation, as it would process the conditionals for all n-1 fields before it to determine the start location, even if said location is statically known.

[reventlov]

Yeah, it's probably best for some of the optimization to happen inside Emboss. I'm just thinking it would be better in a separate optimization stage, instead of trying to implement it in the desugaring stage -- separation of concerns, and it would let any similar constructs get optimized.

[jasongraffius]

Ah got it, as in a separate optimization pass that resolves conditionals, then it's more general and applies to other designating like $size_in_bytes too?

You're right I like that idea a lot. I'll consider it out of scope of this proposal since it won't matter for any decent C++ compiler and we don't currently support any other backend, but I see now that we don't need to preemptively optimize in the desugaring step.

[reventlov]

Yeah. The two specific optimizations here are:

• If a CHOICE has a constant condition, replace it with the appropriate branch; i.e. true ? y : z => y and false ? : y : z => z.
• For any expression x ? y : (x ? z : q), replace the inner (x ? z : q) with q. There is a larger possible optimization around propagating constraints into choice operator branches, but this would cover a lot of cases and would be fairly easy to implement.

I'll see about getting the prototype optimizer I hacked together onto GitHub for reference.

[jasongraffius]

Actually now that I took a second to look through it I think the former is already handled by constant_value in ir_utils.

[reventlov]

It's only partially handled by constant_value -- that works for true ? constant : variable and false ? variable : constant, but not for true ? variable : variable2 or false ? variable : variable2, so it would cover:

struct Foo:
  0     [+4]  UInt  x
  $next [+4]  UInt y
  $next [+4]  UInt z

But not:

struct Foo:
  0     [+4]    UInt      x
  $next [+4*x]  UInt:32[] y
  $next [+4]    UInt      z

[reventlov]

I have two major thoughts on this:

1. There probably are not any (correct) uses of $next after a conditional field in the wild (I did not turn up any in an internal audit).
2. In principle, a breaking change should break noisily. In this case, that means turning any use of $next immediately after a conditional field into an error, and either using a new keyword ($pack is fine) or having users opt into the new behavior ([next_keyword = "Pack"]). Compilation errors are annoying, but not nearly as problematic as silently changing behavior.

[reventlov]

#206 has a real-world case of someone getting tripped up by this.

[reventlov]

@jasongraffius have you had any time to work on this? I think the design sketch is very close to being ready to commit, even if the actual implementation will take some time.

[jasongraffius]

Good point, I'll update land the sketch soon