Page MenuHomePhabricator

[Mips] Add support for min/max/umin/umax atomics
ClosedPublic

Authored by mbrkusanin on Dec 4 2019, 10:37 AM.

Details

Summary

In order to properly implement these atomic we need one register more than other
binary atomics. It is used for storing result from comparing values in addition
to the one that is used for actual result of operation.

Diff Detail

Event Timeline

mbrkusanin created this revision.Dec 4 2019, 10:37 AM
mbrkusanin added a comment.EditedDec 4 2019, 10:49 AM

There is a trick we can do to avoid taking an additional register. We can reuse either OldVal or Incr for intermediate results. I know that return value needs to be same as OldVal but I don't know if changing Incr is allowed.

If we reuse Incr it would look something like this for max:

slt BinOpRes, OldVal, Incr
movz Incr, OldVal ,BinOpRes, Incr #overwrite Incr with OldVal if it is higher value
move BinOpRes, Incr

or for R6

slt BinOpRes, OldVal Incr
selnez Incr, Incr, BinOpRes
seleqz BinOpRes, OldVal, BinOpRes
or BinOpRes, BinOpRes, Incr

If we reuse OldVal we can reload it with LL.

mbrkusanin edited the summary of this revision. (Show Details)Dec 4 2019, 10:49 AM
mbrkusanin edited the summary of this revision. (Show Details)Dec 5 2019, 1:58 AM
atanasyan accepted this revision.Dec 5 2019, 3:10 AM

LGTM. Thanks.

I do not think we can modify the Incr.

This revision is now accepted and ready to land.Dec 5 2019, 3:10 AM

Sorry @atanasyan, you already reviewed this, but for Mips64 tests would fail with -verify-machineinstrs option. Apparently both 'SLT' and 'SLT64' use GPR32 for result. It's been corrected now and there should be no issues for EXPENSIVE_CHECKS builds. Can you take a quick look at new changes? Thanks.

mbrkusanin closed this revision.Dec 12 2019, 2:59 AM