On 10/4/19 2:38 PM, Richard Sandiford wrote:
> Bernd Edlinger <[email protected]> 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 <[email protected]>
>>
>> * 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 ?
> which could also address some of the fears around cost.
>
> Personally I'm not fussed about keeping the checking and assert
> together though, just a suggestion.
>
Okay I'll consider that.
BTW, I noticed that the patch is still incomplete and breaks
with --enable-checking=yes,rtl :-/
I did not try that before, just did it after Jakub mentioned it.
Thanks
Bernd.
