Add support for Register extern to PSA/eBPF backend #3202
Conversation
Co-authored-by: Mateusz Kossakowski <[email protected]> Co-authored-by: Jan Palimąka <[email protected]>
| } else if (method->method->type->name == "read") { | ||
| ::warning(ErrorType::WARN_UNUSED, "This Register(%1%) read value is not used!", name); | ||
| reg->emitRegisterRead(builder, method, this, nullptr); | ||
| } |
There was a problem hiding this comment.
could give an error for other methods
|
|
||
| namespace EBPF { | ||
|
|
||
| EBPFRegisterPSA::EBPFRegisterPSA(const EBPFProgram *program, |
There was a problem hiding this comment.
This stuff could be useful even in the basic ebpf backend, but perhaps it's too late to move it there.
There was a problem hiding this comment.
We can think about moving it to EBPF model, but we would need to agree on the extern definition, which might be different from the one from PSA.
|
|
||
| if (auto wt = dynamic_cast<IHasWidth *>(this->keyType)) { | ||
| unsigned keyWidth = wt->widthInBits(); | ||
| // For keys <= 32 bit register is based on array map, |
There was a problem hiding this comment.
Maybe you should put this into a separate policy function; annotations could be used to generalize this later.
| size = declaredSize->asUnsigned(); | ||
|
|
||
| if (di->arguments->size() == 2) { | ||
| this->initialValue = di->arguments->at(1)->expression->to<IR::Constant>(); |
There was a problem hiding this comment.
Maybe you should document the fact that this can be nullptr.
But if the argument is present and not a constant you will also get a nullptr, which is probably wrong.
| } | ||
|
|
||
| auto declaredSize = di->arguments->at(0)->expression->to<IR::Constant>(); | ||
| if (!declaredSize->fitsUint()) { |
There was a problem hiding this comment.
no CHECK_NULL?
Has this been validated someplace else?
| void EBPFRegisterPSA::emitRegisterWrite(CodeBuilder* builder, const P4::ExternMethod* method, | ||
| ControlBodyTranslatorPSA* translator) { | ||
| auto indexArgExpr = method->expr->arguments->at(0)->expression->to<IR::PathExpression>(); | ||
| cstring indexParamName = translator->getActionParamName(indexArgExpr); |
There was a problem hiding this comment.
is this implying that within an action the index must be an action parameter?
So it cannot be some expression that involves the action parameter?
There was a problem hiding this comment.
the function name is misleading, I'll change it
There was a problem hiding this comment.
I believe that the behavior that you mention is tested in https://github.com/p4lang/p4c/blob/1c179a11f52f2b38c7db5e3fda68e16893ed05b2/backends/ebpf/tests/p4testdata/register-action.p4.
|
The ptf tests are currently failing. |
|
@mbudiu-vmw I've fixed PTF tests. They are passing now. |
No description provided.