* [Bug target/116275] [15 regression] ICE when building llvm-18.1.8 (convert_insn, at config/i386/i386-features.cc:1502)
2024-08-07 19:57 [Bug target/116275] New: [15 regression] ICE when building llvm-18.1.8 (convert_insn, at config/i386/i386-features.cc:1502) sjames at gcc dot gnu.org
@ 2024-08-07 21:16 ` roger at nextmovesoftware dot com
2024-08-08 0:57 ` sjames at gcc dot gnu.org
` (6 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: roger at nextmovesoftware dot com @ 2024-08-07 21:16 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116275
Roger Sayle <roger at nextmovesoftware dot com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|UNCONFIRMED |ASSIGNED
Ever confirmed|0 |1
Assignee|unassigned at gcc dot gnu.org |roger at nextmovesoftware dot com
Last reconfirmed| |2024-08-07
Target Milestone|--- |15.0
--- Comment #1 from Roger Sayle <roger at nextmovesoftware dot com> ---
Doh! This is almost certainly caused by my recent ashrv2di patch.
Investigating.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Bug target/116275] [15 regression] ICE when building llvm-18.1.8 (convert_insn, at config/i386/i386-features.cc:1502)
2024-08-07 19:57 [Bug target/116275] New: [15 regression] ICE when building llvm-18.1.8 (convert_insn, at config/i386/i386-features.cc:1502) sjames at gcc dot gnu.org
2024-08-07 21:16 ` [Bug target/116275] " roger at nextmovesoftware dot com
@ 2024-08-08 0:57 ` sjames at gcc dot gnu.org
2024-08-08 6:28 ` sjames at gcc dot gnu.org
` (5 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: sjames at gcc dot gnu.org @ 2024-08-08 0:57 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116275
--- Comment #2 from Sam James <sjames at gcc dot gnu.org> ---
Thank you Roger!
Reduced:
```
struct SymbolDesc push_back(SymbolDesc);
struct SymbolDesc {
long long ELFLocalSymIdx;
};
struct Expected {
long long &operator*();
};
void SymbolizableObjectFileaddSymbol() {
Expected SymbolAddressOrErr;
long long SymbolAddress = *SymbolAddressOrErr << 8 >> 8;
push_back({SymbolAddress});
}
```
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Bug target/116275] [15 regression] ICE when building llvm-18.1.8 (convert_insn, at config/i386/i386-features.cc:1502)
2024-08-07 19:57 [Bug target/116275] New: [15 regression] ICE when building llvm-18.1.8 (convert_insn, at config/i386/i386-features.cc:1502) sjames at gcc dot gnu.org
2024-08-07 21:16 ` [Bug target/116275] " roger at nextmovesoftware dot com
2024-08-08 0:57 ` sjames at gcc dot gnu.org
@ 2024-08-08 6:28 ` sjames at gcc dot gnu.org
2024-08-08 10:42 ` roger at nextmovesoftware dot com
` (4 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: sjames at gcc dot gnu.org @ 2024-08-08 6:28 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116275
--- Comment #3 from Sam James <sjames at gcc dot gnu.org> ---
(In reply to Sam James from comment #2)
(Make SymbolAddressOrErr a parameter for it to be well-defined.)
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Bug target/116275] [15 regression] ICE when building llvm-18.1.8 (convert_insn, at config/i386/i386-features.cc:1502)
2024-08-07 19:57 [Bug target/116275] New: [15 regression] ICE when building llvm-18.1.8 (convert_insn, at config/i386/i386-features.cc:1502) sjames at gcc dot gnu.org
` (2 preceding siblings ...)
2024-08-08 6:28 ` sjames at gcc dot gnu.org
@ 2024-08-08 10:42 ` roger at nextmovesoftware dot com
2024-08-08 10:52 ` sjames at gcc dot gnu.org
` (3 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: roger at nextmovesoftware dot com @ 2024-08-08 10:42 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116275
--- Comment #4 from Roger Sayle <roger at nextmovesoftware dot com> ---
Created attachment 58868
--> https://gcc.gnu.org/bugzilla/attachment.cgi?id=58868&action=edit
proposed patch
Here's my proposed fix (the first of two patches) that resolves the ICE with
the testcase. The problem is that i386.md has a *extenddi2_doubleword_highpart
pattern on !TARGET_64BIT for noticing that sign-extending a doubleword can be
optimized to just manipulating the highpart. The STV pass recognizes this as a
candidate, where previously ASHIFTRT of V2DI required AVX512VL, which runs into
problems as the post-reload splitter *extendv2di2_highpart_stv is conditional
on AVX512VL.
The first patch resolves this problem by adding a pre-reload splitter for this
case, called imaginatively called *extendv2di2_highpart_stv_noavx512vl. This
relies on split1's recursive splitting, so that the generated code for
(x<<8)>>8 in the testcase becomes:
vpsllq $8, %xmm0, %xmm0
vpsrad $8, %xmm0, %xmm1
vpsrlq $8, %xmm0, %xmm0
vpblendd $5, %xmm0, %xmm1, %xmm0
I'll post a follow-up patch (part 2) that provides a better implementation for
this V2DI highpart sign extension, which can be done in 3 insns (taking
advantage of SSE's ability to arithmetic shift right the "highpart"):
vpsllq $8, %xmm0, %xmm1
vpsrad $8, %xmm1, %xmm1
vpblendd $5, %xmm0, %xmm1, %xmm0
Sorry for the inconvenience. In the meantime, -mno-stv can be used as a
workaround. The bootstrap and regression testing should finish in a little
while.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Bug target/116275] [15 regression] ICE when building llvm-18.1.8 (convert_insn, at config/i386/i386-features.cc:1502)
2024-08-07 19:57 [Bug target/116275] New: [15 regression] ICE when building llvm-18.1.8 (convert_insn, at config/i386/i386-features.cc:1502) sjames at gcc dot gnu.org
` (3 preceding siblings ...)
2024-08-08 10:42 ` roger at nextmovesoftware dot com
@ 2024-08-08 10:52 ` sjames at gcc dot gnu.org
2024-08-12 5:54 ` cvs-commit at gcc dot gnu.org
` (2 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: sjames at gcc dot gnu.org @ 2024-08-08 10:52 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116275
--- Comment #5 from Sam James <sjames at gcc dot gnu.org> ---
Thank you, I'll use that for now, but please don't apologise. I do the testing
because I enjoy it and to help. No harm done.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Bug target/116275] [15 regression] ICE when building llvm-18.1.8 (convert_insn, at config/i386/i386-features.cc:1502)
2024-08-07 19:57 [Bug target/116275] New: [15 regression] ICE when building llvm-18.1.8 (convert_insn, at config/i386/i386-features.cc:1502) sjames at gcc dot gnu.org
` (4 preceding siblings ...)
2024-08-08 10:52 ` sjames at gcc dot gnu.org
@ 2024-08-12 5:54 ` cvs-commit at gcc dot gnu.org
2024-08-15 21:05 ` cvs-commit at gcc dot gnu.org
2024-08-18 18:57 ` sjames at gcc dot gnu.org
7 siblings, 0 replies; 9+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2024-08-12 5:54 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116275
--- Comment #6 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Roger Sayle <sayle@gcc.gnu.org>:
https://gcc.gnu.org/g:7a970bd03f1d8eed7703db8a8db3c753ea68899f
commit r15-2880-g7a970bd03f1d8eed7703db8a8db3c753ea68899f
Author: Roger Sayle <roger@nextmovesoftware.com>
Date: Mon Aug 12 06:52:48 2024 +0100
PR target/116275: Handle STV of *extenddi2_doubleword_highpart on i386.
This patch resolves PR target/116275, a recent ICE-on-valid regression on
-m32 caused by my recent change to enable STV of DImode arithmeric right
shift on non-AVX512VL targets. The oversight is that the i386 backend
contains an *extenddi2_doubleword_highpart instruction (whose pattern
is an arithmetic right shift of a left shift) that optimizes the case where
sign-extension need only update the highpart word of a DImode value when
generating 32-bit code (!TARGET_64BIT). STV accepts this pattern as a
candidate, as there are patterns to handle this form of extension on SSE
using AVX512VL instructions (and previously ASHIFTRT was only allowed on
AVX512VL). Now that ASHIFTRT is a candidate on non-AVX512vL targets, we
either need to check that the first operand is a register, or as done
below provide the define_insn_and_split that provides a non-AVX512VL
implementation of *extendv2di_highpart_stv.
The new testcase only ICEed with -m32, so this test could be limited to
target ia32, but there's no harm also running this test on -m64 to
provide a little extra test coverage.
2024-08-12 Roger Sayle <roger@nextmovesoftware.com>
gcc/ChangeLog
PR target/116275
* config/i386/i386.md (*extendv2di2_highpart_stv_noavx512vl): New
define_insn_and_split to handle the STV conversion of the DImode
pattern *extendsi2_doubleword_highpart.
gcc/testsuite/ChangeLog
PR target/116275
* g++.target/i386/pr116275.C: New test case.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Bug target/116275] [15 regression] ICE when building llvm-18.1.8 (convert_insn, at config/i386/i386-features.cc:1502)
2024-08-07 19:57 [Bug target/116275] New: [15 regression] ICE when building llvm-18.1.8 (convert_insn, at config/i386/i386-features.cc:1502) sjames at gcc dot gnu.org
` (5 preceding siblings ...)
2024-08-12 5:54 ` cvs-commit at gcc dot gnu.org
@ 2024-08-15 21:05 ` cvs-commit at gcc dot gnu.org
2024-08-18 18:57 ` sjames at gcc dot gnu.org
7 siblings, 0 replies; 9+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2024-08-15 21:05 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116275
--- Comment #7 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Roger Sayle <sayle@gcc.gnu.org>:
https://gcc.gnu.org/g:b6fb4f7f651d2aa89548c5833fe2679af2638df5
commit r15-2940-gb6fb4f7f651d2aa89548c5833fe2679af2638df5
Author: Roger Sayle <roger@nextmovesoftware.com>
Date: Thu Aug 15 22:02:05 2024 +0100
i386: Improve split of *extendv2di2_highpart_stv_noavx512vl.
This patch follows up on the previous patch to fix PR target/116275 by
improving the code STV (ultimately) generates for highpart sign extensions
like (x<<8)>>8. The arithmetic right shift is able to take advantage of
the available common subexpressions from the preceding left shift.
Hence previously with -O2 -m32 -mavx -mno-avx512vl we'd generate:
vpsllq $8, %xmm0, %xmm0
vpsrad $8, %xmm0, %xmm1
vpsrlq $8, %xmm0, %xmm0
vpblendw $51, %xmm0, %xmm1, %xmm0
But with improved splitting, we now generate three instructions:
vpslld $8, %xmm1, %xmm0
vpsrad $8, %xmm0, %xmm0
vpblendw $51, %xmm1, %xmm0, %xmm0
This patch also implements Uros' suggestion that the pre-reload
splitter could introduced a new pseudo to hold the intermediate
to potentially help reload with register allocation, which applies
when not performing the above optimization, i.e. on TARGET_XOP.
2024-08-15 Roger Sayle <roger@nextmovesoftware.com>
Uros Bizjak <ubizjak@gmail.com>
gcc/ChangeLog
* config/i386/i386.md (*extendv2di2_highpart_stv_noavx512vl): Split
to an improved implementation on !TARGET_XOP. On TARGET_XOP, use
a new pseudo for the intermediate to simplify register allocation.
gcc/testsuite/ChangeLog
* g++.target/i386/pr116275-2.C: New test case.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Bug target/116275] [15 regression] ICE when building llvm-18.1.8 (convert_insn, at config/i386/i386-features.cc:1502)
2024-08-07 19:57 [Bug target/116275] New: [15 regression] ICE when building llvm-18.1.8 (convert_insn, at config/i386/i386-features.cc:1502) sjames at gcc dot gnu.org
` (6 preceding siblings ...)
2024-08-15 21:05 ` cvs-commit at gcc dot gnu.org
@ 2024-08-18 18:57 ` sjames at gcc dot gnu.org
7 siblings, 0 replies; 9+ messages in thread
From: sjames at gcc dot gnu.org @ 2024-08-18 18:57 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116275
Sam James <sjames at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Resolution|--- |FIXED
Status|ASSIGNED |RESOLVED
Keywords| |ice-on-valid-code
--- Comment #8 from Sam James <sjames at gcc dot gnu.org> ---
Confirmed fixed. Thank you Roger!
^ permalink raw reply [flat|nested] 9+ messages in thread