[PATCH] Use the load-acquire/store-release instructions optimally in AArch32

[Artyom Skrobov]

Thanks for your feedback Tim

> Together with how allocation works, I think this means that every
> SDNode LLVM allocates will be substantially larger after this change.
> Is that justified?

Not that much larger, but you're right, I should be allocating dynamically.

diff --git a/include/llvm/CodeGen/SelectionDAGNodes.h b/include/llvm/CodeGen/SelectionDAGNodes.h
index 987f290..35499e1 100644
--- a/include/llvm/CodeGen/SelectionDAGNodes.h
+++ b/include/llvm/CodeGen/SelectionDAGNodes.h
@@ -1068,6 +1068,7 @@ public:
 ///
 class AtomicSDNode : public MemSDNode {
   SDUse Ops[4];
+  SDUse* DynOps;

   void InitAtomic(AtomicOrdering Ordering, SynchronizationScope SynchScope) {
     // This must match encodeMemSDNodeFlags() in SelectionDAG.cpp.
@@ -1094,7 +1095,7 @@ public:
                SDValue Chain, SDValue Ptr,
                SDValue Cmp, SDValue Swp, MachineMemOperand *MMO,
                AtomicOrdering Ordering, SynchronizationScope SynchScope)
-    : MemSDNode(Opc, Order, dl, VTL, MemVT, MMO) {
+    : MemSDNode(Opc, Order, dl, VTL, MemVT, MMO), DynOps(NULL) {
     InitAtomic(Ordering, SynchScope);
     InitOperands(Ops, Chain, Ptr, Cmp, Swp);
   }
@@ -1102,7 +1103,7 @@ public:
                SDValue Chain, SDValue Ptr,
                SDValue Val, MachineMemOperand *MMO,
                AtomicOrdering Ordering, SynchronizationScope SynchScope)
-    : MemSDNode(Opc, Order, dl, VTL, MemVT, MMO) {
+    : MemSDNode(Opc, Order, dl, VTL, MemVT, MMO), DynOps(NULL) {
     InitAtomic(Ordering, SynchScope);
     InitOperands(Ops, Chain, Ptr, Val);
   }
@@ -1110,10 +1111,22 @@ public:
                SDValue Chain, SDValue Ptr,
                MachineMemOperand *MMO,
                AtomicOrdering Ordering, SynchronizationScope SynchScope)
-    : MemSDNode(Opc, Order, dl, VTL, MemVT, MMO) {
+    : MemSDNode(Opc, Order, dl, VTL, MemVT, MMO), DynOps(NULL) {
     InitAtomic(Ordering, SynchScope);
     InitOperands(Ops, Chain, Ptr);
   }
+  AtomicSDNode(unsigned Opc, unsigned Order, DebugLoc dl, SDVTList VTL, EVT MemVT,
+               SDValue* AllOps, unsigned NumOps,
+               MachineMemOperand *MMO,
+               AtomicOrdering Ordering, SynchronizationScope SynchScope)
+    : MemSDNode(Opc, Order, dl, VTL, MemVT, MMO) {
+    DynOps = new SDUse[NumOps];
+    InitAtomic(Ordering, SynchScope);
+    InitOperands(DynOps, AllOps, NumOps);
+  }
+  ~AtomicSDNode() {
+    delete[] DynOps;
+  }

   const SDValue &getBasePtr() const { return getOperand(1); }
   const SDValue &getVal() const { return getOperand(2); }
@@ -1840,7 +1853,7 @@ template <> struct GraphTraits<SDNode*> {

 /// LargestSDNode - The largest SDNode class.
 ///
-typedef LoadSDNode LargestSDNode;
+typedef AtomicSDNode LargestSDNode;

 /// MostAlignedSDNode - The SDNode class with the greatest alignment
 /// requirement.
====================


> Why are you only marking the 64-bit operations with mayLoad and mayStore?

This is the way it used to be originally: the ATOMxxx6432 pseudo-insts were marked as mayLoad = 1, mayStore = 1, while none of the ATOMIC_xxx pseudo-insts were.

I don't know why it was like this, but if I remove "mayLoad = 1, mayStore = 1" from the 64-bit ops, everything continues to work, so probably, they should not be there at all.

> +++ b/test/CodeGen/ARM/atomic-ops.ll
> Might be an idea to put v8 in the filename so people know what's being tested.

Yes, I agree.


-- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium.  Thank you.

ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No:  2557590
ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No:  2548782