On Tue, 11 Aug 2026 07:00:46 GMT, Shawn Emery <[email protected]> wrote:
>> Can you explain what is happening here? I wonder if this may not be a >> problem with the benchmark, but with the `conditionalSet()` code itself. >> >> You talk about "cross-call" optimization, but what is really happening? If >> C2 really can see that `conditionalSet(0)` doesn't need to be executed, or >> that repeated invocations of `conditionalSet` don't do anything, then that >> is a problem with the crypto code. If it is the latter, then it needs to be >> fixed in the crypto code, for security reasons. > > The existing arbitrary `conditionalSet()` benchmark is not necessarily > indicative of susceptibility to a side-channel attack in production code due > to a number reasons, including the use of non-constant input arguments and > the dependencies of intermediate operations due to adding between successive > `lookup()` calls when calculating point multiplication, as an example. As a > result, the problem being addressed is a benchmark issue, as the current > conditional set benchmark is nothing like what is utilized in EC production > operations. This is why I lifted the existing `lookup()` logic as a form of > measuring conditional set performance, which better emulates a production > state. > > The above code uses flag arguments that are incorrect which would generate > invalid bit masks, subsequently corrupting the associated limbs during > conditional sets. Even with non-binary flags, because the pair-wise > assignments use the same input, C2 could perform a bitwise OR of the two > masks and use the resulting mask only once for the conditional assignment. > As a result C2 may i) not need to reload registers for the second call, ii) > not need to store the result to memory from the first call when the second > call would overwrite the first, and iii) keep the intermediate limbs in > registers. Correcting the flag arguments to binary would make it even easier > for inlining as it could eliminate the the first call if the second call has > a 1 flag. In aggregate, this likely contributes to the 18% drop in > performance with intrinsics when using the above code, compared to a 52% > performance gain when using intrinsics with the originally proposed solution. > The goal is to try to reproduce the same shape that we see in production EC operations. Yes, I think you are wrong. Callers of this routine (in the crypto code, not this test code) appear always to call it with a non-constant argument derived from input data. I don't think that is a contingent state of affairs but rather a consequence of what the crypto algorithms are doing. So, I don't see any danger of constant folding happening where it might actually matter for security. Perhaps @smemery can confirm whether that is the case (he did indicate that in an earlier comment). I think @merykitty's suggestion is the right one: simply run the test with inlining of `conditionalSet` disallowed. We already do that for testing of other routines used to implement PQC kernels. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/32047#discussion_r3841568789
