[Lldb-commits] [PATCH] D12936: Groundwork for better tracking of renderscript allocations and scripts.

[Greg Clayton via lldb-commits]

clayborg accepted this revision.
clayborg added a comment.
This revision is now accepted and ready to land.

A few suggestions to move things over into RenderScriptRuntime.cpp and to possibly use std::unique_ptr, but they are just suggestions.


================
Comment at: source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.h:258-259
@@ -263,1 +257,4 @@
+
+    typedef std::shared_ptr<ScriptDetails> ScriptDetailsSP;
+    typedef std::shared_ptr<AllocationDetails> AllocationDetailsSP;
 
----------------
Are you actually handing out shared pointers to anyone? If you are just using std::shared_ptr so you can put things into the vector, you can probably just use std::unique_ptr.

================
Comment at: source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.h:298
@@ +297,3 @@
+    // then a new Script detail object will be created for this address and returned.
+    ScriptDetailsSP LookUpScript(lldb::addr_t address, bool create);
+
----------------
If you only need memory management you could say that "m_scripts" owns the objects (if they start using std::unique_ptr) and hand out a raw pointer here. Not sure on your memory management needs though.

================
Comment at: source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.h:304
@@ +303,3 @@
+    // detail object will be created for this address and returned.
+    AllocationDetailsSP LookUpAllocation (lldb::addr_t address, bool create);
+};
----------------
Ditto above comment.

================
Comment at: source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.h:307-417
@@ -295,2 +306,113 @@
+
+// The empirical_type adds a basic level of validation to arbitrary data
+// allowing us to track if data has been discovered and stored or not.
+// An empirical_type will be marked as valid only if it has been explicitly assigned to.
+template <typename type_t>
+class empirical_type
+{
+  public:
+    // Ctor. Contents is invalid when constructed.
+    empirical_type()
+        : valid(false)
+    {}
+
+    // Return true and copy contents to out if valid, else return false.
+    bool get(type_t& out) const
+    {
+        if (valid)
+            out = data;
+        return valid;
+    }
+
+    // Return a pointer to the contents or nullptr if it was not valid.
+    const type_t* get() const
+    {
+        return valid ? &data : nullptr;
+    }
+
+    // Assign data explicitly.
+    void set(const type_t in)
+    {
+        data = in;
+        valid = true;
+    }
+
+    // Mark contents as invalid.
+    void invalidate()
+    {
+        valid = false;
+    }
+
+    // Returns true if this type contains valid data.
+    bool isValid() const
+    {
+        return valid;
+    }
+
+    // Assignment operator.
+    empirical_type<type_t>& operator = (const type_t in)
+    {
+        set(in);
+        return *this;
+    }
+
+    // Dereference operator returns contents. 
+    // Warning: Will assert if not valid so use only when you know data is valid.
+    const type_t& operator * () const
+    {
+        assert(valid);
+        return data;
+    }
+
+  protected:
+    bool valid;
+    type_t data;
+};
+
+// The ScriptDetails class collects data associated with a single script instance.
+struct RenderScriptRuntime::ScriptDetails
+{
+    enum ScriptType
+    {
+        eScript,
+        eScriptC
+    };
+
+    // The derived type of the script.
+    empirical_type<ScriptType> type;
+    // The name of the original source file.
+    empirical_type<std::string> resName;
+    // Path to script .so file on the device.
+    empirical_type<std::string> scriptDyLib;
+    // Directory where kernel objects are cached on device.
+    empirical_type<std::string> cacheDir;
+    // Pointer to the context which owns this script.
+    empirical_type<lldb::addr_t> context;
+    // Pointer to the script object itself.
+    empirical_type<lldb::addr_t> script;
+};
+
+// This AllocationDetails class collects data associated with a single
+// allocation instance.
+struct RenderScriptRuntime::AllocationDetails
+{
+    enum DataType
+    {
+        eInt,
+    };
+
+    enum Dimension
+    {
+        e1d,
+        e2d,
+        e3d,
+        eCubeMap,
+    };
+
+    empirical_type<DataType> type;
+    empirical_type<Dimension> dimension;
+    empirical_type<lldb::addr_t> address;
+    empirical_type<lldb::addr_t> dataPtr;
+    empirical_type<lldb::addr_t> context;
 };
 
----------------
Does anyone really need to see these objects outside of RenderScriptRuntime.cpp? If not, move these into RenderScriptRuntime.cpp.


Repository:
  rL LLVM

http://reviews.llvm.org/D12936