On 10/5/19 9:24 AM, Jakub Jelinek wrote:
> On Sat, Oct 05, 2019 at 06:12:37AM +0000, Bernd Edlinger wrote:
>> On 10/3/19 5:25 PM, Jakub Jelinek wrote:
>>> What effect does this have on the cc1/cc1plus .text sizes?
>>
>> r276457:
>>
>> with patch, --enable-languages=all --enable-checking=yes,rtl
>> $ size gcc/cc1
>> text data bss dec hex filename
>> 35117708 50984 1388192 36556884 22dd054 gcc/cc1
>> $ size gcc/cc1plus
>> text data bss dec hex filename
>> 37871569 54640 1391936 39318145 257f281 gcc/cc1plus
>>
>> unpatched, --enable-languages=all --enable-checking=yes,rtl
>> $ size gcc/cc1
>> text data bss dec hex filename
>> 36972031 50984 1388192 38411207 24a1bc7 gcc/cc1
>> $ size gcc/cc1plus
>> text data bss dec hex filename
>> 39725980 54640 1391936 41172556 2743e4c gcc/cc1plus
>>
>> I am a bit surprised, that the patch gives smaller code. I used not the
>> original
>> Version of this patch, but the one based on Richard's suggestion.
>
> In that case, can you check if all the non-failed checks are actually
> inlined? We do want to inline the if (x->code != 123) check_failed (...);
> and don't want to inline the check_failed (...). With partial inlining, it
> might be ok if we inline the if and don't inline the actual caller of
> check_failed, but I'd worry about checks for multiple codes if we inline
> from if (x->code != 123 && x->code != 235 && x->code != 348) inline say
> just x->code != 123 or x->code != 123 && x->code != 235 and leave the rest
> in the out of line function.
>
Ah, the check functions were not inlined at all because an inline keyword was
missing,
that explains the very small text size.
So in the attached patch, I merged all checking code into one template, added
an inline keyword, and now it is inlined as expected, the code is still a bit
smaller
than the original implementation, though.
I don't know why exactly the size is smaller, but everything looks completely
correct
now (numbers are again for r276457 with attached patch):
$ size gcc/cc1
text data bss dec hex filename
36222783 50984 1388192 37661959 23ead07 gcc/cc1
$ size gcc/cc1plus
text data bss dec hex filename
38976708 54640 1391936 40423284 268cf74 gcc/cc1plus
I added a skip for all of rtl.h, since otherwise gdb would need all possible
template
invocations of rtl_check_code<> and rtl_check_bounds<> in the skip statement.
n works now, but s only in the stage1 compiler. In the stage3 compiler
s steps one line into the inlined code and then back to the called function.
But that is generally the case for all inline functions, like INSN_UID
and contains_struct_check which is in tree.h, should be skipped, but same here.
That does not happen with the statement expression since all code is on the
same line.
Is that a show stopper for this patch, or just a bug in the gdb?
>>> Does this affect debuggability of --enable-checking=yes,rtl compilers?
>>> I mean, often when we replace some macros with inlines step in GDB
>>> becomes a bigger nightmare, having to go through tons of inline frames.
>>> gdbinit.in has a lengthy list of inlines to skip in rtl.h, shouldn't this be
>>> added to that list? Not 100% sure how well it will work on rtl checking
>>> vs. non-rtl checking builds.
>>>
>>
>> I don't see a big problem here. If I type "s" in gdb it jumps to the check
>> function and the next s jumps back, adding skip instructions in gdbinit.in
>> does not seem to have any effect for me, but the debug is not that
>> uncomfortable
>> anyway.
>>
>> Interesting is that gdb also jumps in the check function when I press n.
>> That is also Independent of the gdbinit.in, seems to be a bug due to inlining
>> code from another line than the original macro, but nothing terrilby bad.
>
the issue with n is gone now.
the s works in stage1 now, but not in stage3.
Attached is an improved patch that fixes the inline issue and adds all of rtl.h
(including rhs_regno) to the exclusions.
Don't know if there is still a consensus to add -Wshadow=local or if the
resistance
is too big, then it is probably fine to keep the rtl checking code as-is.
Bernd.
> Unfortunately, for me the above two counts as terribly bad, show
> stopper here. A lot of function calls in RTL are like:
> rtx_equal_for_memref_p (XEXP (x, 0), XEXP (y, 0))
> rtx_equal_p (ENTRY_VALUE_EXP (x), ENTRY_VALUE_EXP (y))
> force_operand (XEXP (dest_mem, 0), target)
> etc. (just random examples), during debugging one is absolutely
> uninterested in stepping into the implementation of XEXP, you know what it
> means, you want to go stright into the rtx_equal_for_memref_p etc. call.
> It is already bad that one has to step through the poly_int* stuff or
> rhs_regno for REGNO (we should add rhs_regno to gdbinit.in and some of the
> poly_int* stuff too). And no, one can't use n instead of s, because then
> the whole call is skipped. b rtx_equal_for_memref_p; n works, but it is
> time consuming and one needs to delete the breakpoint again.
>
> Jakub
>
2019-10-04 Bernd Edlinger <bernd.edlin...@hotmail.de>
* gdbinit.in: Add skip file rtl.h
* rtl.h (RTL_CHECK1, RTL_CHECK2, RTL_CHECKC1, RTL_CHECKC2
RTVEC_ELT): Reimplement with inline functions.
(RTL_FLAG_CHECK): New variadic macro.
(RTL_FLAG_CHECK1-6): Use RTL_FLAG_CHECK.
(RTL_FLAG_CHECK7): Remove.
(rtvec_check): New helper function.
(rtl_check_bounds, rtl_check_code,
rtl_flag_check): New helper templates.
Index: gcc/gdbinit.in
===================================================================
--- gcc/gdbinit.in (revision 276625)
+++ gcc/gdbinit.in (working copy)
@@ -282,23 +282,7 @@ skip file line-map.h
skip file timevar.h
# Likewise, skip various inline functions in rtl.h.
-skip rtx_expr_list::next
-skip rtx_expr_list::element
-skip rtx_insn_list::next
-skip rtx_insn_list::insn
-skip rtx_sequence::len
-skip rtx_sequence::element
-skip rtx_sequence::insn
-skip INSN_UID
-skip PREV_INSN
-skip SET_PREV_INSN
-skip NEXT_INSN
-skip SET_NEXT_INSN
-skip BLOCK_FOR_INSN
-skip PATTERN
-skip INSN_LOCATION
-skip INSN_HAS_LOCATION
-skip JUMP_LABEL_AS_INSN
+skip file rtl.h
# Restore pagination to the previous state.
python if __gcc_prev_pagination: gdb.execute("set pagination on")
Index: gcc/rtl.h
===================================================================
--- gcc/rtl.h (revision 276625)
+++ gcc/rtl.h (working copy)
@@ -1070,43 +1070,17 @@ is_a_helper <rtx_note *>::test (rtx_insn *insn)
#if defined ENABLE_RTL_CHECKING && (GCC_VERSION >= 2007)
/* The bit with a star outside the statement expr and an & inside is
so that N can be evaluated only once. */
-#define RTL_CHECK1(RTX, N, C1) __extension__ \
-(*({ __typeof (RTX) const _rtx = (RTX); const int _n = (N); \
- const enum rtx_code _code = GET_CODE (_rtx); \
- if (_n < 0 || _n >= GET_RTX_LENGTH (_code)) \
- rtl_check_failed_bounds (_rtx, _n, __FILE__, __LINE__, \
- __FUNCTION__); \
- if (GET_RTX_FORMAT (_code)[_n] != C1) \
- rtl_check_failed_type1 (_rtx, _n, C1, __FILE__, __LINE__, \
- __FUNCTION__); \
- &_rtx->u.fld[_n]; }))
+#define RTL_CHECK1(RTX, N, C1) \
+ rtl_check_bounds<C1> (RTX, N, __FILE__, __LINE__, __FUNCTION__)
-#define RTL_CHECK2(RTX, N, C1, C2) __extension__ \
-(*({ __typeof (RTX) const _rtx = (RTX); const int _n = (N); \
- const enum rtx_code _code = GET_CODE (_rtx); \
- if (_n < 0 || _n >= GET_RTX_LENGTH (_code)) \
- rtl_check_failed_bounds (_rtx, _n, __FILE__, __LINE__, \
- __FUNCTION__); \
- if (GET_RTX_FORMAT (_code)[_n] != C1 \
- && GET_RTX_FORMAT (_code)[_n] != C2) \
- rtl_check_failed_type2 (_rtx, _n, C1, C2, __FILE__, __LINE__, \
- __FUNCTION__); \
- &_rtx->u.fld[_n]; }))
+#define RTL_CHECK2(RTX, N, C1, C2) \
+ rtl_check_bounds<C1, C2> (RTX, N, __FILE__, __LINE__, __FUNCTION__)
-#define RTL_CHECKC1(RTX, N, C) __extension__ \
-(*({ __typeof (RTX) const _rtx = (RTX); const int _n = (N); \
- if (GET_CODE (_rtx) != (C)) \
- rtl_check_failed_code1 (_rtx, (C), __FILE__, __LINE__, \
- __FUNCTION__); \
- &_rtx->u.fld[_n]; }))
+#define RTL_CHECKC1(RTX, N, C1) \
+ rtl_check_code<C1> (RTX, N, __FILE__, __LINE__, __FUNCTION__)
-#define RTL_CHECKC2(RTX, N, C1, C2) __extension__ \
-(*({ __typeof (RTX) const _rtx = (RTX); const int _n = (N); \
- const enum rtx_code _code = GET_CODE (_rtx); \
- if (_code != (C1) && _code != (C2)) \
- rtl_check_failed_code2 (_rtx, (C1), (C2), __FILE__, __LINE__, \
- __FUNCTION__); \
- &_rtx->u.fld[_n]; }))
+#define RTL_CHECKC2(RTX, N, C1, C2) \
+ rtl_check_code<C1, C2> (RTX, N, __FILE__, __LINE__, __FUNCTION__)
#define RTL_CHECKC3(RTX, N, C1, C2, C3) __extension__ \
(*({ __typeof (RTX) const _rtx = (RTX); const int _n = (N); \
@@ -1116,12 +1090,8 @@ is_a_helper <rtx_note *>::test (rtx_insn *insn)
__LINE__, __FUNCTION__); \
&_rtx->u.fld[_n]; }))
-#define RTVEC_ELT(RTVEC, I) __extension__ \
-(*({ __typeof (RTVEC) const _rtvec = (RTVEC); const int _i = (I); \
- if (_i < 0 || _i >= GET_NUM_ELEM (_rtvec)) \
- rtvec_check_failed_bounds (_rtvec, _i, __FILE__, __LINE__, \
- __FUNCTION__); \
- &_rtvec->elem[_i]; }))
+#define RTVEC_ELT(RTVEC, I) \
+ rtvec_check (RTVEC, I, __FILE__, __LINE__, __FUNCTION__)
#define XWINT(RTX, N) __extension__ \
(*({ __typeof (RTX) const _rtx = (RTX); const int _n = (N); \
@@ -1222,6 +1192,120 @@ extern void rtvec_check_failed_bounds (const_rtvec
const char *)
ATTRIBUTE_NORETURN ATTRIBUTE_COLD;
+static inline rtx const&
+rtvec_check (const_rtvec vec, int i,
+ const char *file, int line, const char *func)
+{
+ if (i < 0 || i >= GET_NUM_ELEM (vec))
+ rtvec_check_failed_bounds (vec, i, file, line, func);
+ return vec->elem[i];
+}
+
+static inline rtx&
+rtvec_check (rtvec vec, int i,
+ const char *file, int line, const char *func)
+{
+ if (i < 0 || i >= GET_NUM_ELEM (vec))
+ rtvec_check_failed_bounds (vec, i, file, line, func);
+ return vec->elem[i];
+}
+
+template<char C1>
+inline const rtunion&
+rtl_check_bounds (const_rtx rtl, int n,
+ const char *file, int line, const char *function)
+{
+ const enum rtx_code code = GET_CODE (rtl);
+ if (n < 0 || n >= GET_RTX_LENGTH (code))
+ rtl_check_failed_bounds (rtl, n, file, line, function);
+ if (GET_RTX_FORMAT (code)[n] != C1)
+ rtl_check_failed_type1 (rtl, n, C1, file, line, function);
+ return rtl->u.fld[n];
+}
+
+template<char C1>
+inline rtunion&
+rtl_check_bounds (rtx rtl, int n,
+ const char *file, int line, const char *function)
+{
+ const enum rtx_code code = GET_CODE (rtl);
+ if (n < 0 || n >= GET_RTX_LENGTH (code))
+ rtl_check_failed_bounds (rtl, n, file, line, function);
+ if (GET_RTX_FORMAT (code)[n] != C1)
+ rtl_check_failed_type1 (rtl, n, C1, file, line, function);
+ return rtl->u.fld[n];
+}
+
+template<char C1, char C2>
+inline const rtunion&
+rtl_check_bounds (const_rtx rtl, int n,
+ const char *file, int line, const char *function)
+{
+ const enum rtx_code code = GET_CODE (rtl);
+ if (n < 0 || n >= GET_RTX_LENGTH (code))
+ rtl_check_failed_bounds (rtl, n, file, line, function);
+ if (GET_RTX_FORMAT (code)[n] != C1
+ && GET_RTX_FORMAT (code)[n] != C2)
+ rtl_check_failed_type2 (rtl, n, C1, C2, file, line, function);
+ return rtl->u.fld[n];
+}
+
+template<char C1, char C2>
+inline rtunion&
+rtl_check_bounds (rtx rtl, int n,
+ const char *file, int line, const char *function)
+{
+ const enum rtx_code code = GET_CODE (rtl);
+ if (n < 0 || n >= GET_RTX_LENGTH (code))
+ rtl_check_failed_bounds (rtl, n, file, line, function);
+ if (GET_RTX_FORMAT (code)[n] != C1
+ && GET_RTX_FORMAT (code)[n] != C2)
+ rtl_check_failed_type2 (rtl, n, C1, C2, file, line, function);
+ return rtl->u.fld[n];
+}
+
+template<RTX_CODE C1>
+inline const rtunion&
+rtl_check_code (const_rtx rtl, int n,
+ const char *file, int line, const char *function)
+{
+ if (GET_CODE (rtl) != C1)
+ rtl_check_failed_code1 (rtl, C1, file, line, function);
+ return rtl->u.fld[n];
+}
+
+template<RTX_CODE C1>
+inline rtunion&
+rtl_check_code (rtx rtl, int n,
+ const char *file, int line, const char *function)
+{
+ if (GET_CODE (rtl) != C1)
+ rtl_check_failed_code1 (rtl, C1, file, line, function);
+ return rtl->u.fld[n];
+}
+
+template<RTX_CODE C1, RTX_CODE C2>
+inline const rtunion&
+rtl_check_code (const_rtx rtl, int n,
+ const char *file, int line, const char *function)
+{
+ if (GET_CODE (rtl) != C1
+ && GET_CODE (rtl) != C2)
+ rtl_check_failed_code2 (rtl, C1, C2, file, line, function);
+ return rtl->u.fld[n];
+}
+
+template<RTX_CODE C1, RTX_CODE C2>
+inline rtunion&
+rtl_check_code (rtx rtl, int n,
+ const char *file, int line, const char *function)
+{
+ if (GET_CODE (rtl) != C1
+ && GET_CODE (rtl) != C2)
+ rtl_check_failed_code2 (rtl, C1, C2, file, line, function);
+ return rtl->u.fld[n];
+}
+
#else /* not ENABLE_RTL_CHECKING */
#define RTL_CHECK1(RTX, N, C1) ((RTX)->u.fld[N])
@@ -1249,66 +1333,17 @@ 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, ...) \
+ rtl_flag_check<__VA_ARGS__> (NAME, RTX, __FILE__, __LINE__, \
+ __FUNCTION__)
-#define RTL_FLAG_CHECK2(NAME, RTX, C1, C2) __extension__ \
-({ __typeof (RTX) const _rtx = (RTX); \
- if (GET_CODE (_rtx) != C1 && GET_CODE(_rtx) != C2) \
- rtl_check_failed_flag (NAME,_rtx, __FILE__, __LINE__, \
- __FUNCTION__); \
- _rtx; })
+#define RTL_FLAG_CHECK1 RTL_FLAG_CHECK
+#define RTL_FLAG_CHECK2 RTL_FLAG_CHECK
+#define RTL_FLAG_CHECK3 RTL_FLAG_CHECK
+#define RTL_FLAG_CHECK4 RTL_FLAG_CHECK
+#define RTL_FLAG_CHECK5 RTL_FLAG_CHECK
+#define RTL_FLAG_CHECK6 RTL_FLAG_CHECK
-#define RTL_FLAG_CHECK3(NAME, RTX, C1, C2, C3) __extension__ \
-({ __typeof (RTX) const _rtx = (RTX); \
- if (GET_CODE (_rtx) != C1 && GET_CODE(_rtx) != C2 \
- && GET_CODE (_rtx) != C3) \
- rtl_check_failed_flag (NAME, _rtx, __FILE__, __LINE__, \
- __FUNCTION__); \
- _rtx; })
-
-#define RTL_FLAG_CHECK4(NAME, RTX, C1, C2, C3, C4) __extension__ \
-({ __typeof (RTX) const _rtx = (RTX); \
- if (GET_CODE (_rtx) != C1 && GET_CODE(_rtx) != C2 \
- && GET_CODE (_rtx) != C3 && GET_CODE(_rtx) != C4) \
- rtl_check_failed_flag (NAME, _rtx, __FILE__, __LINE__, \
- __FUNCTION__); \
- _rtx; })
-
-#define RTL_FLAG_CHECK5(NAME, RTX, C1, C2, C3, C4, C5) __extension__ \
-({ __typeof (RTX) const _rtx = (RTX); \
- if (GET_CODE (_rtx) != C1 && GET_CODE (_rtx) != C2 \
- && GET_CODE (_rtx) != C3 && GET_CODE (_rtx) != C4 \
- && GET_CODE (_rtx) != C5) \
- rtl_check_failed_flag (NAME, _rtx, __FILE__, __LINE__, \
- __FUNCTION__); \
- _rtx; })
-
-#define RTL_FLAG_CHECK6(NAME, RTX, C1, C2, C3, C4, C5, C6) \
- __extension__ \
-({ __typeof (RTX) const _rtx = (RTX); \
- if (GET_CODE (_rtx) != C1 && GET_CODE (_rtx) != C2 \
- && GET_CODE (_rtx) != C3 && GET_CODE (_rtx) != C4 \
- && GET_CODE (_rtx) != C5 && GET_CODE (_rtx) != C6) \
- rtl_check_failed_flag (NAME,_rtx, __FILE__, __LINE__, \
- __FUNCTION__); \
- _rtx; })
-
-#define RTL_FLAG_CHECK7(NAME, RTX, C1, C2, C3, C4, C5, C6, C7) \
- __extension__ \
-({ __typeof (RTX) const _rtx = (RTX); \
- if (GET_CODE (_rtx) != C1 && GET_CODE (_rtx) != C2 \
- && GET_CODE (_rtx) != C3 && GET_CODE (_rtx) != C4 \
- && GET_CODE (_rtx) != C5 && GET_CODE (_rtx) != C6 \
- && GET_CODE (_rtx) != C7) \
- rtl_check_failed_flag (NAME, _rtx, __FILE__, __LINE__, \
- __FUNCTION__); \
- _rtx; })
-
#define RTL_INSN_CHAIN_FLAG_CHECK(NAME, RTX) \
__extension__ \
({ __typeof (RTX) const _rtx = (RTX); \
@@ -1322,6 +1357,75 @@ extern void rtl_check_failed_flag (const char *, c
ATTRIBUTE_NORETURN ATTRIBUTE_COLD
;
+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;
+}
+
+template<RTX_CODE C1, RTX_CODE C2, 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 && GET_CODE (rtl) != C2)
+ rtl_check_failed_flag (name, rtl, file, line, func);
+ return rtl;
+}
+
+template<RTX_CODE C1, RTX_CODE C2, RTX_CODE C3, 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 && GET_CODE (rtl) != C2
+ && GET_CODE (rtl) != C3)
+ rtl_check_failed_flag (name, rtl, file, line, func);
+ return rtl;
+}
+
+template<RTX_CODE C1, RTX_CODE C2, RTX_CODE C3,
+ RTX_CODE C4, 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 && GET_CODE (rtl) != C2
+ && GET_CODE (rtl) != C3 && GET_CODE (rtl) != C4)
+ rtl_check_failed_flag (name, rtl, file, line, func);
+ return rtl;
+}
+
+template<RTX_CODE C1, RTX_CODE C2, RTX_CODE C3,
+ RTX_CODE C4, RTX_CODE C5, 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 && GET_CODE (rtl) != C2
+ && GET_CODE (rtl) != C3 && GET_CODE (rtl) != C4
+ && GET_CODE (rtl) != C5)
+ rtl_check_failed_flag (name, rtl, file, line, func);
+ return rtl;
+}
+
+template<RTX_CODE C1, RTX_CODE C2, RTX_CODE C3,
+ RTX_CODE C4, RTX_CODE C5, RTX_CODE C6, 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 && GET_CODE (rtl) != C2
+ && GET_CODE (rtl) != C3 && GET_CODE (rtl) != C4
+ && GET_CODE (rtl) != C5 && GET_CODE (rtl) != C6)
+ rtl_check_failed_flag (name, rtl, file, line, func);
+ return rtl;
+}
+
#else /* not ENABLE_RTL_FLAG_CHECKING */
#define RTL_FLAG_CHECK1(NAME, RTX, C1) (RTX)
@@ -1330,7 +1434,6 @@ extern void rtl_check_failed_flag (const char *, c
#define RTL_FLAG_CHECK4(NAME, RTX, C1, C2, C3, C4) (RTX)
#define RTL_FLAG_CHECK5(NAME, RTX, C1, C2, C3, C4, C5) (RTX)
#define RTL_FLAG_CHECK6(NAME, RTX, C1, C2, C3, C4, C5, C6) (RTX)
-#define RTL_FLAG_CHECK7(NAME, RTX, C1, C2, C3, C4, C5, C6, C7) (RTX)
#define RTL_INSN_CHAIN_FLAG_CHECK(NAME, RTX) (RTX)
#endif
