From 62cc79bdf0c52c773602d9e93bbf732b1c54b934 Mon Sep 17 00:00:00 2001
From: Jean-Luc Brouillet <jeanluc@google.com>
Date: Sat, 9 Sep 2017 20:45:55 -0700
Subject: [PATCH] Refactor how arguments are tracked.

Use an explicit description of the lifetime of an operand
rather than relying on poolIndex to do the job.

Bug: 63905942
Test: Ran unit tests

Change-Id: I53c4c10b40fd3312232b0689b43d0f72a2818490
---
 neuralnetworks/1.0/types.hal                  | 55 +++++++++++++------
 .../VtsHalNeuralnetworksV1_0TargetTest.cpp    | 16 ++++--
 2 files changed, 49 insertions(+), 22 deletions(-)

diff --git a/neuralnetworks/1.0/types.hal b/neuralnetworks/1.0/types.hal
index 5fe151326f..844c44c14d 100644
--- a/neuralnetworks/1.0/types.hal
+++ b/neuralnetworks/1.0/types.hal
@@ -83,14 +83,20 @@ enum FusedActivationFunc : int32_t {
     RELU6 = 3,
 };
 
-// Two special values that can be used instead of a regular poolIndex.
-enum LocationValues : uint32_t {
-    // The location will be specified at runtime. It's either a temporary
-    // variable, an input, or an output.
-    LOCATION_AT_RUN_TIME = 0xFFFFFFFF,
-    // The operand's value is stored in the
-    // TODO: Only for old
-    LOCATION_SAME_BLOCK = 0xFFFFFFFE
+// How an operand is used.
+enum OperandLifeTime : uint32_t {
+    // The operand is internal to the model.  It's created by an operation
+    // and consumed by other operations.
+    TEMPORARY_VARIABLE,
+    // The operand is an input of the model. An operand can't be both
+    // input and output of a model.
+    MODEL_INPUT,
+    // The operand is an output of the model.
+    MODEL_OUTPUT,
+    // The operand is a constant found in Model.operandValues.
+    CONSTANT_COPY,
+    // The operand is a constant that was specified via a Memory object.
+    CONSTANT_REFERENCE
 };
 
 // Status of a device.
@@ -150,7 +156,20 @@ struct Operand {
     float scale;
     int32_t zeroPoint;
 
+    // How the operand is used.
+    OperandLifeTime lifetime;
+
     // Where to find the data for this operand.
+    // If the lifetime is TEMPORARY_VARIABLE, MODEL_INPUT, or MODEL_OUTPUT:
+    // - All the fields will be 0.
+    // If the lifetime is CONSTANT_COPY:
+    // - location.poolIndex is 0.
+    // - location.offset is the offset in bytes into Model.operandValues.
+    // - location.length is set.
+    // If the lifetime is CONSTANT_REFERENCE:
+    // - location.poolIndex is set.
+    // - location.offset is the offset in bytes into the specified pool.
+    // - location.length is set.
     DataLocation location;
 };
 
@@ -166,12 +185,6 @@ struct Operation {
     vec<uint32_t> outputs;
 };
 
-struct InputOutputInfo {
-    DataLocation location;
-    // If dimensions.size() > 0, we have updated dimensions.
-    vec<uint32_t> dimensions;
-};
-
 struct Model {
     vec<Operand> operands;
     vec<Operation> operations;
@@ -181,9 +194,19 @@ struct Model {
     vec<memory> pools;
 };
 
+struct RequestArgument {
+    // The location within one of the memory pools
+    DataLocation location;
+    // If dimensions.size() > 0, dimension information was provided along with the
+    // argument.  This can be the case for models that accept inputs of varying size.
+    // This can't change the rank, just the value of the dimensions that were
+    // unspecified in the model.
+    vec<uint32_t> dimensions;
+};
+
 struct Request {
-    vec<InputOutputInfo> inputs;
-    vec<InputOutputInfo> outputs;
+    vec<RequestArgument> inputs;
+    vec<RequestArgument> outputs;
     vec<memory> pools;
 };
 
diff --git a/neuralnetworks/1.0/vts/functional/VtsHalNeuralnetworksV1_0TargetTest.cpp b/neuralnetworks/1.0/vts/functional/VtsHalNeuralnetworksV1_0TargetTest.cpp
index bb9f9424ae..90ccd0609d 100644
--- a/neuralnetworks/1.0/vts/functional/VtsHalNeuralnetworksV1_0TargetTest.cpp
+++ b/neuralnetworks/1.0/vts/functional/VtsHalNeuralnetworksV1_0TargetTest.cpp
@@ -103,7 +103,8 @@ Model createTestModel() {
             .numberOfConsumers = 1,
             .scale = 0.0f,
             .zeroPoint = 0,
-            .location = {.poolIndex = static_cast<uint32_t>(LocationValues::LOCATION_AT_RUN_TIME),
+            .lifetime = OperandLifeTime::MODEL_INPUT,
+            .location = {.poolIndex = 0,
                          .offset = 0,
                          .length = 0},
         },
@@ -113,7 +114,8 @@ Model createTestModel() {
             .numberOfConsumers = 1,
             .scale = 0.0f,
             .zeroPoint = 0,
-            .location = {.poolIndex = static_cast<uint32_t>(LocationValues::LOCATION_SAME_BLOCK),
+            .lifetime = OperandLifeTime::CONSTANT_COPY,
+            .location = {.poolIndex = 0,
                          .offset = 0,
                          .length = size},
         },
@@ -123,7 +125,8 @@ Model createTestModel() {
             .numberOfConsumers = 1,
             .scale = 0.0f,
             .zeroPoint = 0,
-            .location = {.poolIndex = static_cast<uint32_t>(LocationValues::LOCATION_SAME_BLOCK),
+            .lifetime = OperandLifeTime::CONSTANT_COPY,
+            .location = {.poolIndex = 0,
                          .offset = size,
                          .length = sizeof(int32_t)},
         },
@@ -133,7 +136,8 @@ Model createTestModel() {
             .numberOfConsumers = 0,
             .scale = 0.0f,
             .zeroPoint = 0,
-            .location = {.poolIndex = static_cast<uint32_t>(LocationValues::LOCATION_AT_RUN_TIME),
+            .lifetime = OperandLifeTime::MODEL_OUTPUT,
+            .location = {.poolIndex = 0,
                          .offset = 0,
                          .length = 0},
         },
@@ -213,10 +217,10 @@ TEST_F(NeuralnetworksHidlTest, SimpleExecuteGraphTest) {
     // prepare inputs
     uint32_t inputSize = static_cast<uint32_t>(inputData.size() * sizeof(float));
     uint32_t outputSize = static_cast<uint32_t>(outputData.size() * sizeof(float));
-    std::vector<InputOutputInfo> inputs = {{
+    std::vector<RequestArgument> inputs = {{
         .location = {.poolIndex = INPUT, .offset = 0, .length = inputSize}, .dimensions = {},
     }};
-    std::vector<InputOutputInfo> outputs = {{
+    std::vector<RequestArgument> outputs = {{
         .location = {.poolIndex = OUTPUT, .offset = 0, .length = outputSize}, .dimensions = {},
     }};
     std::vector<hidl_memory> pools = {allocateSharedMemory(inputSize),