Jakub Jelinek <ja...@redhat.com> writes:
> On Wed, May 10, 2023 at 07:57:05PM +0800, pan2...@intel.com wrote:
>> --- a/gcc/var-tracking.cc
>> +++ b/gcc/var-tracking.cc
>> @@ -116,9 +116,14 @@
>> #include "fibonacci_heap.h"
>> #include "print-rtl.h"
>> #include "function-abi.h"
>> +#include "mux-utils.h"
>>
>> typedef fibonacci_heap <long, basic_block_def> bb_heap_t;
>>
>> +/* A declaration of a variable, or an RTL value being handled like a
>> + declaration by pointer_mux. */
>> +typedef pointer_mux<tree_node, rtx_def> decl_or_value;
>> +
>> /* var-tracking.cc assumes that tree code with the same value as VALUE rtx
>> code
>> has no chance to appear in REG_EXPR/MEM_EXPRs and isn't a decl.
>> Currently the value is the same as IDENTIFIER_NODE, which has such
>> @@ -196,15 +201,11 @@ struct micro_operation
>> };
>>
>>
>> -/* A declaration of a variable, or an RTL value being handled like a
>> - declaration. */
>> -typedef void *decl_or_value;
>
> Why do you move the typedef?
>
>> @@ -503,9 +505,7 @@ variable_hasher::hash (const variable *v)
>> inline bool
>> variable_hasher::equal (const variable *v, const void *y)
>> {
>> - decl_or_value dv = CONST_CAST2 (decl_or_value, const void *, y);
>> -
>> - return (dv_as_opaque (v->dv) == dv_as_opaque (dv));
>> + return dv_as_opaque (v->dv) == y;
>> }
>
> I'm not convinced this is correct. I think all the find_slot_with_hash
> etc. pass in a decl_or_value, so I'd expect y to have decl_or_value
> type or something similar.
>
>> /* Free the element of VARIABLE_HTAB (its type is struct variable_def). */
>> @@ -1396,8 +1396,7 @@ onepart_pool_allocate (onepart_enum onepart)
>> static inline decl_or_value
>> dv_from_decl (tree decl)
>> {
>> - decl_or_value dv;
>> - dv = decl;
>> + decl_or_value dv = decl_or_value::first (decl);
>
> Can't you just decl_or_value dv = decl; ? I think pointer_mux has ctors
> from pointers to the template parameter types.
>
>> gcc_checking_assert (dv_is_decl_p (dv));
>> return dv;
>> }
>> @@ -1406,8 +1405,7 @@ dv_from_decl (tree decl)
>> static inline decl_or_value
>> dv_from_value (rtx value)
>> {
>> - decl_or_value dv;
>> - dv = value;
>> + decl_or_value dv = decl_or_value::second (value);
>
> Ditto.
>
>> @@ -1661,7 +1659,8 @@ shared_hash_find_slot_unshare_1 (shared_hash **pvars,
>> decl_or_value dv,
>> {
>> if (shared_hash_shared (*pvars))
>> *pvars = shared_hash_unshare (*pvars);
>> - return shared_hash_htab (*pvars)->find_slot_with_hash (dv, dvhash, ins);
>> + return shared_hash_htab (*pvars)->find_slot_with_hash (dv_as_opaque (dv),
>> + dvhash, ins);
>
> Then you wouldn't need to change all these.
Also, please do try changing variable_hasher::compare_type to
decl_or_value, and changing the type of the second parameter to
variable_hasher::equal accordingly. I still feel that we should
be able to get rid of dv_as_opaque entirely.
Thanks,
Richard
