pull-request
|
View:
New views
7 Messages
—
Rating Filter:
Alert me
|
 |
pull-request
by Cyrill Gorcunov
::
Rate this Message:
Hi Peter,
please consider pulling from my repo. As usually I would appreciate any kind of comments/complains. I do not include the whole diff here since it's a bunch of LOC changed (especially due to merge of elf headers). Also some work done in elfX.c in a sake of future merging. Though the one patch deserve to be included into the message just for easier review. -- Cyrill --- From: Cyrill Gorcunov <gorcunov@...> Date: Fri, 6 Nov 2009 09:07:00 +0300 Subject: [PATCH 2/8] hashtbl.c: Unify common hash ops by macros Instead of opencoded repeatable computation use macro helpers. Signed-off-by: Cyrill Gorcunov <gorcunov@...> --- hashtbl.c | 54 +++++++++++++++++++++++++++++++----------------------- 1 files changed, 31 insertions(+), 23 deletions(-) diff --git a/hashtbl.c b/hashtbl.c index 2b55755..181da49 100644 --- a/hashtbl.c +++ b/hashtbl.c @@ -46,12 +46,18 @@ #define HASH_MAX_LOAD 2 /* Higher = more memory-efficient, slower */ +#define __hash_calc(key) crc64(CRC64_INIT, key) +#define __hash_calci(key) crc64i(CRC64_INIT, key) +#define __hash_max_load(size) (size * (HASH_MAX_LOAD - 1) / HASH_MAX_LOAD) +#define __hash_mask(size) (size - 1) +#define __hash_pos(hash, mask) (hash & mask) +#define __hash_inc(hash, mask) (((hash >> 32) & mask) | 1) /* always odd */ +#define __hash_pos_next(pos, inc, mask) ((pos + inc) & mask) + static struct hash_tbl_node *alloc_table(size_t newsize) { - size_t bytes = newsize*sizeof(struct hash_tbl_node); - struct hash_tbl_node *newtbl = nasm_zalloc(bytes); - - return newtbl; + size_t bytes = newsize * sizeof(struct hash_tbl_node); + return nasm_zalloc(bytes); } void hash_init(struct hash_table *head, size_t size) @@ -59,7 +65,7 @@ void hash_init(struct hash_table *head, size_t size) head->table = alloc_table(size); head->load = 0; head->size = size; - head->max_load = size*(HASH_MAX_LOAD-1)/HASH_MAX_LOAD; + head->max_load = __hash_max_load(size); } /* @@ -78,16 +84,16 @@ void **hash_find(struct hash_table *head, const char *key, struct hash_insert *insert) { struct hash_tbl_node *np; - uint64_t hash = crc64(CRC64_INIT, key); struct hash_tbl_node *tbl = head->table; - size_t mask = head->size-1; - size_t pos = hash & mask; - size_t inc = ((hash >> 32) & mask) | 1; /* Always odd */ + uint64_t hash = __hash_calc(key); + size_t mask = __hash_mask(head->size); + size_t pos = __hash_pos(hash, mask); + size_t inc = __hash_inc(hash, mask); while ((np = &tbl[pos])->key) { if (hash == np->hash && !strcmp(key, np->key)) return &np->data; - pos = (pos+inc) & mask; + pos = __hash_pos_next(pos, inc, mask); } /* Not found. Store info for insert if requested. */ @@ -106,16 +112,16 @@ void **hash_findi(struct hash_table *head, const char *key, struct hash_insert *insert) { struct hash_tbl_node *np; - uint64_t hash = crc64i(CRC64_INIT, key); struct hash_tbl_node *tbl = head->table; - size_t mask = head->size-1; - size_t pos = hash & mask; - size_t inc = ((hash >> 32) & mask) | 1; /* Always odd */ + uint64_t hash = __hash_calci(key); + size_t mask = __hash_mask(head->size); + size_t pos = __hash_pos(hash, mask); + size_t inc = __hash_inc(hash, mask); while ((np = &tbl[pos])->key) { if (hash == np->hash && !nasm_stricmp(key, np->key)) return &np->data; - pos = (pos+inc) & mask; + pos = __hash_pos_next(pos, inc, mask); } /* Not found. Store info for insert if requested. */ @@ -136,8 +142,10 @@ void **hash_add(struct hash_insert *insert, const char *key, void *data) struct hash_table *head = insert->head; struct hash_tbl_node *np = insert->where; - /* Insert node. We can always do this, even if we need to - rebalance immediately after. */ + /* + * Insert node. We can always do this, even if we need to + * rebalance immediately after. + */ np->hash = insert->hash; np->key = key; np->data = data; @@ -146,7 +154,7 @@ void **hash_add(struct hash_insert *insert, const char *key, void *data) /* Need to expand the table */ size_t newsize = head->size << 1; struct hash_tbl_node *newtbl = alloc_table(newsize); - size_t mask = newsize-1; + size_t mask = __hash_mask(newsize); if (head->table) { struct hash_tbl_node *op, *xp; @@ -155,11 +163,11 @@ void **hash_add(struct hash_insert *insert, const char *key, void *data) /* Rebalance all the entries */ for (i = 0, op = head->table; i < head->size; i++, op++) { if (op->key) { - size_t pos = op->hash & mask; - size_t inc = ((op->hash >> 32) & mask) | 1; + size_t pos = __hash_pos(op->hash, mask); + size_t inc = __hash_inc(op->hash, mask); while ((xp = &newtbl[pos])->key) - pos = (pos+inc) & mask; + pos = __hash_pos_next(pos, inc, mask); *xp = *op; if (op == np) @@ -171,7 +179,7 @@ void **hash_add(struct hash_insert *insert, const char *key, void *data) head->table = newtbl; head->size = newsize; - head->max_load = newsize*(HASH_MAX_LOAD-1)/HASH_MAX_LOAD; + head->max_load = __hash_max_load(newsize); } return &np->data; @@ -197,7 +205,7 @@ void *hash_iterate(const struct hash_table *head, while (np < ep) { if (np->key) { - *iterator = np+1; + *iterator = np + 1; if (key) *key = np->key; return np->data; -- 1.6.5 --- The following changes since commit 19f9f60efbc3f240a9cdb04b4c33e7812d78463c: H. Peter Anvin (1): MOVD xmmreg: not valid with REX.W are available in the git repository at: git://repo.or.cz/nasm-cyr.git for-hpa Cyrill Gorcunov (8): Introduce is_power2 helper hashtbl.c: Unify common hash ops by macros hash_init: check for size being power of two Merge elf header files Document BR 2887108 in changes.src output/outelf64.c: simplify stabs64_cleanup, dwarf64_cleanup dwarfX_output: dont check for "ln" twice outelf32/64 - various cleanups in a sake of unification Makefile.in | 10 +- Mkfiles/msvc.mak | 8 +- Mkfiles/netware.mak | 6 +- Mkfiles/openwcom.mak | 6 +- Mkfiles/owlinux.mak | 6 +- doc/changes.src | 6 +- hashtbl.c | 55 +++--- nasmlib.h | 3 + output/elf.h | 531 ++++++++++++++++++++++++++++++++++++++++++++++++++ output/elf32.h | 167 ---------------- output/elf64.h | 189 ------------------ output/elfcommon.h | 250 ------------------------ output/outelf.c | 2 +- output/outelf32.c | 39 ++-- output/outelf64.c | 406 ++++++++++++++++++--------------------- 15 files changed, 796 insertions(+), 888 deletions(-) create mode 100644 output/elf.h delete mode 100644 output/elf32.h delete mode 100644 output/elf64.h delete mode 100644 output/elfcommon.h ------------------------------------------------------------------------------ Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day trial. Simplify your report design, integration and deployment - and focus on what you do best, core application coding. Discover what's new with Crystal Reports now. http://p.sf.net/sfu/bobj-july _______________________________________________ Nasm-devel mailing list Nasm-devel@... https://lists.sourceforge.net/lists/listinfo/nasm-devel |
|
|
Re: pull-request
by H. Peter Anvin
::
Rate this Message:
On 11/06/2009 10:10 AM, Cyrill Gorcunov wrote:
> Hi Peter, > > please consider pulling from my repo. As usually I would appreciate any kind > of comments/complains. > > I do not include the whole diff here since it's a bunch of LOC changed > (especially due to merge of elf headers). > > Also some work done in elfX.c in a sake of future merging. > > Though the one patch deserve to be included into the message just > for easier review. > > -- Cyrill > > +#define __hash_calc(key) crc64(CRC64_INIT, key) > +#define __hash_calci(key) crc64i(CRC64_INIT, key) > +#define __hash_max_load(size) (size * (HASH_MAX_LOAD - 1) / HASH_MAX_LOAD) > +#define __hash_mask(size) (size - 1) > +#define __hash_pos(hash, mask) (hash & mask) > +#define __hash_inc(hash, mask) (((hash >> 32) & mask) | 1) /* always odd */ > +#define __hash_pos_next(pos, inc, mask) ((pos + inc) & mask) > + Please avoid double-underscore symbols (or underscore + capital letter symbols). They belong to the C implementation (compiler/library) and are therefore nonportable -- and NASM strives to be maximally portable. Furthermore, these are macros but don't ()-protect their arguments, which is not good. -hpa ------------------------------------------------------------------------------ Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day trial. Simplify your report design, integration and deployment - and focus on what you do best, core application coding. Discover what's new with Crystal Reports now. http://p.sf.net/sfu/bobj-july _______________________________________________ Nasm-devel mailing list Nasm-devel@... https://lists.sourceforge.net/lists/listinfo/nasm-devel |
|
|
Re: pull-request
by Cyrill Gorcunov
::
Rate this Message:
[H. Peter Anvin - Fri, Nov 06, 2009 at 10:30:38AM -0800]
| On 11/06/2009 10:10 AM, Cyrill Gorcunov wrote: | > Hi Peter, | > | > please consider pulling from my repo. As usually I would appreciate any kind | > of comments/complains. | > | > I do not include the whole diff here since it's a bunch of LOC changed | > (especially due to merge of elf headers). | > | > Also some work done in elfX.c in a sake of future merging. | > | > Though the one patch deserve to be included into the message just | > for easier review. | > | > -- Cyrill | | > | > +#define __hash_calc(key) crc64(CRC64_INIT, key) | > +#define __hash_calci(key) crc64i(CRC64_INIT, key) | > +#define __hash_max_load(size) (size * (HASH_MAX_LOAD - 1) / HASH_MAX_LOAD) | > +#define __hash_mask(size) (size - 1) | > +#define __hash_pos(hash, mask) (hash & mask) | > +#define __hash_inc(hash, mask) (((hash >> 32) & mask) | 1) /* always odd */ | > +#define __hash_pos_next(pos, inc, mask) ((pos + inc) & mask) | > + | | Please avoid double-underscore symbols (or underscore + capital letter | symbols). They belong to the C implementation (compiler/library) and | are therefore nonportable -- and NASM strives to be maximally portable. | Furthermore, these are macros but don't ()-protect their arguments, | which is not good. | | -hpa | Yeah, will do. But Peter, I doubt a bit about this patch in general (ie about an idea to use macros here). I found it convenient but still would like to get ACK or NACK on them. -- Cyrill ------------------------------------------------------------------------------ Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day trial. Simplify your report design, integration and deployment - and focus on what you do best, core application coding. Discover what's new with Crystal Reports now. http://p.sf.net/sfu/bobj-july _______________________________________________ Nasm-devel mailing list Nasm-devel@... https://lists.sourceforge.net/lists/listinfo/nasm-devel |
|
|
Re: pull-request
by H. Peter Anvin
::
Rate this Message:
On 11/06/2009 10:40 AM, Cyrill Gorcunov wrote:
> > Yeah, will do. But Peter, I doubt a bit about this patch in general > (ie about an idea to use macros here). I found it convenient but still > would like to get ACK or NACK on them. > It probably is worthwhile, because they are configurable parameters that need to be kept in sync across the implementation. However, they obviously need to be done properly. -hpa ------------------------------------------------------------------------------ Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day trial. Simplify your report design, integration and deployment - and focus on what you do best, core application coding. Discover what's new with Crystal Reports now. http://p.sf.net/sfu/bobj-july _______________________________________________ Nasm-devel mailing list Nasm-devel@... https://lists.sourceforge.net/lists/listinfo/nasm-devel |
|
|
Re: pull-request
by Cyrill Gorcunov
::
Rate this Message:
[H. Peter Anvin - Fri, Nov 06, 2009 at 10:48:44AM -0800]
| On 11/06/2009 10:40 AM, Cyrill Gorcunov wrote: | > | > Yeah, will do. But Peter, I doubt a bit about this patch in general | > (ie about an idea to use macros here). I found it convenient but still | > would like to get ACK or NACK on them. | > | | It probably is worthwhile, because they are configurable parameters that | need to be kept in sync across the implementation. However, they | obviously need to be done properly. | | -hpa | Something like this? (if yes -- don;t take it from here -- I will update either my repo for pulling or push to main repo). Also added hash_expand since new size should be power of 2 as well. -- Cyrill --- From: Cyrill Gorcunov <gorcunov@...> Date: Fri, 6 Nov 2009 21:58:48 +0300 Subject: [PATCH 2/8] hashtbl.c: Unify common hash ops by macros Instead of opencoded repeatable computation use macro helpers. Signed-off-by: Cyrill Gorcunov <gorcunov@...> --- hashtbl.c | 59 ++++++++++++++++++++++++++++++++++------------------------- 1 files changed, 34 insertions(+), 25 deletions(-) diff --git a/hashtbl.c b/hashtbl.c index 2b55755..879dd93 100644 --- a/hashtbl.c +++ b/hashtbl.c @@ -46,12 +46,19 @@ #define HASH_MAX_LOAD 2 /* Higher = more memory-efficient, slower */ +#define hash_calc(key) crc64(CRC64_INIT, (key)) +#define hash_calci(key) crc64i(CRC64_INIT, (key)) +#define hash_max_load(size) ((size) * (HASH_MAX_LOAD - 1) / HASH_MAX_LOAD) +#define hash_expand(size) ((size) << 1) +#define hash_mask(size) ((size) - 1) +#define hash_pos(hash, mask) ((hash) & (mask)) +#define hash_inc(hash, mask) ((((hash) >> 32) & (mask)) | 1) /* always odd */ +#define hash_pos_next(pos, inc, mask) (((pos) + (inc)) & (mask)) + static struct hash_tbl_node *alloc_table(size_t newsize) { - size_t bytes = newsize*sizeof(struct hash_tbl_node); - struct hash_tbl_node *newtbl = nasm_zalloc(bytes); - - return newtbl; + size_t bytes = newsize * sizeof(struct hash_tbl_node); + return nasm_zalloc(bytes); } void hash_init(struct hash_table *head, size_t size) @@ -59,7 +66,7 @@ void hash_init(struct hash_table *head, size_t size) head->table = alloc_table(size); head->load = 0; head->size = size; - head->max_load = size*(HASH_MAX_LOAD-1)/HASH_MAX_LOAD; + head->max_load = hash_max_load(size); } /* @@ -78,16 +85,16 @@ void **hash_find(struct hash_table *head, const char *key, struct hash_insert *insert) { struct hash_tbl_node *np; - uint64_t hash = crc64(CRC64_INIT, key); struct hash_tbl_node *tbl = head->table; - size_t mask = head->size-1; - size_t pos = hash & mask; - size_t inc = ((hash >> 32) & mask) | 1; /* Always odd */ + uint64_t hash = hash_calc(key); + size_t mask = hash_mask(head->size); + size_t pos = hash_pos(hash, mask); + size_t inc = hash_inc(hash, mask); while ((np = &tbl[pos])->key) { if (hash == np->hash && !strcmp(key, np->key)) return &np->data; - pos = (pos+inc) & mask; + pos = hash_pos_next(pos, inc, mask); } /* Not found. Store info for insert if requested. */ @@ -106,16 +113,16 @@ void **hash_findi(struct hash_table *head, const char *key, struct hash_insert *insert) { struct hash_tbl_node *np; - uint64_t hash = crc64i(CRC64_INIT, key); struct hash_tbl_node *tbl = head->table; - size_t mask = head->size-1; - size_t pos = hash & mask; - size_t inc = ((hash >> 32) & mask) | 1; /* Always odd */ + uint64_t hash = hash_calci(key); + size_t mask = hash_mask(head->size); + size_t pos = hash_pos(hash, mask); + size_t inc = hash_inc(hash, mask); while ((np = &tbl[pos])->key) { if (hash == np->hash && !nasm_stricmp(key, np->key)) return &np->data; - pos = (pos+inc) & mask; + pos = hash_pos_next(pos, inc, mask); } /* Not found. Store info for insert if requested. */ @@ -136,17 +143,19 @@ void **hash_add(struct hash_insert *insert, const char *key, void *data) struct hash_table *head = insert->head; struct hash_tbl_node *np = insert->where; - /* Insert node. We can always do this, even if we need to - rebalance immediately after. */ + /* + * Insert node. We can always do this, even if we need to + * rebalance immediately after. + */ np->hash = insert->hash; np->key = key; np->data = data; if (++head->load > head->max_load) { /* Need to expand the table */ - size_t newsize = head->size << 1; - struct hash_tbl_node *newtbl = alloc_table(newsize); - size_t mask = newsize-1; + size_t newsize = hash_expand(head->size); + struct hash_tbl_node *newtbl = alloc_table(newsize); + size_t mask = hash_mask(newsize); if (head->table) { struct hash_tbl_node *op, *xp; @@ -155,11 +164,11 @@ void **hash_add(struct hash_insert *insert, const char *key, void *data) /* Rebalance all the entries */ for (i = 0, op = head->table; i < head->size; i++, op++) { if (op->key) { - size_t pos = op->hash & mask; - size_t inc = ((op->hash >> 32) & mask) | 1; + size_t pos = hash_pos(op->hash, mask); + size_t inc = hash_inc(op->hash, mask); while ((xp = &newtbl[pos])->key) - pos = (pos+inc) & mask; + pos = hash_pos_next(pos, inc, mask); *xp = *op; if (op == np) @@ -171,7 +180,7 @@ void **hash_add(struct hash_insert *insert, const char *key, void *data) head->table = newtbl; head->size = newsize; - head->max_load = newsize*(HASH_MAX_LOAD-1)/HASH_MAX_LOAD; + head->max_load = hash_max_load(newsize); } return &np->data; @@ -197,7 +206,7 @@ void *hash_iterate(const struct hash_table *head, while (np < ep) { if (np->key) { - *iterator = np+1; + *iterator = np + 1; if (key) *key = np->key; return np->data; -- 1.6.5 ------------------------------------------------------------------------------ Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day trial. Simplify your report design, integration and deployment - and focus on what you do best, core application coding. Discover what's new with Crystal Reports now. http://p.sf.net/sfu/bobj-july _______________________________________________ Nasm-devel mailing list Nasm-devel@... https://lists.sourceforge.net/lists/listinfo/nasm-devel |
|
|
Re: pull-request
by H. Peter Anvin
::
Rate this Message:
On 11/06/2009 11:03 AM, Cyrill Gorcunov wrote:
> [H. Peter Anvin - Fri, Nov 06, 2009 at 10:48:44AM -0800] > | On 11/06/2009 10:40 AM, Cyrill Gorcunov wrote: > | > > | > Yeah, will do. But Peter, I doubt a bit about this patch in general > | > (ie about an idea to use macros here). I found it convenient but still > | > would like to get ACK or NACK on them. > | > > | > | It probably is worthwhile, because they are configurable parameters that > | need to be kept in sync across the implementation. However, they > | obviously need to be done properly. > | > | -hpa > | > > Something like this? (if yes -- don;t take it from > here -- I will update either my repo for pulling > or push to main repo). Also added hash_expand since > new size should be power of 2 as well. > Looks good. -hpa ------------------------------------------------------------------------------ Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day trial. Simplify your report design, integration and deployment - and focus on what you do best, core application coding. Discover what's new with Crystal Reports now. http://p.sf.net/sfu/bobj-july _______________________________________________ Nasm-devel mailing list Nasm-devel@... https://lists.sourceforge.net/lists/listinfo/nasm-devel |
|
|
Re: pull-request
by Cyrill Gorcunov
::
Rate this Message:
[H. Peter Anvin - Fri, Nov 06, 2009 at 11:07:35AM -0800]
| On 11/06/2009 11:03 AM, Cyrill Gorcunov wrote: | > [H. Peter Anvin - Fri, Nov 06, 2009 at 10:48:44AM -0800] | > | On 11/06/2009 10:40 AM, Cyrill Gorcunov wrote: | > | > | > | > Yeah, will do. But Peter, I doubt a bit about this patch in general | > | > (ie about an idea to use macros here). I found it convenient but still | > | > would like to get ACK or NACK on them. | > | > | > | | > | It probably is worthwhile, because they are configurable parameters that | > | need to be kept in sync across the implementation. However, they | > | obviously need to be done properly. | > | | > | -hpa | > | | > | > Something like this? (if yes -- don;t take it from | > here -- I will update either my repo for pulling | > or push to main repo). Also added hash_expand since | > new size should be power of 2 as well. | > | | Looks good. | | -hpa | OK, I've updated -for-hpa branch on my repo -- Cyrill --- The following changes since commit 19f9f60efbc3f240a9cdb04b4c33e7812d78463c: H. Peter Anvin (1): MOVD xmmreg: not valid with REX.W are available in the git repository at: git://repo.or.cz/nasm-cyr.git for-hpa Cyrill Gorcunov (8): Introduce is_power2 helper hashtbl.c: Unify common hash ops by macros hash_init: check for size being power of two Merge elf header files Document BR 2887108 in changes.src output/outelf64.c: simplify stabs64_cleanup, dwarf64_cleanup dwarfX_output: dont check for "ln" twice outelf32/64 - various cleanups in a sake of unification Makefile.in | 10 +- Mkfiles/msvc.mak | 8 +- Mkfiles/netware.mak | 6 +- Mkfiles/openwcom.mak | 6 +- Mkfiles/owlinux.mak | 6 +- doc/changes.src | 6 +- hashtbl.c | 60 ++++--- nasmlib.h | 3 + output/elf.h | 531 ++++++++++++++++++++++++++++++++++++++++++++++++++ output/elf32.h | 167 ---------------- output/elf64.h | 189 ------------------ output/elfcommon.h | 250 ------------------------ output/outelf.c | 2 +- output/outelf32.c | 39 ++-- output/outelf64.c | 406 ++++++++++++++++++--------------------- 15 files changed, 799 insertions(+), 890 deletions(-) create mode 100644 output/elf.h delete mode 100644 output/elf32.h delete mode 100644 output/elf64.h delete mode 100644 output/elfcommon.h ------------------------------------------------------------------------------ Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day trial. Simplify your report design, integration and deployment - and focus on what you do best, core application coding. Discover what's new with Crystal Reports now. http://p.sf.net/sfu/bobj-july _______________________________________________ Nasm-devel mailing list Nasm-devel@... https://lists.sourceforge.net/lists/listinfo/nasm-devel |
| Free embeddable forum powered by Nabble | Forum Help |
