Re: [PATCH, v2, OpenMP 5.0, libgomp] Structure element mapping for OpenMP 5.0

[Chung-Lin Tang <cltang@codesourcery.com>]

On 2021/1/16 5:45 下午, Jakub Jelinek wrote:
>> +    /* Unified reference count for structure element siblings, this is used
>> +       when REFCOUNT_STRUCTELEM_FIRST_P(k->refcount) == true, the first sibling
>> +       in a structure element sibling list item sequence.  */
>> +    uintptr_t structelem_refcount;
>> +
>> +    /* When REFCOUNT_STRUCTELEM_P (k->refcount) == true, this field points
> 
> REFCOUNT_STRUCTELEM_P (k->refcount) is true even for
> REFCOUNT_STRUCTELEM_FIRST_P(k->refcount), so shouldn't the description say
> that structelem_refcount_ptr is only used if
> REFCOUNT_STRUCTELEM_P (k->refcount) && !REFCOUNT_STRUCTELEM_FIRST_P (k->refcount)
> ?

Sure, I'll revise the comments a bit.

>> +       into the (above) structelem_refcount field of the _FIRST splay_tree_key,
>> +       the first key in the created sequence. All structure element siblings
>> +       share a single refcount in this manner. Since these two fields won't be
>> +       used at the same time, they are stashed in a union.  */
>> +    uintptr_t *structelem_refcount_ptr;
>> +  };
>>     struct splay_tree_aux *aux;
>>   };
>>   
>>   /* The comparison function.  */
> 
> Anyway, most of the patch looks good, but I'd like to understand the
> rationale for choosing a htab over what I've been trying to suggest, which
> was essentially instead of incrementing or decrementing refcounts push them
> into a vector for later incrementing/decrementing, then qsort the vector
> (by the pointers to refcounts) and increment what the elements point to unless
> the same address has been incremented/decremented already.
> 
> 	Jakub

Essentially the requirement is to increment/decrement a refcount only once per construct,
so using a pointer-set (implemented by htab_t here) to track the processing status
seemed to be more intuitive in code, and probably faster than sorting a vector I think
(at least in most cases).

Chung-Lin