Implement ldrex and strex in ARM simulator

src/arm/simulator-arm.h

Index: src/arm/simulator-arm.h
diff --git a/src/arm/simulator-arm.h b/src/arm/simulator-arm.h
index 71b8e4086245c1a3179b10ab1895925ea40adaf4..7b3e3df9a9516fb94d139ca89bd649fcdc11d3c1 100644
--- a/src/arm/simulator-arm.h
+++ b/src/arm/simulator-arm.h
@@ -14,6 +14,8 @@
 #define V8_ARM_SIMULATOR_ARM_H_
 
 #include "src/allocation.h"
+#include "src/base/lazy-instance.h"
+#include "src/base/platform/mutex.h"
 
 #if !defined(USE_SIMULATOR)
 // Running without a simulator on a native arm platform.
@@ -302,17 +304,17 @@ class Simulator {
   // Read and write memory.
   inline uint8_t ReadBU(int32_t addr);
   inline int8_t ReadB(int32_t addr);
-  inline void WriteB(int32_t addr, uint8_t value);
-  inline void WriteB(int32_t addr, int8_t value);
+  inline void WriteB_Internal(int32_t addr, uint8_t value);
+  inline void WriteB_Internal(int32_t addr, int8_t value);
 
   inline uint16_t ReadHU(int32_t addr, Instruction* instr);
   inline int16_t ReadH(int32_t addr, Instruction* instr);
   // Note: Overloaded on the sign of the value.
-  inline void WriteH(int32_t addr, uint16_t value, Instruction* instr);
-  inline void WriteH(int32_t addr, int16_t value, Instruction* instr);
+  inline void WriteH_Internal(int32_t addr, uint16_t value, Instruction* instr);
+  inline void WriteH_Internal(int32_t addr, int16_t value, Instruction* instr);
 
   inline int ReadW(int32_t addr, Instruction* instr);
-  inline void WriteW(int32_t addr, int value, Instruction* instr);
+  inline void WriteW_Internal(int32_t addr, int value, Instruction* instr);
 
   int32_t* ReadDW(int32_t addr);
   void WriteDW(int32_t addr, int32_t value1, int32_t value2);
@@ -427,6 +429,101 @@ class Simulator {
     char* desc;
   };
   StopCountAndDesc watched_stops_[kNumOfWatchedStops];
+
+  // Syncronization primitives. See ARM DDI 0406C.b, A2.9.
+  enum class MonitorAccess {
+    Open,
+    Exclusive,
+  };
+
+  enum class TransactionSize {
+    None = 0,
+    Byte = 1,
+    HalfWord = 2,
+    Word = 4,
+  };
+
+  // The least-significant bits of the address are ignored. The number of bits
+  // is implementation-defined, between 3 and 11. See ARM DDI 0406C.b, A3.4.3.
+  static const int32_t kTaggedAddrMask = ~7;

[jbramley, 2016/07/20 16:09:24]

Picking (2^)11 would probably be more demanding on software because it means
that exclusive access locations must be spread out if they are to be accessed
simultaneously.

I'd also consider naming it something along the lines of
"kExclusiveTaggedAddrMask". The term "tagged" is somewhat overloaded already so
I think it would be useful to qualify it.

Alternatively, you could replace it with kExclusivesReservationGranule, which
perhaps has the clearest name but requires a bit more work to use since it
should be set to a power of 2.

+
+  // Writes can affect the local and global monitors.
+  void WriteB(int32_t addr, uint8_t value);
+  void WriteB(int32_t addr, int8_t value);

[jbramley, 2016/07/20 16:09:24]

There's no practical difference between storing signed or unsigned bytes or
halfwords. Do you know why these are separated in this way? I realise that the
existing methods were already separated, but the only difference is in an error
message and I don't think it's worth the duplication. Have I missed something?

These could all take just uint{8,16,32}_t types and they'd work fine. "uint*" is
a better default than "int*" because it avoids implementation-defined casting
behaviour, and the function doesn't need to do any arithmetic on it anyway.

[binji, 2016/07/29 21:46:30]

I agree, and I started to make this change, but it seemed like it might be
better as a separate CL.

+  void WriteH(int32_t addr, uint16_t value, Instruction* instr);
+  void WriteH(int32_t addr, int16_t value, Instruction* instr);
+  void WriteW(int32_t addr, int value, Instruction* instr);

[jbramley, 2016/07/20 16:09:24]

I suggest using (u)int32_t explicitly, for consistency if nothing else.

[binji, 2016/07/29 21:46:30]

Yes, but I was trying to have consistency with the current WriteW, which takes
int. I agree it should probably be uint32_t (though all the registers are stored
as int32_t), and bizarrely, this function then casts to intptr_t* to write the
value...? Again, I think this might be better as a separate change.

+
+  // Read and write exclusive. The writes return the strex status: 0 if the
+  // write succeeds, and 1 if the write fails.
+  uint8_t ReadExBU(int32_t addr);
+  uint16_t ReadExHU(int32_t addr, Instruction* instr);
+  int ReadExW(int32_t addr, Instruction* instr);
+  int WriteExB(int32_t addr, uint8_t value);
+  int WriteExH(int32_t addr, uint16_t value, Instruction* instr);
+  int WriteExW(int32_t addr, int value, Instruction* instr);
+
+  class LocalMonitor {
+   public:
+    LocalMonitor();
+
+    // These functions manage the state machine for the local monitor, but do
+    // not actually perform loads and stores.
+    // CheckStoreExcl only returns true if the exclusive store is allowed; the
+    // global monitor will still have to be checked to see whether the memory
+    // should be updated.
+    void LoadExcl(int32_t addr, TransactionSize size);

[jbramley, 2016/07/20 16:09:24]

How about "NotifyLoadExcl" and so on? Then it's clear at the call site that no
load is actually performed.

[binji, 2016/07/29 21:46:30]

Done.

+    void Store(int32_t addr, TransactionSize size);
+    bool CheckStoreExcl(int32_t addr, TransactionSize size);
+
+   private:
+    MonitorAccess access_state_;
+    int32_t tagged_addr_;
+    TransactionSize size_;
+  };
+
+  class GlobalMonitor {
+   public:
+    GlobalMonitor();
+
+    class Node {

[jbramley, 2016/07/20 16:09:24]

Perhaps "Processor" or something similar would be clearer. "Node" is a bit
general, and I don't think it's a term used by the ARM ARM.

[binji, 2016/07/29 21:46:30]

Done.

+     public:
+      Node();
+
+     private:
+      friend class GlobalMonitor;
+      // These functions manage the state machine for the global monitor, but do
+      // not actually perform loads and stores.
+      void LoadExcl_Locked(int32_t addr);
+      void Store_Locked(int32_t addr, bool is_requesting_processor);
+      bool CheckStoreExcl_Locked(int32_t addr, bool is_requesting_processor);
+
+      MonitorAccess access_state_;
+      int32_t tagged_addr_;
+      Node* next_;
+      Node* prev_;
+    };
+
+    // Exposed so it can be accessed by Simulator::{Read,Write}Ex*.
+    base::Mutex mutex;
+
+    void LoadExcl_Locked(int32_t addr, Node* node);
+    void Store_Locked(int32_t addr, Node* node);
+    bool CheckStoreExcl_Locked(int32_t addr, Node* node);
+
+    // Called when the simulator is destroyed.
+    void RemoveNode(Node* node);
+
+   private:
+    bool IsNodeInLinkedList_Locked(Node* node) const;
+    void PrependNode_Locked(Node* node);
+
+    Node* head_;
+  };
+
+  LocalMonitor local_monitor_;
+  GlobalMonitor::Node global_monitor_node_;
+  static base::LazyInstance<GlobalMonitor>::type global_monitor_;
 };