[PATCH] Add a thread-model knob for lowering atomics on baremetal & single threaded systems

Jon Roelofs jonathan at codesourcery.com
Wed Aug 20 13:49:09 PDT 2014


================
Comment at: lib/Target/ARM/ARMISelLowering.cpp:757
@@ +756,3 @@
+      TM.Options.ThreadModel == ThreadModel::Single) {
+    if (TM.Options.ThreadModel == ThreadModel::Single) {
+      setOperationAction(ISD::ATOMIC_FENCE,   MVT::Other, Expand);
----------------
Robin Morisset wrote:
> This repetition of the test for ThreadModel::Single looks unnecessary to me.
> 
> Why not test for it first, then put the rest in an else block ?
> 
> 
> ```
> if (TM.Options>ThreadModel == ThreadModel::Single) {
>   setOperationAction(ISD::ATOMIC_FENCE,  MVT::Other, Expand);
> else if (Subtarget->hasAnyDataBarrier() && !Subtarget->isThumb1Only()) {
> ...
> ```
> 
> As a bonus, it makes the logic simpler, avoiding setting insertFencesForAtomic (which is a no-op in that case I believe, since all atomic operations will have been lowered before).
That does look cleaner.

================
Comment at: lib/Target/ARM/ARMTargetMachine.cpp:161
@@ -160,3 +160,3 @@
 void ARMPassConfig::addIRPasses() {
   addPass(createAtomicExpandLoadLinkedPass(TM));
 
----------------
Robin Morisset wrote:
> Shouldn't this pass be disabled when the ThreadModel is single? Especially as it runs first, I expect it will turn for example atomicrmw add in a loop of ldrex/strex instead of a simple add instruction (whenever the subtarget does not disable it at least).
Oh, good point!

http://reviews.llvm.org/D4984






More information about the llvm-commits mailing list