Re: [PATCH][committed] Fix ICE in maybe_record_trace_start
On 02/23/2018 04:59 PM, Jeff Law wrote: On 02/22/2018 03:59 AM, Tom de Vries wrote: On 02/12/2018 07:32 PM, Jeff Law wrote: diff --git a/gcc/testsuite/gcc.c-torture/compile/regs-arg-size.c b/gcc/testsuite/gcc.c-torture/compile/regs-arg-size.c new file mode 100644 index 000..0ca0b9f034b --- /dev/null +++ b/gcc/testsuite/gcc.c-torture/compile/regs-arg-size.c @@ -0,0 +1,36 @@ +int foo; +typedef long unsigned int size_t; +typedef short unsigned int wchar_t; +struct tm +{ + int tm_mday; + int tm_mon; + int tm_year; +}; +size_t +__strftime (wchar_t * s, size_t maxsize, const wchar_t * format, const struct tm *tim_p) +{ + size_t count = 0; + int len = 0; + size_t i, ctloclen; + unsigned long width; + { + if (foo) + { + { + wchar_t *fmt = L"%s%.*d"; + len = swprintf (&s[count], maxsize, fmt, "-", width, 0); + } + if ((count) >= maxsize) + return 0; + } + else + { + len = + swprintf (&s[count], maxsize - count, L"%.2d/%.2d/%.2d", 42, 99, 0); + if ((count) >= maxsize) + return 0; + + } + } +} Hi, when compiling this test for nvptx, the missing declaration for swprintf results in this default declaration in the .s file based on the arguments of the first call: ... .extern .func (.param.u32 %value_out) swprintf (.param.u64 %in_ar0, .param.u64 %in_ar1, .param.u64 %in_ar2, .param.u64 %in_ar3, .param.u64 %in_ar4, .param.u32 %in_ar5); ... and this error message when ptxas process the second call: ... ptxas regs-arg-size.o, line 97; error : Type or alignment of argument does not match formal parameter '%in_ar3' ptxas regs-arg-size.o, line 97; error : Type or alignment of argument does not match formal parameter '%in_ar4' ptxas fatal : Ptx assembly aborted due to errors nvptx-as: ptxas returned 255 exit status ... When adding a declaration in the source like so: ... diff --git a/gcc/testsuite/gcc.c-torture/compile/regs-arg-size.c b/gcc/testsuite/gcc.c-torture/compile/regs-arg-size.c index 0ca0b9f034b..81943a2c459 100644 --- a/gcc/testsuite/gcc.c-torture/compile/regs-arg-size.c +++ b/gcc/testsuite/gcc.c-torture/compile/regs-arg-size.c @@ -1,6 +1,7 @@ int foo; typedef long unsigned int size_t; typedef short unsigned int wchar_t; +extern int swprintf(wchar_t *wcs, size_t maxlen, const wchar_t *format, ...); struct tm { int tm_mday; ... the declaration changes to: ... .extern .func (.param.u32 %value_out) swprintf (.param.u64 %in_ar0, .param.u64 %in_ar1, .param.u64 %in_ar2, .param.u64 %in_ar3); ... and test test-case passes. Is it ok to update the test-case like this, or should it require effective target 'untyped_assembly' (which is false for nvptx). It's OK to update the test in either way -- I don't think either change would compromise the test. Adding the prototype seems like the better choice to me. Done. I need to remember that PTX (and the PA) are much more sensitive to these issues than any other port and once a testcase minimization is complete to go back and make sure we have suitable prototypes. I think the root cause here is that compile.exp uses "-w". Without that, a warning triggers: ... regs-arg-size.c:22:10: warning: implicit declaration of function 'swprintf' [-Wimplicit-function-declaration]... ... and the test for excess errors fails. Thanks, - Tom [testsuite] Add missing function decl to regs-arg-size.c 2018-02-26 Tom de Vries * gcc.c-torture/compile/regs-arg-size.c (swprintf): Declare. --- gcc/testsuite/gcc.c-torture/compile/regs-arg-size.c | 1 + 1 file changed, 1 insertion(+) diff --git a/gcc/testsuite/gcc.c-torture/compile/regs-arg-size.c b/gcc/testsuite/gcc.c-torture/compile/regs-arg-size.c index 0ca0b9f..f5f0111 100644 --- a/gcc/testsuite/gcc.c-torture/compile/regs-arg-size.c +++ b/gcc/testsuite/gcc.c-torture/compile/regs-arg-size.c @@ -1,6 +1,7 @@ int foo; typedef long unsigned int size_t; typedef short unsigned int wchar_t; +extern int swprintf (wchar_t *wcs, size_t maxlen, const wchar_t *format, ...); struct tm { int tm_mday;
Re: [PATCH][committed] Fix ICE in maybe_record_trace_start
On 02/22/2018 03:59 AM, Tom de Vries wrote: > On 02/12/2018 07:32 PM, Jeff Law wrote: >> diff --git a/gcc/testsuite/gcc.c-torture/compile/regs-arg-size.c >> b/gcc/testsuite/gcc.c-torture/compile/regs-arg-size.c >> new file mode 100644 >> index 000..0ca0b9f034b >> --- /dev/null >> +++ b/gcc/testsuite/gcc.c-torture/compile/regs-arg-size.c >> @@ -0,0 +1,36 @@ >> +int foo; >> +typedef long unsigned int size_t; >> +typedef short unsigned int wchar_t; >> +struct tm >> +{ >> + int tm_mday; >> + int tm_mon; >> + int tm_year; >> +}; >> +size_t >> +__strftime (wchar_t * s, size_t maxsize, const wchar_t * format, >> const struct tm *tim_p) >> +{ >> + size_t count = 0; >> + int len = 0; >> + size_t i, ctloclen; >> + unsigned long width; >> + { >> + if (foo) >> + { >> + { >> + wchar_t *fmt = L"%s%.*d"; >> + len = swprintf (&s[count], maxsize, fmt, "-", width, 0); >> + } >> + if ((count) >= maxsize) >> + return 0; >> + } >> + else >> + { >> + len = >> + swprintf (&s[count], maxsize - count, L"%.2d/%.2d/%.2d", 42, >> 99, 0); >> + if ((count) >= maxsize) >> + return 0; >> + >> + } >> + } >> +} > > Hi, > > when compiling this test for nvptx, the missing declaration for swprintf > results in this default declaration in the .s file based on the > arguments of the first call: > ... > .extern .func (.param.u32 %value_out) swprintf (.param.u64 > %in_ar0, .param.u64 %in_ar1, .param.u64 %in_ar2, .param.u64 %in_ar3, > .param.u64 %in_ar4, .param.u32 %in_ar5); > ... > > and this error message when ptxas process the second call: > ... > ptxas regs-arg-size.o, line 97; error : Type or alignment of argument > does not match formal parameter '%in_ar3' > ptxas regs-arg-size.o, line 97; error : Type or alignment of argument > does not match formal parameter '%in_ar4' > ptxas fatal : Ptx assembly aborted due to errors > nvptx-as: ptxas returned 255 exit status > ... > > When adding a declaration in the source like so: > ... > diff --git a/gcc/testsuite/gcc.c-torture/compile/regs-arg-size.c > b/gcc/testsuite/gcc.c-torture/compile/regs-arg-size.c > index 0ca0b9f034b..81943a2c459 100644 > --- a/gcc/testsuite/gcc.c-torture/compile/regs-arg-size.c > +++ b/gcc/testsuite/gcc.c-torture/compile/regs-arg-size.c > @@ -1,6 +1,7 @@ > int foo; > typedef long unsigned int size_t; > typedef short unsigned int wchar_t; > +extern int swprintf(wchar_t *wcs, size_t maxlen, const wchar_t *format, > ...); > struct tm > { > int tm_mday; > ... > > the declaration changes to: > ... > .extern .func (.param.u32 %value_out) swprintf (.param.u64 %in_ar0, > .param.u64 %in_ar1, .param.u64 %in_ar2, .param.u64 %in_ar3); > ... > and test test-case passes. > > Is it ok to update the test-case like this, or should it require > effective target 'untyped_assembly' (which is false for nvptx). It's OK to update the test in either way -- I don't think either change would compromise the test. Adding the prototype seems like the better choice to me. I need to remember that PTX (and the PA) are much more sensitive to these issues than any other port and once a testcase minimization is complete to go back and make sure we have suitable prototypes. jeff
Re: [PATCH][committed] Fix ICE in maybe_record_trace_start
On 02/12/2018 07:32 PM, Jeff Law wrote: diff --git a/gcc/testsuite/gcc.c-torture/compile/regs-arg-size.c b/gcc/testsuite/gcc.c-torture/compile/regs-arg-size.c new file mode 100644 index 000..0ca0b9f034b --- /dev/null +++ b/gcc/testsuite/gcc.c-torture/compile/regs-arg-size.c @@ -0,0 +1,36 @@ +int foo; +typedef long unsigned int size_t; +typedef short unsigned int wchar_t; +struct tm +{ + int tm_mday; + int tm_mon; + int tm_year; +}; +size_t +__strftime (wchar_t * s, size_t maxsize, const wchar_t * format, const struct tm *tim_p) +{ + size_t count = 0; + int len = 0; + size_t i, ctloclen; + unsigned long width; + { +if (foo) + { + { + wchar_t *fmt = L"%s%.*d"; + len = swprintf (&s[count], maxsize, fmt, "-", width, 0); + } + if ((count) >= maxsize) + return 0; + } +else + { + len = + swprintf (&s[count], maxsize - count, L"%.2d/%.2d/%.2d", 42, 99, 0); + if ((count) >= maxsize) + return 0; + + } + } +} Hi, when compiling this test for nvptx, the missing declaration for swprintf results in this default declaration in the .s file based on the arguments of the first call: ... .extern .func (.param.u32 %value_out) swprintf (.param.u64 %in_ar0, .param.u64 %in_ar1, .param.u64 %in_ar2, .param.u64 %in_ar3, .param.u64 %in_ar4, .param.u32 %in_ar5); ... and this error message when ptxas process the second call: ... ptxas regs-arg-size.o, line 97; error : Type or alignment of argument does not match formal parameter '%in_ar3' ptxas regs-arg-size.o, line 97; error : Type or alignment of argument does not match formal parameter '%in_ar4' ptxas fatal : Ptx assembly aborted due to errors nvptx-as: ptxas returned 255 exit status ... When adding a declaration in the source like so: ... diff --git a/gcc/testsuite/gcc.c-torture/compile/regs-arg-size.c b/gcc/testsuite/gcc.c-torture/compile/regs-arg-size.c index 0ca0b9f034b..81943a2c459 100644 --- a/gcc/testsuite/gcc.c-torture/compile/regs-arg-size.c +++ b/gcc/testsuite/gcc.c-torture/compile/regs-arg-size.c @@ -1,6 +1,7 @@ int foo; typedef long unsigned int size_t; typedef short unsigned int wchar_t; +extern int swprintf(wchar_t *wcs, size_t maxlen, const wchar_t *format, ...); struct tm { int tm_mday; ... the declaration changes to: ... .extern .func (.param.u32 %value_out) swprintf (.param.u64 %in_ar0, .param.u64 %in_ar1, .param.u64 %in_ar2, .param.u64 %in_ar3); ... and test test-case passes. Is it ok to update the test-case like this, or should it require effective target 'untyped_assembly' (which is false for nvptx). Thanks, - Tom
[PATCH][committed] Fix ICE in maybe_record_trace_start
This was something my tester was tripping over on h8-elf. I was hoping it was going to fix the similar ICEs for the SH port, but alas those are different. The fundamental problem is generic code generated something like this: (set (temp) (plus (stack_pointer_rtx) (const_int)) (set (stack_pointer_rtx) (temp)) REG_ARGS_SIZE note The backward propagation step in cse.c turns the first insn into: (set (stack_pointer_rtx) (plus (stack_pointer_rtx) (const_int)) And the second insn gets deleted, losing the REG_ARGS_SIZE note. We then cross jump the tail of that block with the tail of another block which has REG_ARGS_SIZE notes that do not get deleted. The net result is at the commonized tail we have two paths which different notions of REG_ARGS_SIZE and thus different CFIs, triggering the ICE. The most sensible way to fix this is to move the REG_ARGS_SIZE note during the backward propagation step in cse.c That allows the H8 port to build libgcc/newlib across all its multilib variants. I've also bootstrapped and regression tested on x86_64-linux-gnu, though I doubt it really got exercised there. Installing on the trunk. Now back to the SH, rx and mips problems with maybe_record_trace_start. Jeff diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 5a264391268..d5913d0a7db 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,8 @@ +2018-02-12 Jeff Law + + * cse.c (try_back_substitute_reg): Move any REG_ARGS_SIZE note when + successfully back substituting a reg. + 2018-02-12 Richard Biener PR tree-optimization/84037 diff --git a/gcc/cse.c b/gcc/cse.c index 825b0bd8989..a73a771041a 100644 --- a/gcc/cse.c +++ b/gcc/cse.c @@ -4256,6 +4256,15 @@ try_back_substitute_reg (rtx set, rtx_insn *insn) && (reg_mentioned_p (dest, XEXP (note, 0)) || rtx_equal_p (src, XEXP (note, 0 remove_note (insn, note); + + /* If INSN has a REG_ARGS_SIZE note, move it to PREV. */ + note = find_reg_note (insn, REG_ARGS_SIZE, NULL_RTX); + if (note != 0) + { + remove_note (insn, note); + gcc_assert (!find_reg_note (prev, REG_ARGS_SIZE, NULL_RTX)); + set_unique_reg_note (prev, REG_ARGS_SIZE, XEXP (note, 0)); + } } } } diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index dba0bedb7cf..8f22a65c7bb 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,7 @@ +2018-02-12 Jeff Law + + * gcc.c-torture/compile/reg-args-size.c: New test. + 2018-02-12 Carl Love * gcc.target/powerpc/builtins-4-runnable.c (main): Move int128 and diff --git a/gcc/testsuite/gcc.c-torture/compile/regs-arg-size.c b/gcc/testsuite/gcc.c-torture/compile/regs-arg-size.c new file mode 100644 index 000..0ca0b9f034b --- /dev/null +++ b/gcc/testsuite/gcc.c-torture/compile/regs-arg-size.c @@ -0,0 +1,36 @@ +int foo; +typedef long unsigned int size_t; +typedef short unsigned int wchar_t; +struct tm +{ + int tm_mday; + int tm_mon; + int tm_year; +}; +size_t +__strftime (wchar_t * s, size_t maxsize, const wchar_t * format, const struct tm *tim_p) +{ + size_t count = 0; + int len = 0; + size_t i, ctloclen; + unsigned long width; + { +if (foo) + { + { + wchar_t *fmt = L"%s%.*d"; + len = swprintf (&s[count], maxsize, fmt, "-", width, 0); + } + if ((count) >= maxsize) + return 0; + } +else + { + len = + swprintf (&s[count], maxsize - count, L"%.2d/%.2d/%.2d", 42, 99, 0); + if ((count) >= maxsize) + return 0; + + } + } +}