Hi, Richard,
In order to be consistent with other flags in flag-types.h, for example,
“sanitize_code”,
I didn’t use namespace, instead making the name more specific as following:
/* Different settings for zeroing subset of registers. */
enum zero_regs_flags {
ZERO_REGS_UNSET = 0,
ZERO_REGS_SKIP = 1UL << 0,
ZERO_REGS_ONLY_USED = 1UL << 1,
ZERO_REGS_ONLY_GPR = 1UL << 2,
ZERO_REGS_ONLY_ARG = 1UL << 3,
ZERO_REGS_ENABLED = 1UL << 4,
ZERO_REGS_USED_GPR_ARG = ZERO_REGS_ENABLED | ZERO_REGS_ONLY_USED
| ZERO_REGS_ONLY_GPR | ZERO_REGS_ONLY_ARG,
ZERO_REGS_USED_GPR = ZERO_REGS_ENABLED | ZERO_REGS_ONLY_USED
| ZERO_REGS_ONLY_GPR,
ZERO_REGS_USED_ARG = ZERO_REGS_ENABLED | ZERO_REGS_ONLY_USED
| ZERO_REGS_ONLY_ARG,
ZERO_REGS_USED = ZERO_REGS_ENABLED | ZERO_REGS_ONLY_USED,
ZERO_REGS_ALL_GPR_ARG = ZERO_REGS_ENABLED | ZERO_REGS_ONLY_GPR
| ZERO_REGS_ONLY_ARG,
ZERO_REGS_ALL_GPR = ZERO_REGS_ENABLED | ZERO_REGS_ONLY_GPR,
ZERO_REGS_ALL_ARG = ZERO_REGS_ENABLED | ZERO_REGS_ONLY_ARG,
ZERO_REGS_ALL = ZERO_REGS_ENABLED
};
Is this good?
Or you still prefer namespace?
thanks.
Qing
> On Oct 27, 2020, at 10:36 AM, Richard Sandiford <richard.sandif...@arm.com>
> wrote:
>
> Qing Zhao <qing.z...@oracle.com> writes:
>>>> diff --git a/gcc/flag-types.h b/gcc/flag-types.h
>>>> index 852ea76..0f7e503 100644
>>>> --- a/gcc/flag-types.h
>>>> +++ b/gcc/flag-types.h
>>>> @@ -285,6 +285,15 @@ enum sanitize_code {
>>>> | SANITIZE_BOUNDS_STRICT
>>>> };
>>>>
>>>> +enum zero_call_used_regs_code {
>>>> + UNSET = 0,
>>>> + SKIP = 1UL << 0,
>>>> + ONLY_USED = 1UL << 1,
>>>> + ONLY_GPR = 1UL << 2,
>>>> + ONLY_ARG = 1UL << 3,
>>>> + ALL = 1UL << 4
>>>> +};
>>>
>>> I'd suggested these names on the assumption that we'd be using
>>> a C++ enum class, so that the enum would be referenced as
>>> name::ALL, name::SKIP, etc. But I guess using a C++ enum class
>>> doesn't work well with bitfields after all.
>>>
>>> These names are too generic without the name:: scoping though.
>>> Perhaps we should put them in a namespace:
>>>
>>> namespace zero_regs_flags {
>>> const unsigned int UNSET = 0;
>>> …etc…
>>> }
>>>
>>> (call-used probably doesn't need to be part of the flag names,
>>> since the concept is more general than that and call-usedness
>>> is really a filter that's being applied on top. Although I guess
>>> the same is true of “zero”. ;-))
>>>
>>> I don't think we should have ALL as a separate flag: ALL is the absence
>>> of ONLY_*. Maybe we should have an ENABLED flag that all non-skip
>>> combinations use?
>>>
>>> If it makes things easier, I think it would be good to have e.g.:
>>>
>>> unsigned int USED_GPR = ENABLED | ONLY_USED | ONLY_GPR;
>>>
>>> inside the namespace, to reduce the verbosity in the option table.
>>
>> Then, the final namespace will look like:
>>
>> namespace zero_regs_flags {
>> const unsigned int UNSET = 0;
>> const unsigned int SKIP = 1UL << 0;
>> const unsigned int ONLY_USED = 1UL << 1;
>> const unsigned int ONLY_GPR = 1UL << 2;
>> const unsigned int ONLY_ARG = 1UL << 3;
>> const unsigned int ENABLED = 1UL << 4;
>> const unsigned int USED_GPR_ARG = ONLY_USED | ONLY_GPR | ONLY_ARG;
>
> “ENABLED |” here
>
>> const unsigned int USED_GPR = ENABLED | ONLY_USED | ONLY_GPR;
>> const unsigned int USED_ARG = ENABLED | ONLY_USED | ONLY_ARG;
>> const unsigned int USED = ENABLED | ONLY_USED;
>> const unsigned int ALL_GRP_ARG = ENABLED | ONLY_GPR | ONLY_ARG;
>
> GPR
>
>> const unsigned int ALL_GPR = ENABLED | ONLY_GPR;
>> const unsigned int ALL_ARG = ENABLED | ONLY_ARG;
>> const unsigned int ALL = ENABLED;
>> }
>>
>> ??
>
