Ilia Mirkin <imir...@alum.mit.edu> writes: > Francisco, any opinion on this patch (as the resident C++ expert)? It > seems a little odd. I'd just as soon skip this, and just use "using > std::tr1::unordered_set" or "using std::unordered_set" as necessary in > the next patch. But perhaps this is a common technique? > This seems really ugly to me. I've given the patch a try and the single instantiation that this patch saves is some 2.7 kB of the binary, some 0.1% of the size of nouveau_compiler alone and 0.03% of the size of the DRI driver with my configuration. IMHO the tiny saving in disk space hardly justifies the additional complexity and giving up the type-safe interface provided by the STL (except for the begin() and end() methods you have overridden with correct types, which is a tiny fraction of the unordered_set interface).
If this is required for compatibility reasons with old Android systems I
suggest you wrap it under a preprocessor conditional like:
#if __cplusplus >= 201103L
using std::unordered_set;
#elif building-on-old-android-version-with-broken-stlport
using my-funky-wrapper-for-std-unordered-set;
#else
using std::tr1::unordered_set;
#endif
> On Tue, May 19, 2015 at 11:25 PM, Chih-Wei Huang
> <cwhu...@android-x86.org> wrote:
>> Instead of using unordered_set<user-defined-type *> directly, the patch
>> changes to use unordered_set<void *> and adds a wrapper template class
>> to convert the iterators to the expected user-defined type.
>>
>> This avoid instantiating the template multiple times and make the object
>> code be smaller about 4KB.
>>
>> Signed-off-by: Chih-Wei Huang <cwhu...@linux.org.tw>
>> ---
>> src/gallium/drivers/nouveau/codegen/nv50_ir.h | 28
>> +++++++++++++++++++---
>> .../nouveau/codegen/nv50_ir_lowering_nvc0.cpp | 4 ++--
>> .../nouveau/codegen/nv50_ir_lowering_nvc0.h | 4 +---
>> src/gallium/drivers/nouveau/codegen/nv50_ir_ra.cpp | 5 ++--
>> 4 files changed, 30 insertions(+), 11 deletions(-)
>>
>> diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir.h
>> b/src/gallium/drivers/nouveau/codegen/nv50_ir.h
>> index 529dcb9..f4d52b7 100644
>> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir.h
>> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir.h
>> @@ -451,6 +451,28 @@ struct Storage
>> #define NV50_IR_INTERP_OFFSET (2 << 2)
>> #define NV50_IR_INTERP_SAMPLEID (3 << 2)
>>
>> +typedef std::tr1::unordered_set<void *> voidptr_unordered_set;
>> +
>> +template <typename V>
>> +class ptr_unordered_set : public voidptr_unordered_set {
>> + public:
>> + typedef voidptr_unordered_set _base;
>> + typedef _base::iterator _biterator;
>> +
>> + class iterator : public _biterator {
>> + public:
>> + iterator(const _biterator & i) : _biterator(i) {}
>> + V *operator*() { return reinterpret_cast<V *>(*_biterator(*this)); }
>> + const V *operator*() const { return reinterpret_cast<const V
>> *>(*_biterator(*this)); }
>> + };
>> + typedef const iterator const_iterator;
>> +
>> + iterator begin() { return _base::begin(); }
>> + iterator end() { return _base::end(); }
>> + const_iterator begin() const { return _base::begin(); }
>> + const_iterator end() const { return _base::end(); }
>> +};
>> +
>> // do we really want this to be a class ?
>> class Modifier
>> {
>> @@ -583,10 +605,10 @@ public:
>>
>> static inline Value *get(Iterator&);
>>
>> - std::tr1::unordered_set<ValueRef *> uses;
>> + ptr_unordered_set<ValueRef> uses;
>> std::list<ValueDef *> defs;
>> - typedef std::tr1::unordered_set<ValueRef *>::iterator UseIterator;
>> - typedef std::tr1::unordered_set<ValueRef *>::const_iterator UseCIterator;
>> + typedef ptr_unordered_set<ValueRef>::iterator UseIterator;
>> + typedef ptr_unordered_set<ValueRef>::const_iterator UseCIterator;
>> typedef std::list<ValueDef *>::iterator DefIterator;
>> typedef std::list<ValueDef *>::const_iterator DefCIterator;
>>
>> diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp
>> b/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp
>> index b61f3c4..669d292 100644
>> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp
>> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp
>> @@ -224,7 +224,7 @@ NVC0LegalizePostRA::findFirstUses(
>> const Instruction *texi,
>> const Instruction *insn,
>> std::list<TexUse> &uses,
>> - std::tr1::unordered_set<const Instruction *>& visited)
>> + ptr_unordered_set<const Instruction>& visited)
>> {
>> for (int d = 0; insn->defExists(d); ++d) {
>> Value *v = insn->getDef(d);
>> @@ -318,7 +318,7 @@ NVC0LegalizePostRA::insertTextureBarriers(Function *fn)
>> if (!uses)
>> return false;
>> for (size_t i = 0; i < texes.size(); ++i) {
>> - std::tr1::unordered_set<const Instruction *> visited;
>> + ptr_unordered_set<const Instruction> visited;
>> findFirstUses(texes[i], texes[i], uses[i], visited);
>> }
>>
>> diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.h
>> b/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.h
>> index 260e101..17b6f6f 100644
>> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.h
>> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.h
>> @@ -20,8 +20,6 @@
>> * OTHER DEALINGS IN THE SOFTWARE.
>> */
>>
>> -#include <tr1/unordered_set>
>> -
>> #include "codegen/nv50_ir.h"
>> #include "codegen/nv50_ir_build_util.h"
>>
>> @@ -73,7 +71,7 @@ private:
>> inline bool insnDominatedBy(const Instruction *, const Instruction *)
>> const;
>> void findFirstUses(const Instruction *tex, const Instruction *def,
>> std::list<TexUse>&,
>> - std::tr1::unordered_set<const Instruction *>&);
>> + ptr_unordered_set<const Instruction>&);
>> void findOverwritingDefs(const Instruction *tex, Instruction *insn,
>> const BasicBlock *term,
>> std::list<TexUse>&);
>> diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_ra.cpp
>> b/src/gallium/drivers/nouveau/codegen/nv50_ir_ra.cpp
>> index 898653c..03acba7 100644
>> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_ra.cpp
>> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_ra.cpp
>> @@ -25,7 +25,6 @@
>>
>> #include <stack>
>> #include <limits>
>> -#include <tr1/unordered_set>
>>
>> namespace nv50_ir {
>>
>> @@ -1551,7 +1550,7 @@ SpillCodeInserter::run(const std::list<ValuePair>& lst)
>> // Keep track of which instructions to delete later. Deleting them
>> // inside the loop is unsafe since a single instruction may have
>> // multiple destinations that all need to be spilled (like OP_SPLIT).
>> - std::tr1::unordered_set<Instruction *> to_del;
>> + ptr_unordered_set<Instruction> to_del;
>>
>> for (Value::DefIterator d = lval->defs.begin(); d != lval->defs.end();
>> ++d) {
>> @@ -1593,7 +1592,7 @@ SpillCodeInserter::run(const std::list<ValuePair>& lst)
>> }
>> }
>>
>> - for (std::tr1::unordered_set<Instruction *>::const_iterator it =
>> to_del.begin();
>> + for (ptr_unordered_set<Instruction>::iterator it = to_del.begin();
>> it != to_del.end(); ++it)
>> delete_Instruction(func->getProgram(), *it);
>> }
>> --
>> 1.9.1
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
signature.asc
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
