Bernd Edlinger <bernd.edlin...@hotmail.de> writes:
> On 10/4/19 2:38 PM, Richard Sandiford wrote:
>> Bernd Edlinger <bernd.edlin...@hotmail.de> writes:
>>> Hi,
>>>
>>> this fixes -Wshadow=local warnings in the RTL_FLAG_CHECKx macros,
>>> which happen when this macro is used recursively in a macro
>>> argument. The __typeof (RTX) const _rtx in the inner macro
>>> expansions shadows the outer macro expansions.
>>>
>>> So reworked the macro to not use statement expressions but
>>> use templates instead. Since the 7-argument overload is not
>>> used anywhere removed RTL_FLAG_CHECK7 for now.
>>>
>>>
>>> Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
>>> Is it OK for trunk?
>>>
>>>
>>> Thanks
>>> Bernd.
>>>
>>> 2019-10-03 Bernd Edlinger <bernd.edlin...@hotmail.de>
>>>
>>> * rtl.h (RTL_FLAG_CHECK): New variadic macro.
>>> (RTL_FLAG_CHECK1-6): Use RTL_FLAG_CHECK.
>>> (RTL_FLAG_CHECK7): Remove.
>>> (rtl_flag_check, check_rtl_code): New helper functions.
>>>
>>> Index: gcc/rtl.h
>>> ===================================================================
>>> --- gcc/rtl.h (revision 276484)
>>> +++ gcc/rtl.h (working copy)
>>> @@ -1249,66 +1249,18 @@ extern void rtvec_check_failed_bounds (const_rtvec
>>> #define RTX_FLAG(RTX, FLAG) ((RTX)->FLAG)
>>>
>>> #if defined ENABLE_RTL_FLAG_CHECKING && (GCC_VERSION >= 2007)
>>> -#define RTL_FLAG_CHECK1(NAME, RTX, C1) __extension__
>>> \
>>> -({ __typeof (RTX) const _rtx = (RTX);
>>> \
>>> - if (GET_CODE (_rtx) != C1)
>>> \
>>> - rtl_check_failed_flag (NAME, _rtx, __FILE__, __LINE__,
>>> \
>>> - __FUNCTION__); \
>>> - _rtx; })
>>> +#define RTL_FLAG_CHECK(NAME, RTX, ...)
>>> \
>>> + ((__typeof (&*(RTX))) rtl_flag_check (check_rtl_code<__VA_ARGS__>,
>>> \
>>> + NAME, RTX, __FILE__, __LINE__, \
>>> + __FUNCTION__))
>>
>> Yet another comment on this one, sorry, but why not just make
>> rtl_flag_check a template function instead of defining two versions
>> of it and using __typeof? Is the idea is to make sure that GET_CODE
>> is only applied to genuine rtxes rather than some random structure
>> with a field called "code"? If so, I think we should do that in
>> GET_CODE itself, and do it regardless of whether rtl checking is
>> enabled. E.g. something like:
>>
>
> Actually I wanted to do it with a template, and invoke it using __typeof(RTX).
>
> BUT with that I ran into a lmitation of the template vs. block statements
> See PR#91803: We cannot instantiate a template on the
> type of a statement expression, only when the expression is completely
> written down without statement expressions.
>
> So even if we remove that limitation, it will be impossible to use
> templates dependent on __typeof(statement-expressions) as we need
> to bootstrap from rather old gcc versions.
>
> However if we are able to replace all statement-expressions in
> rtl.h then it will probably be possible to use a template here,
> without that type cast.
>
> I still need a const and a non-const version of that function because
> otherwise the -Werror=cast-qual warning will kill me.
>
>
>> #define GET_CODE(RTX) ((rtx_code) static_cast<const_rtx> (RTX)->code)
>>
>> (Wonder how much code that will break :-))
>>
>> And if we do use templates instead of const_rtx/rtx variants,
>> it might be simpler to keep the checking and assert together:
>>
>> #define RTL_FLAG_CHECK(NAME, RTX, ...) \
>> (rtl_flag_check<__VA_ARGS__> (NAME, RTX, __FILE__, __LINE__, \
>> __FUNCTION__))
>>
>> template<rtx_code C1, typename T>
>> inline T
>> rtl_flag_check (const char *name, T rtl, const char *file, int line,
>> const char *func)
>> {
>> if (GET_CODE (rtl) != C1)
>> rtl_check_failed_flag (name, rtl, file, line, func);
>> return rtl;
>> }
>>
>> ...etc...
>>
>
> I somehow expected the typename T need to be given int the template arguments
> like rtl_flag_check<__typeof(RTX), __VA_ARGS__> (NAME, RTX,
>
> Could be that there is way to make that work without __typeof ?
Yeah, the typename will be deduced automatically in the example above.
I don't think we need __typeof here.
Richard
