[PATCH] D18201: Switch over targets to use AtomicExpandPass, and clean up target atomics code.

[Tim Northover via llvm-commits]

t.p.northover added a subscriber: t.p.northover.
t.p.northover added a comment.

> Targets call initSyncLibcalls() to request them where they're supported and required. (This is ARM and MIPS16 at the moment)


What's the benefit here, performance? I can't seem to find where they'd ever be used unset (only getATOMIC/getSYNC references them, and all callees I can find assert that they get a valid libcall).

> Removed unnecessary selection dag expansions of ATOMIC_LOAD into ATOMIC_CMP_SWAP and ATOMIC_STORE into ATOMIC_SWAP. Targets that don't support atomic load/store should now be handled by the translation in AtomicExpandPass into atomic_load/atomic_store, rather than expanding into unnatural operations.


What if the target doesn't have those libcalls? Darwin doesn't, for example, and we're going to have to allow deploying back to older OSs for quite a while even if we added them immediately (and, as you say, they have to be in a .dylib so we can't ship them with the compiler).


================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAG.cpp:1436-1438
@@ -1435,1 +1435,5 @@
 SDValue SelectionDAG::getExternalSymbol(const char *Sym, EVT VT) {
+  if (!Sym)
+    report_fatal_error(
+        "Attempted to use null symbol in SelectionDAG::getExternalSymbol!");
+
----------------
Isn't this an assertion-level error? It doesn't seem to be actionable by the user in any reasonable way.


http://reviews.llvm.org/D18201