Implement packet_in.extract logic for hs.next.

[vaalfreja]

Fixes #273. Adds required verify operation an nextIndex field increase in case of extract being used with header_stack.next
Signed-off-by: Julia Koval c_yuliak@xsightlabs.com

[asl]

Value is nullable. No need for optional here

[mtsamis]

Looking at detectNextPattern, this is based on matching a very particular IR structure to do the transformation. Some issues with that:

1. There is a number of assumptions that may not generally hold here: No checking for the blocks that various operations reside, and if they're in different blocks this won't work (although we don't expect them to be), and also as far as I know getUsers() has no guaranteed order, so picking the first user may be problematic.
2. Given that we have p4hir-copyin-copyout-elimination pass, matching the particular IR means that this will only work if that pass hasn't been run. If we're to have a pass ordering requirement anyway, why not require the inverse, since copyin-copyout-elimination will greatly simplify what we'd want to match.

I looked a bit further and noticed that the IR that we transform is already partly processed (nextIndex is not present in source code, so it's already synthesized by the compiler). The translator does that thing (check resolveReference, there's also // TODO: Insert verify() call comment).

Can you investigate the implementation again? Can we adjust the strategy (possibly including modifications in the translator) so that we are not depending on very particular IR being matched for this to work?

[vaalfreja]

I looked a bit further and noticed that the IR that we transform is already partly processed (nextIndex is not present in source code, so it's already synthesized by the compiler). The translator does that thing (check resolveReference, there's also // TODO: Insert verify() call comment).

Can you investigate the implementation again? Can we adjust the strategy (possibly including modifications in the translator) so that we are not depending on very particular IR being matched for this to work?

Last time we discussed it with @asl, it was that we can't modify the frontend, because other things relies on it, so we need to rely on pattern match, but I agree it's better to drag it as some kind of intrinsic if something changed and it's an option now

[mtsamis]

I looked a bit further and noticed that the IR that we transform is already partly processed (nextIndex is not present in source code, so it's already synthesized by the compiler). The translator does that thing (check resolveReference, there's also // TODO: Insert verify() call comment).
Can you investigate the implementation again? Can we adjust the strategy (possibly including modifications in the translator) so that we are not depending on very particular IR being matched for this to work?

Last time we discussed it with @asl, it was that we can't modify the frontend, because other things relies on it, so we need to rely on pattern match, but I agree it's better to drag it as some kind of intrinsic if something changed and it's an option now

Ok, that's understandable, that's why I said possibly.
Regardless of whether we can change the frontend, we'd still need to address the first part of the issue:
The implementation needs to either make sure it matches the exact thing it can transform (so you'd have to add additional checks for basic blocks and getUsers) or if deemed acceptable to require copyin-copyout-elimination first, then change / simplify the matching logic based on that.

Let's see what @asl thinks about copyin-copyout-elimination here

[vaalfreja]

The implementation needs to either make sure it matches the exact thing it can transform (so you'd have to add additional checks for basic blocks and getUsers) or if deemed acceptable to require copyin-copyout-elimination first, then change / simplify the matching logic based on that.

Alternatively I can propose to separate it to some kind of pattern matching pass and run it(I don't know if we plan to have more things like this) just after the translator like I do in a test. All other operations can be safely optimized around it afterwards. Maybe it should be P4HIR.Extract in this case though.