Fix issue: annotations get optimized out.#328
Conversation
lib/Dialect/P4HIR/P4HIR_Ops.cpp
Outdated
| void P4HIR::ScopeOp::getEffects( | ||
| SmallVectorImpl<SideEffects::EffectInstance<MemoryEffects::Effect>> &effects) { | ||
| if (getAnnotations()) { | ||
| effects.emplace_back(MemoryEffects::Write::get(), ExternResource::get()); |
There was a problem hiding this comment.
I am not quite sure this is semantically correct... After all, there no extern calls there. Also, you'd definitely want to reason about some annotations, e.g. the builtin ones: https://p4.org/wp-content/uploads/sites/53/p4-spec/docs/p4-16-working-draft.html#sec-branch-annotations
There was a problem hiding this comment.
We can probably remove @ atomic empty block, since nothing is atomic.
Branch annotations are for IfOps not ScopeOp
@ NoWarn should stay - default behavior
Other are not for scopes afaik
There was a problem hiding this comment.
IfOp annotations could become ScopeOp ones in case of constant true / false condition.
There was a problem hiding this comment.
It doesn't become scopeOp it just becomes basic block after flattening, we don't preserve annotations when it happen though and we probably should. Unlikely probably not, if it isn't a branch unlikely doesn't make sense but custom annotations probably should be saved..
There was a problem hiding this comment.
I am not talking about flattening. I'm talking this kind of transformation:
if (true) {
...
}
should become
{
...
}
with annotations preserved.
There was a problem hiding this comment.
We didn't canonicalize that, but now we do, I've moved likely/unlikely logic there also because it doesn't make sense to have a scope with a branch hint - it's not a branch.
Signed-off-by: Julia Koval <c_yuliak@xsightlabs.com>
2 participants
Fix the issue when annotations gets optimized out from empty blocks when canonicalizer or any recursive pass is run.