First some high-level remarks:
Why is `finalized` necessary? The `finalize` operation should be
idempotent, i.e. if you call if twice in a row, the second time should
be a no-op. So you can just call finalize on the target array
unconditionally. That would make the code cleaner.
Similarly, the `reswizzle` bit seems a bit redundant. Just check whether
the array was remapped somewhere else -- if it wasn't, there's no need
for swizzling, and if it was, the chances that the components stayed in
the same place are miniscule (and you don't currently check for that
anyway).
Also, why have two separate swizzle arrays? Just keep a single array
(which can actually be an int8_t...) and use it for everything.
I also think that it would be better and less surprising to change how
you think about the lifetime of the objects representing arrays.
Basically, it should work like in a union-find structure: you have a
single constructor which initializes the array to
a) not be mapped anywhere else, and
b) correctly initializes the component use bitmask
Then you have a merge function
merge_arrays(dst, src)
or possibly a method
dst.merge_arrays(src)
(although with the way you're using indices instead of pointers, I'd
prefer a standalone function) that
a) updates the dst array's component use bitmask, and
b) updates the src array's target pointer/index and swizzle
This would also allow you to get rid of the duplicate access masks. In
the end, the only data you really need per-array is:
unsigned target_id;
int8_t swizzle[4];
uint8_t access_mask;
I have some further comments, mostly about style, below.
On 28.04.2018 21:30, Gert Wollny wrote:
Signed-off-by: Gert Wollny <gw.foss...@gmail.com>
---
src/mesa/Makefile.sources | 2 +
src/mesa/meson.build | 2 +
.../state_tracker/st_glsl_to_tgsi_array_merge.cpp | 283 +++++++++++++++++++++
.../state_tracker/st_glsl_to_tgsi_array_merge.h | 116 +++++++++
4 files changed, 403 insertions(+)
create mode 100644 src/mesa/state_tracker/st_glsl_to_tgsi_array_merge.cpp
create mode 100644 src/mesa/state_tracker/st_glsl_to_tgsi_array_merge.h
diff --git a/src/mesa/Makefile.sources b/src/mesa/Makefile.sources
index f910b41d0a..391c62640a 100644
--- a/src/mesa/Makefile.sources
+++ b/src/mesa/Makefile.sources
@@ -519,6 +519,8 @@ STATETRACKER_FILES = \
state_tracker/st_glsl_to_nir.cpp \
state_tracker/st_glsl_to_tgsi.cpp \
state_tracker/st_glsl_to_tgsi.h \
+ state_tracker/st_glsl_to_tgsi_array_merge.cpp \
+ state_tracker/st_glsl_to_tgsi_array_merge.h \
state_tracker/st_glsl_to_tgsi_private.cpp \
state_tracker/st_glsl_to_tgsi_private.h \
state_tracker/st_glsl_to_tgsi_temprename.cpp \
diff --git a/src/mesa/meson.build b/src/mesa/meson.build
index 72d87178aa..df4d727454 100644
--- a/src/mesa/meson.build
+++ b/src/mesa/meson.build
@@ -564,6 +564,8 @@ files_libmesa_gallium = files(
'state_tracker/st_glsl_to_nir.cpp',
'state_tracker/st_glsl_to_tgsi.cpp',
'state_tracker/st_glsl_to_tgsi.h',
+ 'state_tracker/st_glsl_to_tgsi_array_merge.cpp',
+ 'state_tracker/st_glsl_to_tgsi_array_merge.h',
'state_tracker/st_glsl_to_tgsi_private.cpp',
'state_tracker/st_glsl_to_tgsi_private.h',
'state_tracker/st_glsl_to_tgsi_temprename.cpp',
diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi_array_merge.cpp
b/src/mesa/state_tracker/st_glsl_to_tgsi_array_merge.cpp
new file mode 100644
index 0000000000..52d0d81796
--- /dev/null
+++ b/src/mesa/state_tracker/st_glsl_to_tgsi_array_merge.cpp
@@ -0,0 +1,283 @@
+/*
+ * Copyright © 2017 Gert Wollny
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ */
+
+#include "program/prog_instruction.h"
+#include "util/u_math.h"
+#include <ostream>
+#include <cassert>
+#include <algorithm>
+
+#include <iostream>
+
+#include "st_glsl_to_tgsi_array_merge.h"
+
+#if __cplusplus >= 201402L
+#include <memory>
+using std::unique_ptr;
+using std::make_unique;
+#endif
+
+namespace tgsi_array_merge {
+
+array_remapping::array_remapping():
+ target_id(0),
+ reswizzle(false),
+ finalized(true)
+{
+}
+
+array_remapping::array_remapping(int trgt_array_id, unsigned src_access_mask):
+ target_id(trgt_array_id),
+ original_src_access_mask(src_access_mask),
+ reswizzle(false),
+ finalized(false)
+{
+}
+
+array_remapping::array_remapping(int trgt_array_id, int trgt_access_mask,
+ int src_access_mask):
+ target_id(trgt_array_id),
+ summary_access_mask(trgt_access_mask),
+ original_src_access_mask(src_access_mask),
+ reswizzle(true),
+ finalized(false)
+{
+ for (int i = 0; i < 4; ++i) {
+ read_swizzle_map[i] = -1;
+ writemask_map[i] = 0;
+ }
+
+ int src_swizzle_bit = 1;
+ int next_free_swizzle_bit = 1;
+ int k = 0;
+ bool skip = true;
+ unsigned last_src_bit = util_last_bit(src_access_mask);
+
+ for (unsigned i = 0; i < 4; ++i, src_swizzle_bit <<= 1) {
+
+ /* The swizzle mapping fills the unused slots with the last used
+ * component (think temp[A].xyyy) and maps the write mask accordingly.
+ * Hence, if (i < last_src_bit) skip is true and mappings are only addeed
+ * for used the components, but for (i >= last_src_bit) the mapping
+ * is set for remaining slots.
+ */
+ if (skip && !(src_swizzle_bit & src_access_mask))
+ continue;
+ skip = (i < last_src_bit);
+
+ /* Find the next free access slot in the target.*/
+ while ((trgt_access_mask & next_free_swizzle_bit) &&
+ k < 4) {
+ next_free_swizzle_bit <<= 1;
+ ++k;
+ }
+ assert(k < 4 &&
+ "Interleaved array would have more then four components");
+
+ /* Set the mapping for this component. */
+ read_swizzle_map[i] = k;
+ writemask_map[i] = next_free_swizzle_bit;
+ trgt_access_mask |= next_free_swizzle_bit;
+
+ /* Update the joined access mask if we didn't just fill the mapping.*/
+ if (src_swizzle_bit & src_access_mask)
+ summary_access_mask |= next_free_swizzle_bit;
+ }
+}
+
+int array_remapping::map_writemask(int writemask_to_map) const
+{
+ assert(is_valid());
+ if (!reswizzle)
+ return writemask_to_map;
+
+ assert(original_src_access_mask & writemask_to_map);
You probably want `original_src_access_mask & writemask_to_map ==
writemask_to_map` here. Otherwise you're only checking for overlap, not
set inclusion.
+ int result = 0;
+ for (int i = 0; i < 4; ++i) {
+ if (1 << i & writemask_to_map)
+ result |= writemask_map[i];
+ }
+ return result;
+}
+
+uint16_t array_remapping::move_read_swizzles(uint16_t original_swizzle) const
+{
+ assert(is_valid());
+ if (!reswizzle)
+ return original_swizzle;
+
+ /* Since
+ *
+ * dst.zw = src.xy in glsl actually is MOV dst.__zw src.__xy
+ *
+ * when interleaving the arrays the source swizzles must be moved
+ * according to the changed dst write mask.
+ */
+ uint16_t out_swizzle = 0;
+ for (int idx = 0; idx < 4; ++idx) {
+ uint16_t orig_swz = GET_SWZ(original_swizzle, idx);
+ int new_idx = read_swizzle_map[idx];
+ if (new_idx >= 0)
+ out_swizzle |= orig_swz << 3 * new_idx;
+ }
+ return out_swizzle;
+}
+
+int array_remapping::map_one_swizzle(int swizzle_to_map) const
+{
+ if (!reswizzle)
+ return swizzle_to_map;
+
+ assert(read_swizzle_map[swizzle_to_map] >= 0);
+ return read_swizzle_map[swizzle_to_map];
+}
+
+uint16_t array_remapping::map_swizzles(uint16_t old_swizzle) const
+{
+ if (!reswizzle)
+ return old_swizzle;
+
+ uint16_t out_swizzle = 0;
+ for (int idx = 0; idx < 4; ++idx) {
+ uint16_t swz = map_one_swizzle(GET_SWZ(old_swizzle, idx));
+ out_swizzle |= swz << 3 * idx;
+ }
+ return out_swizzle;
+}
+
+void array_remapping::print(std::ostream& os) const
+{
+ static const char xyzw[] = "xyzw";
+ if (is_valid()) {
+ os << "[aid: " << target_id;
+
+ if (reswizzle) {
+ os << " write-swz: ";
+ for (int i = 0; i < 4; ++i) {
+ if (1 << i & original_src_access_mask) {
+ switch (writemask_map[i]) {
+ case 1: os << "x"; break;
+ case 2: os << "y"; break;
+ case 4: os << "z"; break;
+ case 8: os << "w"; break;
+ }
+ } else {
+ os << "_";
+ }
+ }
+ os << ", read-swz: ";
+ for (int i = 0; i < 4; ++i) {
+ if (1 << i & original_src_access_mask && read_swizzle_map[i] >= 0)
+ os << xyzw[read_swizzle_map[i]];
+ else
+ os << "_";
+ }
+ }
+ os << "]";
+ } else {
+ os << "[unused]";
+ }
+}
+
+void array_remapping::finalize_mappings(array_remapping *arr_map)
+{
+ assert(is_valid());
+
+ array_remapping& forward_map = arr_map[target_id];
+
+ /* If no valid map is provided than we have a final target array
+ * at the target_id index, no finalization needed. */
+ if (!forward_map.is_valid())
+ return;
+
+ /* This mappoints to another mapped array that may need finalization. */
+ if (!forward_map.is_finalized())
+ forward_map.finalize_mappings(arr_map);
+
+ /* Now finalize this mapping by translating the map to represent
+ * a mapping to a final target array (i.e. one that is not mapped).
+ * This is only necessary if the target_id array map provides reswizzling.
+ */
+ if (forward_map.reswizzle) {
+
+ /* If this mapping doesn't have a reswizzle map build one now.*/
+ if (!reswizzle) {
+ for (int i = 0; i < 4; ++i) {
+ if (1 << i & original_src_access_mask) {
+ read_swizzle_map[i] = i;
+ writemask_map[i] = 1 << i;
+ } else {
+ read_swizzle_map[i] = -1;
+ writemask_map[i] = 0;
+ }
+ }
+ reswizzle = true;
+ }
+
+ /* Propagate the swizzle mapping of the forward map.*/
+ for (int i = 0; i < 4; ++i) {
+ if ((1 << i & original_src_access_mask) == 0)
+ continue;
+ read_swizzle_map[i] =
forward_map.map_one_swizzle(read_swizzle_map[i]);
+ writemask_map[i] = forward_map.map_writemask(writemask_map[i]);
+ }
+
+ }
+
+ /* Now we can skip the intermediate mapping.*/
+ target_id = forward_map.target_id;
+ finalized = true;
+}
+
+/* Required by the unit tests */
+bool operator == (const array_remapping& lhs, const array_remapping& rhs)
+{
+ if (lhs.target_id != rhs.target_id)
+ return false;
+
+ if (lhs.target_id == 0)
+ return true;
+
+ if (lhs.reswizzle) {
+ if (!rhs.reswizzle)
+ return false;
+
+ if (lhs.original_src_access_mask != rhs.original_src_access_mask)
+ return false;
+
+ for (int i = 0; i < 4; ++i) {
+ if (1 << i & lhs.original_src_access_mask) {
+ if (lhs.writemask_map[i] != rhs.writemask_map[i])
+ return false;
+ if (lhs.read_swizzle_map[i] != rhs.read_swizzle_map[i])
+ return false;
+ }
+ }
+ } else {
+ return !rhs.reswizzle;
+ }
+ return true;
+}
+
+/* end namespace tgsi_array_merge */
+}
diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi_array_merge.h
b/src/mesa/state_tracker/st_glsl_to_tgsi_array_merge.h
new file mode 100644
index 0000000000..c74c854e09
--- /dev/null
+++ b/src/mesa/state_tracker/st_glsl_to_tgsi_array_merge.h
@@ -0,0 +1,116 @@
+/*
+ * Copyright © 2017 Gert Wollny
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ */
+
+#ifndef MESA_GLSL_TO_TGSI_ARRAY_MERGE_H
+#define MESA_GLSL_TO_TGSI_ARRAY_MERGE_H
+
+#include "st_glsl_to_tgsi_private.h"
+#include <iosfwd>
+
+namespace tgsi_array_merge {
+
+/* Helper class to merge and interleave arrays.
+ * The interface is exposed here to make unit tests possible.
+ */
+class array_remapping {
+public:
+
Please avoid empty lines at the beginning of a scope.
+ /** Create an invalid mapping that is used as place-holder for
+ * arrays that are not mapped at all.
+ */
+ array_remapping();
+
+ /** Simple remapping that is done when the lifetimes do not
+ * overlap.
+ * @param trgt_array_id ID of the array that the new array will
+ * be interleaved with
+ */
+ array_remapping(int trgt_array_id, unsigned src_access_mask);
+
+ /** Component interleaving of arrays.
+ * @param target_array_id ID of the array that the new array will
+ * be interleaved with
+ * @param trgt_access_mask the component mast of the target array
*mask
+ * (the components that are already reserved)
+ * @param orig_component_mask
+ */
+ array_remapping(int trgt_array_id, int trgt_access_mask,
+ int src_access_mask);
Please make sure the comment and declaration match.
Cheers,
Nicolai
+
+ /* Defines a valid remapping */
+ bool is_valid() const {return target_id > 0;}
+
+ /* Resolve the mapping chain so that this mapping remaps to an
+ * array that is not remapped.
+ */
+ void finalize_mappings(array_remapping *arr_map);
+
+ void set_target_id(int tid) {target_id = tid;}
+
+ /* Translates the write mask to the new, interleaved component
+ * position
+ */
+ int map_writemask(int original_src_access_mask) const;
+
+ /* Translates all read swizzles to the new, interleaved component
+ * swizzles
+ */
+ uint16_t map_swizzles(uint16_t original_swizzle) const;
+
+ /** Move the read swizzles to the positiones that correspond to
+ * a changed write mask.
+ */
+ uint16_t move_read_swizzles(uint16_t original_swizzle) const;
+
+ unsigned target_array_id() const {return target_id;}
+
+ int combined_access_mask() const {return summary_access_mask;}
+
+ void print(std::ostream& os) const;
+
+ bool is_finalized() { return finalized;}
+
+ friend bool operator == (const array_remapping& lhs,
+ const array_remapping& rhs);
+
+ int map_one_swizzle(int swizzle_to_map) const;
+
+private:
+ unsigned target_id;
+ uint16_t writemask_map[4];
+ int16_t read_swizzle_map[4];
+ unsigned summary_access_mask:4;
+ unsigned original_src_access_mask:4;
+ int reswizzle:1;
+ int finalized:1;
+};
+
+inline
+std::ostream& operator << (std::ostream& os, const array_remapping& am)
+{
+ am.print(os);
+ return os;
+}
+
+}
+#endif
--
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
