Bug 90075 - [7/8 Regression] [AArch64] ICE during RTL pass when member of union passed to copysignf
Summary: [7/8 Regression] [AArch64] ICE during RTL pass when member of union passed to...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: middle-end (show other bugs)
Version: 8.3.0
: P2 normal
Target Milestone: 7.5
Assignee: Srinath Parvathaneni
URL:
Keywords: ice-on-valid-code
: 99012 (view as bug list)
Depends on:
Blocks:
 
Reported: 2019-04-12 20:56 UTC by Jonathan Helmus
Modified: 2021-02-09 07:27 UTC (History)
3 users (show)

See Also:
Host:
Target: aarch64
Build:
Known to work: 9.0
Known to fail: 6.5.0, 7.4.1, 8.3.1
Last reconfirmed: 2019-04-15 00:00:00


Attachments
preprocessed source of code mentioned in report (3.18 KB, text/plain)
2019-04-12 20:56 UTC, Jonathan Helmus
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Helmus 2019-04-12 20:56:03 UTC
Created attachment 46156 [details]
preprocessed source of code mentioned in report

GCC 8.3.0 as well as other releases in the 8.x and 7.x line fail with an internal compiler error when compiling the following example with -fPIC and optimization enabled.

#include <math.h>

typedef struct { float one, two; } twofloats;

float
bug(twofloats tf)
{
    float f1, f2;
    union {
        twofloats tfloats; 
        float arr[2]; 
    } utfloats;
    utfloats.tfloats = tf; 
    f1 = utfloats.arr[1];
    f2 = copysignf(0, f1);
    return f2;
}

Using the gcc 8.3.0 docker image on a AArch64 system (RockChip rk3399) running Ubunutu 18.04 produces the following results:

root@37bc4cc2fc49:/io# gcc --version
gcc (GCC) 8.3.0
Copyright (C) 2018 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

root@37bc4cc2fc49:/io# gcc -Wall -Wextra -O1 -fPIC -c mre.c
during RTL pass: expand
mre.c: In function 'bug':
mre.c:15:10: internal compiler error: Segmentation fault
     f2 = copysignf(0, f1);
          ^~~~~~~~~~~~~~~~
Please submit a full bug report,
with preprocessed source if appropriate.
See <https://gcc.gnu.org/bugs/> for instructions.


Removing the -O1 or -fPIC arguments allow for successful compilation.

The preprocessed source is attached.

This issue was initially detected while attempting to build NumPy 1.16.3, a popular Python library for Numerical computation, for AArch64, https://github.com/conda-forge/numpy-feedstock/pull/138.
Comment 1 ktkachov 2019-04-15 07:57:49 UTC
Confirmed on GCC 8 branch as well as 7 and 6 (though 6 is not longer maintained). Trunk doesn't ICE though I don't know if it has been fixed or just made latent
Comment 2 Ramana Radhakrishnan 2019-04-23 13:19:40 UTC
I'll take a look.
Comment 3 Ramana Radhakrishnan 2019-04-23 13:40:59 UTC
Seems to have been "fixed" by the commit to fix PR87369,

Richard, is this something to backport ? Prima-facie , it appears not and we will need an appropriate fix for the release branches.
Comment 4 Richard Earnshaw 2019-04-23 14:57:27 UTC
(In reply to Ramana Radhakrishnan from comment #3)
> Seems to have been "fixed" by the commit to fix PR87369,
> 
> Richard, is this something to backport ? Prima-facie , it appears not and we
> will need an appropriate fix for the release branches.

Given that the patch for PR87369 eliminates the ICE, it's probably preferable for backporting to a separate patch that is only used on the release branches.  That patch has now been soaking on trunk for a while now, so is likely to be pretty safe.

I am a bit worried however, that the patch papers over a likely trunk ICE that isn't really fixed.  It would be nice to investigate further if some additional mitigation is warranted.
Comment 5 ramana.radhakrishnan@arm.com 2019-04-24 08:34:21 UTC
The main reason for the ICE is this bit of code here.

GCC-8 and earlier have this bit of code in the expansion for copysignsf3

..
  rtx op2 = lowpart_subreg (V2SFmode, operands[2], SFmode);
..

which looks quite a bit different to the approach taken with copysigndf3 until your rewrite.

This gets an input in operands[2] which is subreg:SF (reg:SI 100) and then lower_subreg->simplify_gen_subreg seems to get into a tangle that it can't get out of.   That causes simplify_gen_subreg to get confused and that ends up returning a Null pointer as it is unable to do the conversion - we then don't check and thus ICE with a null pointer error.

Having looked at it again this morning my reaction is that while there be dragons in subreg's of vector modes and such mode casting, the newer rewrite seems reasonable and is not papering over any underlying modes.

For the release branches, I think backporting your patch (and any followups , do you remember any ?) should be fine and we should just do it ./

Ramana




________________________________________
From: rearnsha at gcc dot gnu.org <gcc-bugzilla@gcc.gnu.org>
Sent: 23 April 2019 15:57
To: ramana@gcc.gnu.org
Subject: [Bug middle-end/90075] [7/8 Regression] [AArch64] ICE during RTL pass when member of union passed to copysignf

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90075

--- Comment #4 from Richard Earnshaw <rearnsha at gcc dot gnu.org> ---
(In reply to Ramana Radhakrishnan from comment #3)
> Seems to have been "fixed" by the commit to fix PR87369,
>
> Richard, is this something to backport ? Prima-facie , it appears not and we
> will need an appropriate fix for the release branches.

Given that the patch for PR87369 eliminates the ICE, it's probably preferable
for backporting to a separate patch that is only used on the release branches.
That patch has now been soaking on trunk for a while now, so is likely to be
pretty safe.

I am a bit worried however, that the patch papers over a likely trunk ICE that
isn't really fixed.  It would be nice to investigate further if some additional
mitigation is warranted.

--
You are receiving this mail because:
You are on the CC list for the bug.
You are the assignee for the bug.
Comment 6 Richard Earnshaw 2019-04-24 10:07:02 UTC
(In reply to ramana.radhakrishnan@arm.com from comment #5)
> For the release branches, I think backporting your patch (and any followups
> , do you remember any ?) should be fine and we should just do it ./

I don't recall any.  Certainly none are recorded in the PR.
Comment 7 Richard Earnshaw 2019-04-30 09:26:02 UTC
Author: rearnsha
Date: Tue Apr 30 09:25:31 2019
New Revision: 270683

URL: https://gcc.gnu.org/viewcvs?rev=270683&root=gcc&view=rev
Log:
PR target/90075 Prefer bsl/bit/bif for copysignf. (backport GCC-8)

This patch is to fix the ICE caused in expand pattern of copysignf 
builtin. This is a back port to r267019 of trunk.

gcc:

2019-04-29  Srinath Parvathaneni  <srinath.parvathaneni@arm.com>

	Backport from mainline
	2018-12-11  Richard Earnshaw <Richard.Earnshaw@arm.com>

	PR target/37369
	* config/aarch64/iterators.md (sizem1): Add sizes for
	SFmode and DFmode.
	(Vbtype): Add SFmode mapping.
	* config/aarch64/aarch64.md (copysigndf3, copysignsf3): Delete.
	(copysign<GPF:mode>3): New expand pattern.
	(copysign<GPF:mode>3_insn): New insn pattern.

testsuite:

2019-04-29  Srinath Parvathaneni  <srinath.parvathaneni@arm.com>

	PR target/90075
	* gcc.target/aarch64/pr90075.c: New test.


Added:
    branches/gcc-8-branch/gcc/testsuite/gcc.target/aarch64/pr90075.c
Modified:
    branches/gcc-8-branch/gcc/ChangeLog
    branches/gcc-8-branch/gcc/config/aarch64/aarch64.md
    branches/gcc-8-branch/gcc/config/aarch64/iterators.md
    branches/gcc-8-branch/gcc/testsuite/ChangeLog
Comment 8 Richard Earnshaw 2019-04-30 09:31:35 UTC
Author: rearnsha
Date: Tue Apr 30 09:31:04 2019
New Revision: 270684

URL: https://gcc.gnu.org/viewcvs?rev=270684&root=gcc&view=rev
Log:
PR target/90075 Prefer bsl/bit/bif for copysignf. (backport GCC-7)

This patch is to fix the ICE caused by expand pattern of copysignf 
builtin. This is a back port to r267019 of trunk.

gcc:

2019-04-30  Srinath Parvathaneni  <srinath.parvathaneni@arm.com>

	PR target/90075
	* config/aarch64/iterators.md (V_INT_EQUIV): Add mode for
	integer equivalent of floating point values.

	Backport from mainline
	2018-12-11  Richard Earnshaw  <Richard.Earnshaw@arm.com>

	PR target/37369
	* config/aarch64/iterators.md (sizem1): Add sizes for
	SFmode and DFmode.
	(Vbtype): Add SFmode mapping.
	* config/aarch64/aarch64.md (copysigndf3, copysignsf3): Delete.
	(copysign<GPF:mode>3): New expand pattern.
	(copysign<GPF:mode>3_insn): New insn pattern.

testsuite:

2019-04-30  Srinath Parvathaneni  <srinath.parvathaneni@arm.com>

	PR target/90075
	* gcc.target/aarch64/pr90075.c: New test.

Added:
    branches/gcc-7-branch/gcc/testsuite/gcc.target/aarch64/pr90075.c
Modified:
    branches/gcc-7-branch/gcc/ChangeLog
    branches/gcc-7-branch/gcc/config/aarch64/aarch64.md
    branches/gcc-7-branch/gcc/config/aarch64/iterators.md
    branches/gcc-7-branch/gcc/testsuite/ChangeLog
Comment 9 Richard Earnshaw 2019-04-30 09:32:35 UTC
Fixed on gcc-7 and gcc-8
Comment 10 Richard Biener 2021-02-09 07:27:48 UTC
*** Bug 99012 has been marked as a duplicate of this bug. ***