Page MenuHomePhabricator

[mips] Implement Octeon+ `saa` and `saad` instructions
ClosedPublic

Authored by atanasyan on Nov 5 2019, 8:00 AM.

Details

Summary

saa and saad are 32-bit and 64-bit store atomic add instructions.

 
memory[base] = memory[base] + rt

These instructions are available for "Octeon+" CPU. The patch adds support for both instructions to MIPS assembler and diassembler and introduces new CPU type - "octeon+".

Next patches will implement .set arch=octeon+ directive and AFL_EXT_OCTEONP ISA extension flag support.

Diff Detail

Event Timeline

atanasyan created this revision.Nov 5 2019, 8:00 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 5 2019, 8:00 AM

Missing test for SaaAddr and SaadAddr.

llvm/lib/Target/Mips/MipsSubtarget.cpp
76

clang format

llvm/test/MC/Mips/macro-saa.s
10

I didn't go through patch in detail, just a quick question here:
as complains that octeon+ requires mips64r2 (missing check in Subtarget constructor?)
but saa instruction itself does not require mips64r2?

Petar.Avramovic accepted this revision.EditedNov 6 2019, 7:53 AM

Correction from previous comment, tests for SaaAddr and SaadAddr are already present.

Remaining question is about flags for tests,
for gnu-as both -mips32 and -mips64 are not compatible with -march=octeon+, minimum requirement is -mips64r2, but feature compatibility could be addressed in another patch.

LGTM.

This revision is now accepted and ready to land.Nov 6 2019, 7:53 AM

Thanks for review.

Correction from previous comment, tests for SaaAddr and SaadAddr are already present.

Anyway I decided to add more tests. In particular, check saa <reg>, <symbol> expansion in case of PIC.

Remaining question is about flags for tests,
for gnu-as both -mips32 and -mips64 are not compatible with -march=octeon+, minimum requirement is -mips64r2, but feature compatibility could be addressed in another patch.

You are right. Now it's possible to pass -mips64 -march=octeon to the clang and do not get any errors. I'm going to solve that as a separate patch for the driver.

This revision was automatically updated to reflect the committed changes.