-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Sign Extension in SBPFv2 #32924
Comments
is zero extend required by the specification? i see https://www.kernel.org/doc/html/latest/bpf/instruction-set.html#arithmetic-instructions and https://www.kernel.org/doc/Documentation/networking/filter.txt
|
We cannot make a decision on the ground of "likeliness". Either they are ever generated and then must be supported, or never generated and then must NOT be supported because we will not be able to test. |
I am in agreement with Jan here. ISA extensions and changes should be driven by a quantitative approach and where clear need is demonstrated by data, and a cost-benefit analysis (e.g., as is standard in ISA/microarchitecture teams/implementations). Gut feelings and likelihood are not reliable indicators. Overall, given our current state (new runtime work needed, new ABI work needed, Move compiler implementation, and so forth), ISA changes are not a priority. The same goes for all of the recent "proposals" of ISA changes. |
This would change the semantics of ADD32, SUB32, NEG32, MUL32, LMUL32, SDIV32, SREM32 instructions. In addition the compiler would have to generate the explicit sing extension instruction every time it emits one of the ALU32 instructions to preserve the semantics of the operations. What would we gain from this change? |
Not following every arithmetic instruction. Only when one casts from |
Problem
Currently in the RBPF VM some ALU32 instructions do sign extend their 32 bit results (outputs) to 64 bit.
These operations sign extend their results from 32 bit to 64 bit:
ADD32, SUB32, NEG32, MUL32, LMUL32, SDIV32, SREM32
These operations do not:
DIV32, MOD32, XOR32, OR32, AND32, LSH32, RSH32, ARSH32, MOV32, UDIV32, UREM32
It is likely that most programs do not ever use the upper 32 bit of a
u32
ori32
and thus this sign extension work is wasted and could be omitted.Proposed Solution
Add an explicit sign extension instruction and remove implicit sign extension of results, see:
https://github.com/solana-foundation/solana-improvement-documents/pull/87/files
The text was updated successfully, but these errors were encountered: