Re: patch to fix PR65648

[Yvan Roux <yvan dot roux at linaro dot org>]

Hi Jakub,

On 7 April 2015 at 17:51, Jakub Jelinek <jakub@redhat.com> wrote:
> On Tue, Apr 07, 2015 at 11:01:59AM -0400, Vladimir Makarov wrote:
>> 2015-04-07  Vladimir Makarov  <vmakarov@redhat.com>
>>
>>         PR target/65678
>>         * lra-remat.c (do_remat): Process input and non-input insn
>>         registers separately.
>
> Don't have a quick access to arm box right now (without waiting for it to be
> installed etc.), but using a cross-compiler I can at least reproduce
> that your patch changes also:
>
> /* PR target/65648 */
>
> int a = 0, *b = 0, c = 0;
> static int d = 0;
> short e = 1;
> static long long f = 0;
> long long *i = &f;
> unsigned char j = 0;
>
> __attribute__((noinline, noclone)) void
> foo (int x, int *y)
> {
>   asm volatile ("" : : "r" (x), "r" (y) : "memory");
> }
>
> __attribute__((noinline, noclone)) void
> bar (const char *x, long long y)
> {
>   asm volatile ("" : : "r" (x), "r" (&y) : "memory");
>   if (y != 0)
>     __builtin_abort ();
> }
>
> int
> main ()
> {
>   int k = 0;
>   b = &k;
>   j = (!a) - (c <= e);
>   *i = j;
>   foo (a, &k);
>   bar ("", f);
>   return 0;
> }
>
> so, if anybody can confirm this testcase aborts with -march=armv6-m -mthumb -Os
> before Vlad's patch and doesn't abort afterwards, perhaps just sticking
> that into gcc.c-torture/execute/pr65648.c would be sufficient.
> Or, if people don't regularly test with -march=armv6-m -mthumb combination,
> perhaps put it into gcc.c-torture/execute/pr65648.c as is and
> add another gcc.target/arm/pr65648.c testcase that will #include this one,
> and use the right dg-options / dg-skip-if or dg-require-effective-target etc.
> for it to trigger.  As the testcase uses uninitialized r3 in the wrong case,
> I wonder if the usual _start initializes r3 to some value that triggers the abort
> without the fix; if not, perhaps it needs to be in another function and the caller
> should somehow attempt to set l3 somehow (pass arguments to another function etc.)
> to a value that will trigger the abort.

validation is ongoing, but here is my attempt to add this testcase,
does it look correct (it's the first time I use that kind of include
in testsuite)

Cheers,
Yvan

Attachment: pr65648-testcase.diff
Description: Text document