Add @likely/@unlikely annotations for blocks #4979
Conversation
fruffy
added
the
p4-spec
ChrisDodd
force-pushed
the
cdodd-likely
branch
3 times, most recently
from
d86089f to
d482036
Compare
ChrisDodd
force-pushed
the
cdodd-likely
branch
2 times, most recently
from
46257c9 to
1847985
Compare
There was a problem hiding this comment.
Perhaps some frontend passes (in particular, DoSimplifyControlFlow in frontends/p4/simplify.cpp) could be extended to understand these -- currently, any annotations on a block will inhibit combining the blocks, whereas
@likely/@unlikelyon a block should not inhibit combining it with a block with no annotation (the annotation should be on the resulting combined block).
Sounds reasonable. Is there a risk that the annotation will currently inhibit some optimization? That would be very unfortunate.
However, if we start merging blocks where the annotations don't match exactly, could we end up with both @likely and @unlikely on a single blocks? What should be done then? Imagine code like this:
if (some_runtime_condition) @likely {
if (some_condition_compiler_proves_true) @unlikely {
code;
}
}obviously, this example is a bit pathological as the code claims the branch is unlikely, but the compiler proves it can be taken always.
ChrisDodd
force-pushed
the
cdodd-likely
branch
from
1847985 to
e5e00c1
Compare
ChrisDodd
force-pushed
the
cdodd-likely
branch
6 times, most recently
from
b116b77 to
ecb291c
Compare
|
So it seems that something like is quite plausible, and if the compiler can prove it is always true and remove the |
ChrisDodd
force-pushed
the
cdodd-likely
branch
2 times, most recently
from
05e10e9 to
da01db2
Compare
- fix a few places annotations on blocks are accidentally lost - allow merging blocks with same annotation Signed-off-by: Chris Dodd <cdodd@nvidia.com>
Signed-off-by: Chris Dodd <cdodd@nvidia.com>
- warn if @unlikely is always taken. Signed-off-by: Chris Dodd <cdodd@nvidia.com>
These will generally be used by backends and simply passed through the frontend/midend, so not much is needed here.
Perhaps some frontend passes (in particular, DoSimplifyControlFlow in frontends/p4/simplify.cpp) could be extended to understand these -- currently, any annotations on a block will inhibit combining the blocks, whereas
@likely/@unlikelyon a block should not inhibit combining it with a block with no annotation (the annotation should be on the resulting combined block).See discussion in p4lang/p4-spec#1308