public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug tree-optimization/103266] New: [12 regression] llvm-13 miscompilation: __builtin_assume_aligned causes over-aggressive dce
@ 2021-11-15 19:24 slyfox at gcc dot gnu.org
2021-11-15 19:45 ` [Bug tree-optimization/103266] " jakub at gcc dot gnu.org
` (8 more replies)
0 siblings, 9 replies; 10+ messages in thread
From: slyfox at gcc dot gnu.org @ 2021-11-15 19:24 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103266
Bug ID: 103266
Summary: [12 regression] llvm-13 miscompilation:
__builtin_assume_aligned causes over-aggressive dce
Product: gcc
Version: 12.0
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: tree-optimization
Assignee: unassigned at gcc dot gnu.org
Reporter: slyfox at gcc dot gnu.org
Target Milestone: ---
Noticed the bug initially on llvm-13 testsuite failure where 4 new tests fail
when llvm is built with gcc-12:
Failed Tests (4):
LLVM :: CodeGen/AArch64/win64-jumptable.ll
LLVM :: MC/AArch64/seh-packed-unwind.s
LLVM :: tools/llvm-readobj/COFF/arm64-packed-symbol-name.yaml
LLVM :: tools/llvm-readobj/COFF/arm64-packed-unwind.s
Here is the minimum reproducer extracted from llvm:
$ cat llvm-readobj.cpp
/*
$ g++-12.0.0 -UBUGGY -O1 -std=c++14 -o a llvm-readobj.cpp && ./a
# ok
$ g++-12.0.0 -DBUGGY -O1 -std=c++14 -o a llvm-readobj.cpp && ./a
Illegal instruction (core dumped) ./a
*/
typedef unsigned int u32;
typedef unsigned char u8;
static u32 pu8to32(const u8 * p8) __attribute__((noinline));
static u32 pu8to32(const u8 * p8) {
u32 v;
#if BUGGY
__builtin_memcpy(&v, __builtin_assume_aligned(p8, 1), sizeof(u32));
#else
__builtin_memcpy(&v, p8, sizeof(u32));
#endif
return v;
}
int main(void) {
// dse1 throws this store away
u8 d[sizeof(u32)] = {
0x07, 0x00, 0x00, 0x07,
};
if (pu8to32(d) != 0x07000007)
__builtin_trap();
}
Running:
$ g++-12.0.0 -UBUGGY -O1 -std=c++14 -o a llvm-readobj.cpp && ./a
# ok
$ g++-12.0.0 -DBUGGY -O1 -std=c++14 -o a llvm-readobj.cpp && ./a
Illegal instruction (core dumped)
It looks like the cause if failure is removed store to 'd':
--- g/a.S 2021-11-15 19:21:26.946265443 +0000
+++ b/a.S 2021-11-15 19:21:18.119173275 +0000
@@ -12,15 +12,14 @@
.globl main
.type main, @function
main:
.LFB1:
.cfi_startproc
subq $16, %rsp
.cfi_def_cfa_offset 24
- movl $117440519, 12(%rsp)
leaq 12(%rsp), %rdi
call _ZL7pu8to32PKh
cmpl $117440519, %eax
jne .L5
movl $0, %eax
addq $16, %rsp
.cfi_remember_state
Store is removed in '040t.dse1'. Could __builtin_assume_aligned() have
problematic escape annotations?
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug tree-optimization/103266] [12 regression] llvm-13 miscompilation: __builtin_assume_aligned causes over-aggressive dce
2021-11-15 19:24 [Bug tree-optimization/103266] New: [12 regression] llvm-13 miscompilation: __builtin_assume_aligned causes over-aggressive dce slyfox at gcc dot gnu.org
@ 2021-11-15 19:45 ` jakub at gcc dot gnu.org
2021-11-15 21:15 ` hubicka at gcc dot gnu.org
` (7 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: jakub at gcc dot gnu.org @ 2021-11-15 19:45 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103266
Jakub Jelinek <jakub at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Last reconfirmed| |2021-11-15
Target Milestone|--- |12.0
CC| |hubicka at gcc dot gnu.org,
| |jakub at gcc dot gnu.org
Priority|P3 |P1
Status|UNCONFIRMED |NEW
Ever confirmed|0 |1
--- Comment #1 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Started with r12-2353-g8da8ed435e9f01b37bf4ee57fa62509d44121c7d
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug tree-optimization/103266] [12 regression] llvm-13 miscompilation: __builtin_assume_aligned causes over-aggressive dce
2021-11-15 19:24 [Bug tree-optimization/103266] New: [12 regression] llvm-13 miscompilation: __builtin_assume_aligned causes over-aggressive dce slyfox at gcc dot gnu.org
2021-11-15 19:45 ` [Bug tree-optimization/103266] " jakub at gcc dot gnu.org
@ 2021-11-15 21:15 ` hubicka at gcc dot gnu.org
2021-11-15 21:33 ` hubicka at gcc dot gnu.org
` (6 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: hubicka at gcc dot gnu.org @ 2021-11-15 21:15 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103266
Jan Hubicka <hubicka at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |rguenther at suse dot de
--- Comment #2 from Jan Hubicka <hubicka at gcc dot gnu.org> ---
Thanks for a nice testcase!
We use "1cX " fnspec string_for assume aligned which indeed is problematic
since 'X' means that parameter is unused and thus we thus that it is not
returned.
"1" however specifies that function returns first parameter which is bit in a
contradiction with parameter being unused. PTA codes takes precedence to "1"
and thus works, while modref sees 'X' and ignore the parameters and thus
propagation breaks.
In general we may have a specifier for pointer parameter that is used only as a
scalar value and never read/written from. Perhaps 'S'? However I can also
teach modref to ignore unused for return handling.
Following untesed patch should fix the issue
diff --git a/gcc/builtins.c b/gcc/builtins.c
index 7d0f61fc98b..baeaff6eb1f 100644
--- a/gcc/builtins.c
+++ b/gcc/builtins.c
@@ -11101,7 +11101,7 @@ builtin_fnspec (tree callee)
case BUILT_IN_STACK_SAVE:
return ".c";
case BUILT_IN_ASSUME_ALIGNED:
- return "1cX ";
+ return "1cW ";
/* But posix_memalign stores a pointer into the memory pointed to
by its first argument. */
case BUILT_IN_POSIX_MEMALIGN:
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug tree-optimization/103266] [12 regression] llvm-13 miscompilation: __builtin_assume_aligned causes over-aggressive dce
2021-11-15 19:24 [Bug tree-optimization/103266] New: [12 regression] llvm-13 miscompilation: __builtin_assume_aligned causes over-aggressive dce slyfox at gcc dot gnu.org
2021-11-15 19:45 ` [Bug tree-optimization/103266] " jakub at gcc dot gnu.org
2021-11-15 21:15 ` hubicka at gcc dot gnu.org
@ 2021-11-15 21:33 ` hubicka at gcc dot gnu.org
2021-11-16 7:26 ` rguenther at suse dot de
` (5 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: hubicka at gcc dot gnu.org @ 2021-11-15 21:33 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103266
Jan Hubicka <hubicka at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |ASSIGNED
Assignee|unassigned at gcc dot gnu.org |hubicka at gcc dot gnu.org
--- Comment #3 from Jan Hubicka <hubicka at gcc dot gnu.org> ---
mine.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug tree-optimization/103266] [12 regression] llvm-13 miscompilation: __builtin_assume_aligned causes over-aggressive dce
2021-11-15 19:24 [Bug tree-optimization/103266] New: [12 regression] llvm-13 miscompilation: __builtin_assume_aligned causes over-aggressive dce slyfox at gcc dot gnu.org
` (2 preceding siblings ...)
2021-11-15 21:33 ` hubicka at gcc dot gnu.org
@ 2021-11-16 7:26 ` rguenther at suse dot de
2021-11-16 10:03 ` hubicka at kam dot mff.cuni.cz
` (4 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: rguenther at suse dot de @ 2021-11-16 7:26 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103266
--- Comment #4 from rguenther at suse dot de <rguenther at suse dot de> ---
On Mon, 15 Nov 2021, hubicka at gcc dot gnu.org wrote:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103266
>
> Jan Hubicka <hubicka at gcc dot gnu.org> changed:
>
> What |Removed |Added
> ----------------------------------------------------------------------------
> CC| |rguenther at suse dot de
>
> --- Comment #2 from Jan Hubicka <hubicka at gcc dot gnu.org> ---
> Thanks for a nice testcase!
>
> We use "1cX " fnspec string_for assume aligned which indeed is problematic
> since 'X' means that parameter is unused and thus we thus that it is not
> returned.
> "1" however specifies that function returns first parameter which is bit in a
> contradiction with parameter being unused. PTA codes takes precedence to "1"
> and thus works, while modref sees 'X' and ignore the parameters and thus
> propagation breaks.
>
> In general we may have a specifier for pointer parameter that is used only as a
> scalar value and never read/written from. Perhaps 'S'? However I can also
> teach modref to ignore unused for return handling.
I think 'X' means simply not dereferenced or escaping since this was all
PTA based. 'S' would still eventually allow escaping. But yes, PTA
simply takes '1' literally. So the patch below is IMHO too pessimizing.
Can you please fixup modref instead?
> Following untesed patch should fix the issue
> diff --git a/gcc/builtins.c b/gcc/builtins.c
> index 7d0f61fc98b..baeaff6eb1f 100644
> --- a/gcc/builtins.c
> +++ b/gcc/builtins.c
> @@ -11101,7 +11101,7 @@ builtin_fnspec (tree callee)
> case BUILT_IN_STACK_SAVE:
> return ".c";
> case BUILT_IN_ASSUME_ALIGNED:
> - return "1cX ";
> + return "1cW ";
> /* But posix_memalign stores a pointer into the memory pointed to
> by its first argument. */
> case BUILT_IN_POSIX_MEMALIGN:
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug tree-optimization/103266] [12 regression] llvm-13 miscompilation: __builtin_assume_aligned causes over-aggressive dce
2021-11-15 19:24 [Bug tree-optimization/103266] New: [12 regression] llvm-13 miscompilation: __builtin_assume_aligned causes over-aggressive dce slyfox at gcc dot gnu.org
` (3 preceding siblings ...)
2021-11-16 7:26 ` rguenther at suse dot de
@ 2021-11-16 10:03 ` hubicka at kam dot mff.cuni.cz
2021-11-16 10:23 ` rguenther at suse dot de
` (3 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: hubicka at kam dot mff.cuni.cz @ 2021-11-16 10:03 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103266
--- Comment #5 from hubicka at kam dot mff.cuni.cz ---
> I think 'X' means simply not dereferenced or escaping since this was all
> PTA based. 'S' would still eventually allow escaping. But yes, PTA
> simply takes '1' literally. So the patch below is IMHO too pessimizing.
> Can you please fixup modref instead?
If X is not meant to be "completely unused" (that is bit useless for
anotating bulitins I think since most of them shoul dbe sane) but all
the other flags together, we should update docs and eaf_flags production
which would fix the issue too.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug tree-optimization/103266] [12 regression] llvm-13 miscompilation: __builtin_assume_aligned causes over-aggressive dce
2021-11-15 19:24 [Bug tree-optimization/103266] New: [12 regression] llvm-13 miscompilation: __builtin_assume_aligned causes over-aggressive dce slyfox at gcc dot gnu.org
` (4 preceding siblings ...)
2021-11-16 10:03 ` hubicka at kam dot mff.cuni.cz
@ 2021-11-16 10:23 ` rguenther at suse dot de
2021-11-18 17:08 ` hubicka at gcc dot gnu.org
` (2 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: rguenther at suse dot de @ 2021-11-16 10:23 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103266
--- Comment #6 from rguenther at suse dot de <rguenther at suse dot de> ---
On Tue, 16 Nov 2021, hubicka at kam dot mff.cuni.cz wrote:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103266
>
> --- Comment #5 from hubicka at kam dot mff.cuni.cz ---
> > I think 'X' means simply not dereferenced or escaping since this was all
> > PTA based. 'S' would still eventually allow escaping. But yes, PTA
> > simply takes '1' literally. So the patch below is IMHO too pessimizing.
> > Can you please fixup modref instead?
>
> If X is not meant to be "completely unused" (that is bit useless for
> anotating bulitins I think since most of them shoul dbe sane) but all
> the other flags together, we should update docs and eaf_flags production
> which would fix the issue too.
Well, it was "completely irrelevant" for PTA purposes ;)
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug tree-optimization/103266] [12 regression] llvm-13 miscompilation: __builtin_assume_aligned causes over-aggressive dce
2021-11-15 19:24 [Bug tree-optimization/103266] New: [12 regression] llvm-13 miscompilation: __builtin_assume_aligned causes over-aggressive dce slyfox at gcc dot gnu.org
` (5 preceding siblings ...)
2021-11-16 10:23 ` rguenther at suse dot de
@ 2021-11-18 17:08 ` hubicka at gcc dot gnu.org
2021-11-18 17:42 ` cvs-commit at gcc dot gnu.org
2021-11-20 10:34 ` hubicka at gcc dot gnu.org
8 siblings, 0 replies; 10+ messages in thread
From: hubicka at gcc dot gnu.org @ 2021-11-18 17:08 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103266
--- Comment #7 from Jan Hubicka <hubicka at gcc dot gnu.org> ---
I am testing
diff --git a/gcc/ipa-modref.c b/gcc/ipa-modref.c
index c94f0589d44..e5d2b11025a 100644
--- a/gcc/ipa-modref.c
+++ b/gcc/ipa-modref.c
@@ -2033,10 +2033,7 @@ modref_eaf_analysis::merge_call_lhs_flags (gcall *call,
int arg,
bool indirect)
{
int index = SSA_NAME_VERSION (name);
-
- /* If value is not returned at all, do nothing. */
- if (!direct && !indirect)
- return;
+ bool returned_directly = false;
/* If there is no return value, no flags are affected. */
if (!gimple_call_lhs (call))
@@ -2047,10 +2044,23 @@ modref_eaf_analysis::merge_call_lhs_flags (gcall *call,
int arg,
if (arg >= 0)
{
int flags = gimple_call_return_flags (call);
- if ((flags & ERF_RETURNS_ARG)
- && (flags & ERF_RETURN_ARG_MASK) != arg)
- return;
+ if (flags & ERF_RETURNS_ARG)
+ {
+ if ((flags & ERF_RETURN_ARG_MASK) == arg)
+ returned_directly = true;
+ else
+ return;
+ }
+ }
+ /* Make ERF_RETURNS_ARG overwrite EAF_UNUSED. */
+ if (returned_directly)
+ {
+ direct = true;
+ indirect = false;
}
+ /* If value is not returned at all, do nothing. */
+ else if (!direct && !indirect)
+ return;
/* If return value is SSA name determine its flags. */
if (TREE_CODE (gimple_call_lhs (call)) == SSA_NAME)
@@ -2273,11 +2283,13 @@ modref_eaf_analysis::analyze_ssa_name (tree name)
if (gimple_call_arg (call, i) == name)
{
int call_flags = gimple_call_arg_flags (call, i);
- if (!ignore_retval && !(call_flags & EAF_UNUSED))
+ if (!ignore_retval)
merge_call_lhs_flags
(call, i, name,
- !(call_flags & EAF_NOT_RETURNED_DIRECTLY),
- !(call_flags & EAF_NOT_RETURNED_INDIRECTLY));
+ !(call_flags & (EAF_NOT_RETURNED_DIRECTLY
+ | EAF_UNUSED)),
+ !(call_flags & (EAF_NOT_RETURNED_INDIRECTLY
+ | EAF_UNUSED)));
if (!(ecf_flags & (ECF_CONST | ECF_NOVOPS)))
{
call_flags = callee_to_caller_flags
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug tree-optimization/103266] [12 regression] llvm-13 miscompilation: __builtin_assume_aligned causes over-aggressive dce
2021-11-15 19:24 [Bug tree-optimization/103266] New: [12 regression] llvm-13 miscompilation: __builtin_assume_aligned causes over-aggressive dce slyfox at gcc dot gnu.org
` (6 preceding siblings ...)
2021-11-18 17:08 ` hubicka at gcc dot gnu.org
@ 2021-11-18 17:42 ` cvs-commit at gcc dot gnu.org
2021-11-20 10:34 ` hubicka at gcc dot gnu.org
8 siblings, 0 replies; 10+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2021-11-18 17:42 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103266
--- Comment #8 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jan Hubicka <hubicka@gcc.gnu.org>:
https://gcc.gnu.org/g:c331a75d49b6043399f5ccce72a02ccf3b0ddc56
commit r12-5379-gc331a75d49b6043399f5ccce72a02ccf3b0ddc56
Author: Jan Hubicka <jh@suse.cz>
Date: Thu Nov 18 18:41:43 2021 +0100
Fix modref wrt __builtin_assume_aligned
__builtin_assume_aligned has bit contraictionary fnspec description "1cX "
which means that parameter 1 is returned but also unused. PTA code takes
precedence to parameter being returned, while modref takes the info that
parameter is unused. This patch tweaks modref to follow PTA semantics (as
suggested by Richard in the PR log)
gcc/ChangeLog:
2021-11-18 Jan Hubicka <hubicka@ucw.cz>
PR ipa/103266
* ipa-modref.c (modref_eaf_analysis::merge_call_lhs_flags): Unused
parameter may still be returned.
(modref_eaf_analysis::analyze_ssa_name): Call merge_call_lhs_flags
even for unused function args.
gcc/testsuite/ChangeLog:
2021-11-18 Jan Hubicka <hubicka@ucw.cz>
PR ipa/103266
* g++.dg/torture/pr103266.C: New test.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug tree-optimization/103266] [12 regression] llvm-13 miscompilation: __builtin_assume_aligned causes over-aggressive dce
2021-11-15 19:24 [Bug tree-optimization/103266] New: [12 regression] llvm-13 miscompilation: __builtin_assume_aligned causes over-aggressive dce slyfox at gcc dot gnu.org
` (7 preceding siblings ...)
2021-11-18 17:42 ` cvs-commit at gcc dot gnu.org
@ 2021-11-20 10:34 ` hubicka at gcc dot gnu.org
8 siblings, 0 replies; 10+ messages in thread
From: hubicka at gcc dot gnu.org @ 2021-11-20 10:34 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103266
Jan Hubicka <hubicka at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|ASSIGNED |RESOLVED
Resolution|--- |FIXED
--- Comment #9 from Jan Hubicka <hubicka at gcc dot gnu.org> ---
fixed.
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-11-20 10:34 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-15 19:24 [Bug tree-optimization/103266] New: [12 regression] llvm-13 miscompilation: __builtin_assume_aligned causes over-aggressive dce slyfox at gcc dot gnu.org
2021-11-15 19:45 ` [Bug tree-optimization/103266] " jakub at gcc dot gnu.org
2021-11-15 21:15 ` hubicka at gcc dot gnu.org
2021-11-15 21:33 ` hubicka at gcc dot gnu.org
2021-11-16 7:26 ` rguenther at suse dot de
2021-11-16 10:03 ` hubicka at kam dot mff.cuni.cz
2021-11-16 10:23 ` rguenther at suse dot de
2021-11-18 17:08 ` hubicka at gcc dot gnu.org
2021-11-18 17:42 ` cvs-commit at gcc dot gnu.org
2021-11-20 10:34 ` hubicka at gcc dot gnu.org
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).