[PATCH] D20098: [Sparc][LEON] Add LEON-specific CASA instruction.

[James Y Knight via llvm-commits]

jyknight added inline comments.

================
Comment at: lib/Target/Sparc/LeonFeatures.td:28
@@ +27,3 @@
+//===----------------------------------------------------------------------===//
+// CASA Support differs between LEON3-FT GR712RC and LEON3-FT UT699
+// We need to have the option to switch this on and off.
----------------
Comment unnecessary.

================
Comment at: lib/Target/Sparc/LeonFeatures.td:34
@@ +33,3 @@
+def CASASupport : SubtargetFeature<
+  "casasupport", 
+  "CasaEnabled", 
----------------
"Support" in the name is weird. How about calling it leoncas.

================
Comment at: lib/Target/Sparc/LeonFeatures.td:35
@@ +34,3 @@
+  "casasupport", 
+  "CasaEnabled", 
+  "true", 
----------------
HasLeonCAS

================
Comment at: lib/Target/Sparc/Sparc.td:119
@@ -118,4 +118,3 @@
 
-// LEON 3 FT (UT699)
-// TO DO: Place-holder: Processor specific features will be added *very* soon here.
+// LEON 3 FT (UT699). Provides full coverage of UT699 - covers all the erratum fixes for LEON3. No support for CASA
 def : Processor<"ut699", LEON3Itineraries, 
----------------
Not sure what you mean by 'provides full coverage of'

================
Comment at: lib/Target/Sparc/SparcISelLowering.cpp:1620
@@ -1619,1 +1619,3 @@
     setMaxAtomicSizeInBitsSupported(64);
+  else if (Subtarget->isLeon() && Subtarget->casaEnabled())
+    setMaxAtomicSizeInBitsSupported(32);
----------------
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.

================
Comment at: lib/Target/Sparc/SparcISelLowering.cpp:1625-1628
@@ -1622,6 +1624,6 @@
 
   setOperationAction(ISD::ATOMIC_SWAP, MVT::i32, Legal);
-  setOperationAction(ISD::ATOMIC_CMP_SWAP, MVT::i32,
-                     (Subtarget->isV9() ? Legal: Expand));
-
+  setOperationAction(ISD::ATOMIC_CMP_SWAP, MVT::i32, Expand);
+  if (Subtarget->isV9() || (Subtarget->isLeon() && Subtarget->casaEnabled()))
+    setOperationAction(ISD::ATOMIC_CMP_SWAP, MVT::i32, Legal);
 
----------------
Can actually delete these lines I believe. Legal is the default, and Expand isn't useful here, because it's handled via setMaxAtomicSizeInBitsSupported now.

================
Comment at: lib/Target/Sparc/SparcInstrInfo.td:52
@@ -51,1 +51,3 @@
 
+// CASASupported - This is true when the target processor supports the CASA
+// instruction
----------------
HasLeonCAS


Repository:
  rL LLVM

http://reviews.llvm.org/D20098