This is an archive of the discontinued LLVM Phabricator instance.

[Sparc][LEON] Add LEON-specific CASA instruction.
ClosedPublic

Authored by lero_chris on May 10 2016, 6:04 AM.

Details

Summary

Adds the CASA instruction for LEON processors.

Diff Detail

Repository
rL LLVM

Event Timeline

lero_chris updated this revision to Diff 56701.May 10 2016, 6:04 AM
lero_chris retitled this revision from to [Sparc][LEON] Add LEON-specific CASA instruction..
lero_chris updated this object.
lero_chris set the repository for this revision to rL LLVM.
lero_chris added a subscriber: llvm-commits.
lero_chris updated this revision to Diff 56709.May 10 2016, 6:48 AM

Combined the two ATOMIC_CMP_SWAP legalisation commands into one.

jyknight added inline comments.May 10 2016, 11:09 AM
lib/Target/Sparc/LeonFeatures.td
28

Comment unnecessary.

34

"Support" in the name is weird. How about calling it leoncas.

35

HasLeonCAS

lib/Target/Sparc/Sparc.td
119–120

Not sure what you mean by 'provides full coverage of'

lib/Target/Sparc/SparcISelLowering.cpp
1620

Should check just one subtarget feature, hasLeonCAS.

But I'd rather not commit these 2 lines that actually turn on support quite yet, as it'll break code using 8 and 16-bit atomics. (Which, of course, are already broken on 64bit sparc, but that's been true all along, and won't be a regression).

I have a patch in progress to support this generically in AtomicExpandPass, but it's not quite ready yet.

1627–1628

Can actually delete these lines I believe. Legal is the default, and Expand isn't useful here, because it's handled via setMaxAtomicSizeInBitsSupported now.

lib/Target/Sparc/SparcInstrInfo.td
52

HasLeonCAS

lero_chris added inline comments.May 10 2016, 12:39 PM
lib/Target/Sparc/Sparc.td
119–120

OK. The code I took this from in my version has several other LEON features, but I admit that "coverage" is perhaps an unusual choice of words, so I'll alter that. Eventually, there will be more features here - hence "full coverage"

lero_chris updated this revision to Diff 56787.May 10 2016, 1:09 PM

Adjustments in line with comments made.

lero_chris updated this revision to Diff 56789.May 10 2016, 1:12 PM

Typos fixed in updated code.

Query ability to get this code checked-into the trunk by commenting-out lines that will regress atomics behaviour (temporarily)

lib/Target/Sparc/SparcISelLowering.cpp
1620

Can I comment-out these two lines, to be added when the AtomicExpandPass is completed? Leaving them like this will make it difficult to check-in other changes that are upcoming, due to changes in the other files in this review (unrelated to atomics work)

To avoid regressions in areas of code being worked on by James Knight, I've removed the section of code that will enable atomics for LEON processors.

When the work on AtomicExpandPass has completed, this section will be re-inserted.

lero_chris accepted this revision.May 16 2016, 1:08 AM
lero_chris added a reviewer: lero_chris.
This revision is now accepted and ready to land.May 16 2016, 1:08 AM
lero_chris closed this revision.May 16 2016, 1:09 AM

Tests are working. CASA instruction is available as inline ASM and a small change will allow full atomics behaviour for LEON processors when the AtomicExpandPass is completed.