somewhere to put map validation (to avoid doing it multiple times) #1809
Birch-san
started this conversation in
Development
Replies: 1 comment
-
Also seems reasonable to me. I'm sure you've already considered it, but the validation blocks need to print an error when |
Beta Was this translation helpful? Give feedback.
0 replies
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
-
for "big strings", we'll need to use the
map_lookup_elem
helper to acquire storage. every time we do this, we need to validate whether we receive a nullptr. I only want to do that validation once (otherwise every block that uses aprintf()
or astr()
will split into a "true" and a "false" branch).the RFC demonstrates the beginnings of this problem in
if_else_printf.ll
.if your program has two
printf()
s:the first
printf()
requires us to callmap_lookup_elem
(we want to acquire an off-stack buffer to store printf args in, prior to dispatching as perf event).we then check whether the returned map is a nullptr. this creates two additional blocks (failure block and merge block). the failure block is relatively detailed because we want to abort the program the same way
exit()
does.the second
printf()
will also emit a helper call, create two additional blocks and a conditional jump.this will lead to unreadable IR, but there are ways to make it substantially nicer.
for starters, both
printf()
s are actually trying to acquire the same map. so if we can hoist this map declaration and validation to the start of the program, then we could have "declare once, access many times".moreover, I believe it will be far easier to read the codegen if there are clearly-labelled blocks for map declaration and map validation failure.
we don't want to be in the middle of an already-complex part of the program, like one of the blocks created by an
if
:and then find that block gets bifurcated into two more blocks by map validation. we definitely benefit from hoisting map declaration and validation to the start of the program.
so, what's stopping me from just hoisting it like CreateAllocaBPF does?
one problem is that allocas are emitted in reverse order. emitting "just allocas" at the front of the entry block is one thing, but emitting "a mixture of allocas, helper calls and conditional jumps" to the front of the entry block is more consequential. allocas would end up moved into a variety of different blocks, depending on how early CreateAllocaBPF was called.
so this is another reason why I proposed "we shouldn't hoist allocas to the front of the entry block" — rather, hoisted declarations should have their own block, and they should go to the end of it.
if we can do that, the IR blocks can be laid out like so:
map_lookup_elem
calls & validationexit()
)the wider picture is I would want to declare and validate a variety of maps (the map where
printf()
stores its args, eachstr()
map, and maps for anything else which strings can be assigned to, like variables or even string keys). with such a layout, perhaps we can validate all the maps at once with a single conditional statement? eliminating the proliferation of jumps and "validation failure" blocks that we'd have otherwise.Beta Was this translation helpful? Give feedback.
All reactions