[PATCH] Add a thread-model knob for lowering atomics on baremetal & single threaded systems
Robin Morisset
morisset at google.com
Wed Aug 20 13:19:32 PDT 2014
A few inline comments.
I would also suggest tests for atomic loads/stores/fences and not just atomicRMW.
================
Comment at: lib/Target/ARM/ARMISelLowering.cpp:752
@@ -751,6 +751,3 @@
// ARMv6 Thumb1 (except for CPUs that support dmb / dsb) and earlier use
- // the default expansion.
- if (Subtarget->hasAnyDataBarrier() && !Subtarget->isThumb1Only()) {
- // ATOMIC_FENCE needs custom lowering; the others should have been expanded
- // to ldrex/strex loops already.
- setOperationAction(ISD::ATOMIC_FENCE, MVT::Other, Custom);
+ // the default expansion. If we are targeting a single threded system,
+ // then set them all for expand so we can lower them later into their
----------------
single threded -> single threaded
================
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);
----------------
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).
================
Comment at: lib/Target/ARM/ARMTargetMachine.cpp:161
@@ -160,3 +160,3 @@
void ARMPassConfig::addIRPasses() {
addPass(createAtomicExpandLoadLinkedPass(TM));
----------------
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).
http://reviews.llvm.org/D4984
More information about the llvm-commits
mailing list