[PATCH] Sparc: Implement i64 load/store support for 32-bit sparc.

[James Y Knight]

Thanks a bunch for your notes!


================
Comment at: lib/Target/Sparc/SparcFrameLowering.cpp:232-235
@@ -229,3 +231,6 @@
+      unsigned mapped_preg = preg - SP::I0_I1 + SP::O0_O1;
+      MRI.replaceRegWith(preg, mapped_preg);
+    }
     // Mark the reg unused.
     MRI.setPhysRegUnused(reg);
   }
----------------
t.p.northover wrote:
> Don't you need to mark the pair unused as well? I wouldn't swear to this, but a quick look suggests that marking I0_I1 unused would implicitly mark I0 and I1 unused, but not the converse.
As far as I can tell, I0_I1 (etc) don't ever get used bits set on them, so they don't need to be unmarked either.

================
Comment at: lib/Target/Sparc/SparcFrameLowering.cpp:245
@@ +244,3 @@
+      MBB->removeLiveIn(reg);
+      MBB->addLiveIn(reg - SP::I0 + SP::O0);
+    }
----------------
t.p.northover wrote:
> Is this delta correct? I'd have expected "reg - SP::I0_I1 + SP::O0_O1" unless I'm misunderstanding the logic.
This code is indeed *not* correct. Thanks for catching that!! Any idea how to cover that in a test case?

================
Comment at: lib/Target/Sparc/SparcISelLowering.cpp:2641-2642
@@ +2640,4 @@
+  EVT MemVT = LdNode->getMemoryVT();
+  if (MemVT == MVT::f128)
+    return LowerF128Load(Op, DAG);
+
----------------
t.p.northover wrote:
> I don't understand this: it's the only place a 128-bit type is mentioned at all in the patch. Why does the default action change?
Not quite the only place, see also LowerSTORE, which has a similar piece of code.


Note the call to LowerLOAD from LowerOperation: the code there used to say "return LowerF128Load(Op, DAG);", which simply assumed that the type passed in was *always* MVT::f128 when called with ISD::LOAD. That's no longer true, because MVT::i64 is also marked for Custom handling via setOperationAction, so LowerOperation could be called with i64, now too.

Unlike store, I don't actually need any custom handling for it here (because the custom handling for it occurs in ReplaceNodeResults, instead), but this code does need to not do the f128 stuff for it.

http://reviews.llvm.org/D8713

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/