On Fri, 21 Aug 2026 11:30:43 GMT, Andrew Haley <[email protected]> wrote:
> > > This is starting to look good. A much-needed cleanup!
> > > One more thing I'd do, while I was at it. There are several instances of
> > > this pattern:
> > > ```
> > > if (!can_use_fp || !can_use_r18) {
> > > __ ldp(tmp3, tmp4, __ post(sp, 16));
> > > }
> > > ```
> > >
> > > which can be replaced by unconditional `push` and `pop`.
> >
> > I'm concerned that the unconditional `push` & `pop` here, in that it could
> > have some performance impact for Linux-Neoverse systems. As this would
> > create artificial spillage whereas none was needed before this change.
>
> Can you show us an example of this?
Example patch (don't apply - not aligned with PR):
@@ -8925,8 +8921,9 @@ class StubGenerator: public StubCodeGenerator {
} else {
tmp3 = a[4];
tmp4 = a[9];
- __ stp(tmp3, tmp4, __ pre(sp, -16));
}
+ __ push(RegSet::of(tmp3, tmp4), sp);
__ eor3(tmp3, a[0], a[5], a[10]);
__ eor3(tmp4, tmp3, a[15], a[20]); // tmp4 = a0^a5^a10^a15^a20 = c0
@@ -8956,9 +8953,11 @@ class StubGenerator: public StubCodeGenerator {
__ eor(a[7], a[7], tmp2);
__ eor(a[12], a[12], tmp2);
__ rax1(tmp0, tmp0, tmp4); // d4
- if (!can_use_fp || !can_use_r18) {
- __ ldp(tmp3, tmp4, __ post(sp, 16));
- }
+ __ pop(RegSet::of(tmp3, tmp4), sp);
+
__ eor(a[17], a[17], tmp2);
__ eor(a[22], a[22], tmp2);
__ eor(a[4], a[4], tmp0);
On Neoverse systems running Linux, both `tmp3=rfp` and `tmp4=r18_tls` are
already available, so the unconditional `push` and `pop` are unnecessary calls
executed 2 * 24 times per pair for Keccak and 2 * 48 times per pair for
double-Keccak.
-------------
PR Comment: https://git.openjdk.org/jdk/pull/32049#issuecomment-5375983876
