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

[Evan Cheng]

I have no real objection to the plan. But can you take advantage of the opportunity to add some comments? Is there any impact on debug info?

Also, what would happen to atomic load > 64-bit? It would cause isel failure? Can we emit a nicer diagnostics?

Evan
  
On Mar 26, 2014, at 7:07 AM, Tim Northover <t.p.northover at gmail.com> wrote:

> 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
> <D3189.1.patch>_______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits