* [PATCH][committed] Fix ICE in maybe_record_trace_start
@ 2018-02-12 18:32 Jeff Law
2018-02-22 11:00 ` Tom de Vries
0 siblings, 1 reply; 4+ messages in thread
From: Jeff Law @ 2018-02-12 18:32 UTC (permalink / raw)
To: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 1225 bytes --]
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
[-- Attachment #2: P --]
[-- Type: text/plain, Size: 2330 bytes --]
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 <law@redhat.com>
+
+ * cse.c (try_back_substitute_reg): Move any REG_ARGS_SIZE note when
+ successfully back substituting a reg.
+
2018-02-12 Richard Biener <rguenther@suse.de>
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 <law@redhat.com>
+
+ * gcc.c-torture/compile/reg-args-size.c: New test.
+
2018-02-12 Carl Love <cel@us.ibm.com>
* 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 00000000000..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;
+
+ }
+ }
+}
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH][committed] Fix ICE in maybe_record_trace_start
2018-02-12 18:32 [PATCH][committed] Fix ICE in maybe_record_trace_start Jeff Law
@ 2018-02-22 11:00 ` Tom de Vries
2018-02-23 15:59 ` Jeff Law
0 siblings, 1 reply; 4+ messages in thread
From: Tom de Vries @ 2018-02-22 11:00 UTC (permalink / raw)
To: Jeff Law, gcc-patches
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 00000000000..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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH][committed] Fix ICE in maybe_record_trace_start
2018-02-22 11:00 ` Tom de Vries
@ 2018-02-23 15:59 ` Jeff Law
2018-02-26 16:09 ` Tom de Vries
0 siblings, 1 reply; 4+ messages in thread
From: Jeff Law @ 2018-02-23 15:59 UTC (permalink / raw)
To: Tom de Vries, gcc-patches
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 00000000000..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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH][committed] Fix ICE in maybe_record_trace_start
2018-02-23 15:59 ` Jeff Law
@ 2018-02-26 16:09 ` Tom de Vries
0 siblings, 0 replies; 4+ messages in thread
From: Tom de Vries @ 2018-02-26 16:09 UTC (permalink / raw)
To: Jeff Law; +Cc: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 3955 bytes --]
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 00000000000..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
[-- Attachment #2: 0001-testsuite-Add-missing-function-decl-to-regs-arg-size.c.patch --]
[-- Type: text/x-patch, Size: 740 bytes --]
[testsuite] Add missing function decl to regs-arg-size.c
2018-02-26 Tom de Vries <tom@codesourcery.com>
* 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;
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-02-26 16:09 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-12 18:32 [PATCH][committed] Fix ICE in maybe_record_trace_start Jeff Law
2018-02-22 11:00 ` Tom de Vries
2018-02-23 15:59 ` Jeff Law
2018-02-26 16:09 ` Tom de Vries
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).