Hi Paul,

Thanks a bunch for the review.

On Tue, Sep 18, 2018 at 10:25 PM Paul Burton [off-list ref] wrote:

Should this be .set reorder?

Nice catch. Fixed here:
https://git.zx2c4.com/WireGuard/commit/?id=23d97fc333cf85dd07445a9d21a28cbef47c553c
But then...

Even better - could we not just place the addiu before the bne & drop
the .set noreorder, allowing the assembler to fill the delay slot with
the addiu? Likewise in many other places throughout the patch.

That would be more future proof - particularly if we ever want to adjust
this for use with the nanoMIPS ISA which has no delay slots. It may also
allow the assembler the choice to use compact branches (ie. branches
without visible delay slots) when targeting MIPS32r6. I know neither of
these will currently build this code, but I think avoiding all the
noreorder blocks would be a nice cleanup just for the sake of
readability anyway.

Great idea. Rene has committed that here:
https://git.zx2c4.com/WireGuard/commit/?id=5c153a59ac3aa58a3ff17c69fee63d599e5f2758

These will be in the v6 patchset whenever that's posted, and it's
already been merged into the dev tree:
https://git.kernel.org/pub/scm/linux/kernel/git/zx2c4/linux.git/log/?h=jd/wireguard

Regards,
Jason