[PATCH] ARM: I have a cunning plan (atomics)

Tim Northover t.p.northover at gmail.com
Wed Mar 26 07:07:47 PDT 2014


Hi all,

Currently atomic operations work on ARM (& Mips, &PPC), but the code to handle them is a complete nightmare to maintain; it's made up of spaghetti copy-pasta that pokes around at MachineBasicBlocks, together with a healthy dose of .td-level boiler-plate.

This is only going to get worse with various enhancements that are coming up (short-cirtuiting cmpxchg, making use of the failure mode, recognising the common "cmpxchg; did it work?" idiom).

But I think this can be done much more simply as an IR-level pass, where modfiying control-flow isn't a pain in the neck and we can deal with illegal types without even thinking about it. Hence this patch.

Code-wise, it removes ~1000 lines, replacing them with ~350 clearer ones to do the same job.

But there are some caveats: the operations involving 64-bit comparisons (min/max) get rather less efficient. This is simply a reflection of the fact that ARM's 64-bit comparison code is very bad anyway, and I think that's where anyone who cares should look to for a fix.

There are also extra uxtb/uxth operations because the DAGISel isn't clever enough at spotting chains to fold them into an ldrex. This can probably be handled rather simply by a DAGCombine if necessary, though I haven't done so here.

My long-term goal is to move it out of the ARM directory and get (at least) AArch64 using it, though I think the other loopy atomic targets would also benefit. But for now, ARM will do.

So, do the trade-offs look reasonable to everyone?

Cheers.

Tim.

http://llvm-reviews.chandlerc.com/D3189

Files:
  lib/Target/ARM/ARM.h
  lib/Target/ARM/ARMAtomicExpandPass.cpp
  lib/Target/ARM/ARMISelDAGToDAG.cpp
  lib/Target/ARM/ARMISelLowering.cpp
  lib/Target/ARM/ARMISelLowering.h
  lib/Target/ARM/ARMInstrInfo.td
  lib/Target/ARM/ARMTargetMachine.cpp
  lib/Target/ARM/CMakeLists.txt
  test/CodeGen/ARM/atomic-64bit.ll
  test/CodeGen/ARM/atomic-ops-v8.ll
  test/CodeGen/ARM/atomicrmw_minmax.ll
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D3189.1.patch
Type: text/x-patch
Size: 110269 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140326/9dd317a2/attachment.bin>


More information about the llvm-commits mailing list