* [Bug middle-end/98190] GCC 11.0 miscompiles code using _Bool when inlining: bfxil instruction misused?
2020-12-08 0:22 [Bug c/98190] New: GCC 11.0 miscompiles code using _Bool when inlining: bfxil instruction misused? vstinner at redhat dot com
@ 2020-12-08 0:29 ` pinskia at gcc dot gnu.org
2020-12-08 0:31 ` pinskia at gcc dot gnu.org
` (16 subsequent siblings)
17 siblings, 0 replies; 19+ messages in thread
From: pinskia at gcc dot gnu.org @ 2020-12-08 0:29 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98190
Andrew Pinski <pinskia at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Component|c |middle-end
--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
I think this is undefined behavior.
^ permalink raw reply [flat|nested] 19+ messages in thread
* [Bug middle-end/98190] GCC 11.0 miscompiles code using _Bool when inlining: bfxil instruction misused?
2020-12-08 0:22 [Bug c/98190] New: GCC 11.0 miscompiles code using _Bool when inlining: bfxil instruction misused? vstinner at redhat dot com
2020-12-08 0:29 ` [Bug middle-end/98190] " pinskia at gcc dot gnu.org
@ 2020-12-08 0:31 ` pinskia at gcc dot gnu.org
2020-12-08 0:33 ` vstinner at redhat dot com
` (15 subsequent siblings)
17 siblings, 0 replies; 19+ messages in thread
From: pinskia at gcc dot gnu.org @ 2020-12-08 0:31 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98190
Andrew Pinski <pinskia at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|UNCONFIRMED |RESOLVED
Resolution|--- |DUPLICATE
--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
This is a dup of bug 88662.
*** This bug has been marked as a duplicate of bug 88662 ***
^ permalink raw reply [flat|nested] 19+ messages in thread
* [Bug middle-end/98190] GCC 11.0 miscompiles code using _Bool when inlining: bfxil instruction misused?
2020-12-08 0:22 [Bug c/98190] New: GCC 11.0 miscompiles code using _Bool when inlining: bfxil instruction misused? vstinner at redhat dot com
2020-12-08 0:29 ` [Bug middle-end/98190] " pinskia at gcc dot gnu.org
2020-12-08 0:31 ` pinskia at gcc dot gnu.org
@ 2020-12-08 0:33 ` vstinner at redhat dot com
2020-12-08 0:37 ` pinskia at gcc dot gnu.org
` (14 subsequent siblings)
17 siblings, 0 replies; 19+ messages in thread
From: vstinner at redhat dot com @ 2020-12-08 0:33 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98190
--- Comment #3 from Victor Stinner <vstinner at redhat dot com> ---
Well, either all 64 bits of w4 and w5 registries should be initialized
properly, or the comparison should be done only on the least significant 8
bits:
(gdb) p ($w5 & 0xff) == ($w4 & 0xff)
$7 = 1
These bits are equal as expected ;-)
^ permalink raw reply [flat|nested] 19+ messages in thread
* [Bug middle-end/98190] GCC 11.0 miscompiles code using _Bool when inlining: bfxil instruction misused?
2020-12-08 0:22 [Bug c/98190] New: GCC 11.0 miscompiles code using _Bool when inlining: bfxil instruction misused? vstinner at redhat dot com
` (2 preceding siblings ...)
2020-12-08 0:33 ` vstinner at redhat dot com
@ 2020-12-08 0:37 ` pinskia at gcc dot gnu.org
2020-12-08 13:08 ` jakub at gcc dot gnu.org
` (13 subsequent siblings)
17 siblings, 0 replies; 19+ messages in thread
From: pinskia at gcc dot gnu.org @ 2020-12-08 0:37 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98190
--- Comment #4 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Victor Stinner from comment #3)
> Well, either all 64 bits of w4 and w5 registries should be initialized
> properly, or the comparison should be done only on the least significant 8
> bits:
>
> (gdb) p ($w5 & 0xff) == ($w4 & 0xff)
> $7 = 1
>
> These bits are equal as expected ;-)
Again this is undefined behavior.
^ permalink raw reply [flat|nested] 19+ messages in thread
* [Bug middle-end/98190] GCC 11.0 miscompiles code using _Bool when inlining: bfxil instruction misused?
2020-12-08 0:22 [Bug c/98190] New: GCC 11.0 miscompiles code using _Bool when inlining: bfxil instruction misused? vstinner at redhat dot com
` (3 preceding siblings ...)
2020-12-08 0:37 ` pinskia at gcc dot gnu.org
@ 2020-12-08 13:08 ` jakub at gcc dot gnu.org
2020-12-08 13:55 ` [Bug middle-end/98190] [11 Regression] GCC11 miscompiles code using _Bool when inlining: bfxil instruction misused since r11-165 jakub at gcc dot gnu.org
` (12 subsequent siblings)
17 siblings, 0 replies; 19+ messages in thread
From: jakub at gcc dot gnu.org @ 2020-12-08 13:08 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98190
Jakub Jelinek <jakub at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Ever confirmed|0 |1
Status|RESOLVED |REOPENED
Resolution|DUPLICATE |---
CC| |jakub at gcc dot gnu.org
Last reconfirmed| |2020-12-08
--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
I don't see UB there, UB would be if you copy a value other than 0 or 1 to the
_Bool variable, but that is not happening here.
Consider even:
static int __attribute__((noipa))
foo (const char *p, const char *q, const int len)
{
for (int i = 0; i < len; p++, q++, i++)
{
int equal;
_Bool x, y;
__builtin_memcpy ((char *) &x, p, sizeof x);
__builtin_memcpy ((char *) &y, q, sizeof y);
equal = (x == y);
if (equal <= 0)
return equal;
}
return 1;
}
int
main ()
{
const _Bool buf[4] = { 1, 0, 0, 0 };
register long x4 asm ("x4") = 0xdeadbeefULL;
register long x5 asm ("x5") = 0xdeadbeefULL;
asm volatile (""::"r" (x4), "r" (x5));
if (foo ((char *) &buf[0], (char *) &buf[0], 1) != 1)
__builtin_abort ();
return 0;
}
Copying through char * from _Bool to _Bool really must work, that is what
happens e.g. in structure assignments etc. if it has _Bool fields.
The reason this is miscompiled is that the aarch64 backend decides that the x
and y variables should be promoted from QImode to SImode and the expansion of
the memcpy folded into assignment sets a MEM_REF with QImode (i.e. low parts of
the promoted DECL_RTL), but nothing sign or zero extends it.
^ permalink raw reply [flat|nested] 19+ messages in thread
* [Bug middle-end/98190] [11 Regression] GCC11 miscompiles code using _Bool when inlining: bfxil instruction misused since r11-165
2020-12-08 0:22 [Bug c/98190] New: GCC 11.0 miscompiles code using _Bool when inlining: bfxil instruction misused? vstinner at redhat dot com
` (4 preceding siblings ...)
2020-12-08 13:08 ` jakub at gcc dot gnu.org
@ 2020-12-08 13:55 ` jakub at gcc dot gnu.org
2020-12-08 14:52 ` rguenth at gcc dot gnu.org
` (11 subsequent siblings)
17 siblings, 0 replies; 19+ messages in thread
From: jakub at gcc dot gnu.org @ 2020-12-08 13:55 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98190
Jakub Jelinek <jakub at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Summary|GCC 11.0 miscompiles code |[11 Regression] GCC11
|using _Bool when inlining: |miscompiles code using
|bfxil instruction misused? |_Bool when inlining: bfxil
| |instruction misused since
| |r11-165
CC| |rguenth at gcc dot gnu.org
--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Started with r11-165.
Perhaps decls set through MEM_REF even if it sets them in full size, but their
promoted mode is wider, shouldn't have the DECL_GIMPLE_REG_P bit set?
^ permalink raw reply [flat|nested] 19+ messages in thread
* [Bug middle-end/98190] [11 Regression] GCC11 miscompiles code using _Bool when inlining: bfxil instruction misused since r11-165
2020-12-08 0:22 [Bug c/98190] New: GCC 11.0 miscompiles code using _Bool when inlining: bfxil instruction misused? vstinner at redhat dot com
` (5 preceding siblings ...)
2020-12-08 13:55 ` [Bug middle-end/98190] [11 Regression] GCC11 miscompiles code using _Bool when inlining: bfxil instruction misused since r11-165 jakub at gcc dot gnu.org
@ 2020-12-08 14:52 ` rguenth at gcc dot gnu.org
2020-12-08 19:07 ` jakub at gcc dot gnu.org
` (10 subsequent siblings)
17 siblings, 0 replies; 19+ messages in thread
From: rguenth at gcc dot gnu.org @ 2020-12-08 14:52 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98190
Richard Biener <rguenth at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|REOPENED |NEW
Target Milestone|--- |11.0
Target| |aarch64
Priority|P3 |P1
--- Comment #7 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #6)
> Started with r11-165.
> Perhaps decls set through MEM_REF even if it sets them in full size, but
> their promoted mode is wider, shouldn't have the DECL_GIMPLE_REG_P bit set?
Huh, no, that's for sure not a "fix". I presume
_14 = MEM[(char * {ref-all})p_10(D) + ivtmp.8_4 * 1];
MEM[(char * {ref-all})&x] = _14;
_16 = MEM[(char * {ref-all})q_11(D) + ivtmp.8_4 * 1];
MEM[(char * {ref-all})&y] = _16;
x.0_1 = x;
y.1_2 = y;
if (x.0_1 != y.1_2)
is problematical, expanded as
(insn 21 20 0 (set (zero_extract:SI (reg/v:SI 99 [ y ])
(const_int 8 [0x8])
(const_int 0 [0]))
(reg:SI 107)) "t.c":9:7 -1
(nil))
;; if (x.0_1 != y.1_2)
(insn 22 21 23 (set (reg:CC 66 cc)
(compare:CC (reg/v:SI 98 [ x ])
(reg/v:SI 99 [ y ]))) "t.c":11:10 -1
(nil))
where the (set (zero_extract:SI ...) ..) leaves the upper parts of the
register unspecified (old content) but the compare now uses the
full register.
IMHO the compare using the full register w/o first zero/sign-extending
it has the bug. Who guarantees the inits of such promoted regs are
properly extended?
I'd say a fix in expansion of the non-MEM_P store would be OK-ish
(the generated code is also suboptimal I guess).
Note the bug would show up for VECTOR_TYPEs with integer modes in case
we expand those as promoted reg even before the change in question.
Semantic differences between GIMPLE and RTL are bad :/
How do we expand
x = _14;
? I guess one could carefully craft a testcase for this case.
^ permalink raw reply [flat|nested] 19+ messages in thread
* [Bug middle-end/98190] [11 Regression] GCC11 miscompiles code using _Bool when inlining: bfxil instruction misused since r11-165
2020-12-08 0:22 [Bug c/98190] New: GCC 11.0 miscompiles code using _Bool when inlining: bfxil instruction misused? vstinner at redhat dot com
` (6 preceding siblings ...)
2020-12-08 14:52 ` rguenth at gcc dot gnu.org
@ 2020-12-08 19:07 ` jakub at gcc dot gnu.org
2020-12-08 19:16 ` jakub at gcc dot gnu.org
` (9 subsequent siblings)
17 siblings, 0 replies; 19+ messages in thread
From: jakub at gcc dot gnu.org @ 2020-12-08 19:07 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98190
--- Comment #8 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
So, do we need to special case MEM_REF stores that store all bits (i.e. bitpos
0 bitsize equal to mode bitsize) into non-MEM variables which are promoted?
Something like:
--- gcc/expr.c.jj 2020-12-02 11:40:47.000000000 +0100
+++ gcc/expr.c 2020-12-08 19:57:05.147004740 +0100
@@ -5451,6 +5451,14 @@ expand_assignment (tree to, tree from, b
mode1, to_rtx, to, from,
reversep))
result = NULL;
+ else if (TREE_CODE (to) == MEM_REF
+ && !REF_REVERSE_STORAGE_ORDER (to)
+ && mem_ref_refers_to_non_mem_p (to)
+ && SUBREG_P (to_rtx)
+ && SUBREG_PROMOTED_VAR_P (to_rtx)
+ && known_eq (bitpos, 0)
+ && known_eq (bitsize, GET_MODE_BITSIZE (GET_MODE (to_rtx))))
+ result = store_expr (from, to_rtx, 0, nontemporal, false);
else
result = store_field (to_rtx, bitsize, bitpos,
bitregion_start, bitregion_end,
Not sure if it additionally doesn't have to check same mode, or if e.g. should
wrap from into a VCE if it has significantly different type.
It will not handle reverse storage order though.
^ permalink raw reply [flat|nested] 19+ messages in thread
* [Bug middle-end/98190] [11 Regression] GCC11 miscompiles code using _Bool when inlining: bfxil instruction misused since r11-165
2020-12-08 0:22 [Bug c/98190] New: GCC 11.0 miscompiles code using _Bool when inlining: bfxil instruction misused? vstinner at redhat dot com
` (7 preceding siblings ...)
2020-12-08 19:07 ` jakub at gcc dot gnu.org
@ 2020-12-08 19:16 ` jakub at gcc dot gnu.org
2020-12-09 9:04 ` rsandifo at gcc dot gnu.org
` (8 subsequent siblings)
17 siblings, 0 replies; 19+ messages in thread
From: jakub at gcc dot gnu.org @ 2020-12-08 19:16 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98190
--- Comment #9 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Perhaps some of those checks on the other side are redundant and could be
turned e.g. into gcc_checking_assert of gcc_assert, I bet if the MEM_REF
doesn't overwrite all bits, but only some subset of them, then the destination
couldn't be a nonmem decl and thus couldn't be promoted.
^ permalink raw reply [flat|nested] 19+ messages in thread
* [Bug middle-end/98190] [11 Regression] GCC11 miscompiles code using _Bool when inlining: bfxil instruction misused since r11-165
2020-12-08 0:22 [Bug c/98190] New: GCC 11.0 miscompiles code using _Bool when inlining: bfxil instruction misused? vstinner at redhat dot com
` (8 preceding siblings ...)
2020-12-08 19:16 ` jakub at gcc dot gnu.org
@ 2020-12-09 9:04 ` rsandifo at gcc dot gnu.org
2020-12-09 9:16 ` rguenther at suse dot de
` (7 subsequent siblings)
17 siblings, 0 replies; 19+ messages in thread
From: rsandifo at gcc dot gnu.org @ 2020-12-09 9:04 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98190
--- Comment #10 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #9)
> Perhaps some of those checks on the other side are redundant and could be
> turned e.g. into gcc_checking_assert of gcc_assert, I bet if the MEM_REF
> doesn't overwrite all bits, but only some subset of them, then the
> destination couldn't be a nonmem decl and thus couldn't be promoted.
Yeah, asserting sounds good. Checking the other conditions makes it
look like we could still fall through to the else for some promoted
subregs.
If we can't assert, I guess the rule is that we need to extend
whenever we're storing to the MSB of the inner register. We can
do that either by extending the source value and the range to
the outer register, or by assigning to the inner register and
then extending it separately.
^ permalink raw reply [flat|nested] 19+ messages in thread
* [Bug middle-end/98190] [11 Regression] GCC11 miscompiles code using _Bool when inlining: bfxil instruction misused since r11-165
2020-12-08 0:22 [Bug c/98190] New: GCC 11.0 miscompiles code using _Bool when inlining: bfxil instruction misused? vstinner at redhat dot com
` (9 preceding siblings ...)
2020-12-09 9:04 ` rsandifo at gcc dot gnu.org
@ 2020-12-09 9:16 ` rguenther at suse dot de
2020-12-09 9:41 ` jakub at gcc dot gnu.org
` (6 subsequent siblings)
17 siblings, 0 replies; 19+ messages in thread
From: rguenther at suse dot de @ 2020-12-09 9:16 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98190
--- Comment #11 from rguenther at suse dot de <rguenther at suse dot de> ---
On Wed, 9 Dec 2020, rsandifo at gcc dot gnu.org wrote:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98190
>
> --- Comment #10 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> ---
> (In reply to Jakub Jelinek from comment #9)
> > Perhaps some of those checks on the other side are redundant and could be
> > turned e.g. into gcc_checking_assert of gcc_assert, I bet if the MEM_REF
> > doesn't overwrite all bits, but only some subset of them, then the
> > destination couldn't be a nonmem decl and thus couldn't be promoted.
> Yeah, asserting sounds good. Checking the other conditions makes it
> look like we could still fall through to the else for some promoted
> subregs.
>
> If we can't assert, I guess the rule is that we need to extend
> whenever we're storing to the MSB of the inner register. We can
> do that either by extending the source value and the range to
> the outer register, or by assigning to the inner register and
> then extending it separately.
So I guess if you do a GIMPLE FE testcase with a __BIT_INSERT
to the MSB of a promoted var that should end up doing the extension
as well? Like (probably doesn't parse, needs a 1-bit precision '1')
signed char __GIMPLE () foo()
{
signed char _1;
signed char _2;
__BB(2):
_2 = __BIT_INSERT (_1(D), 1, 7);
return _2;
}
int main()
{
if (foo() > 0)
abort ();
}
?
^ permalink raw reply [flat|nested] 19+ messages in thread
* [Bug middle-end/98190] [11 Regression] GCC11 miscompiles code using _Bool when inlining: bfxil instruction misused since r11-165
2020-12-08 0:22 [Bug c/98190] New: GCC 11.0 miscompiles code using _Bool when inlining: bfxil instruction misused? vstinner at redhat dot com
` (10 preceding siblings ...)
2020-12-09 9:16 ` rguenther at suse dot de
@ 2020-12-09 9:41 ` jakub at gcc dot gnu.org
2020-12-09 15:03 ` rguenther at suse dot de
` (5 subsequent siblings)
17 siblings, 0 replies; 19+ messages in thread
From: jakub at gcc dot gnu.org @ 2020-12-09 9:41 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98190
--- Comment #12 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
(In reply to rsandifo@gcc.gnu.org from comment #10)
> If we can't assert, I guess the rule is that we need to extend
> whenever we're storing to the MSB of the inner register. We can
> do that either by extending the source value and the range to
> the outer register, or by assigning to the inner register and
> then extending it separately.
So perhaps:
--- gcc/expr.c.jj 2020-12-09 00:00:08.622548080 +0100
+++ gcc/expr.c 2020-12-09 10:36:12.198801940 +0100
@@ -5451,6 +5451,33 @@ expand_assignment (tree to, tree from, b
mode1, to_rtx, to, from,
reversep))
result = NULL;
+ else if (SUBREG_P (to_rtx)
+ && SUBREG_PROMOTED_VAR_P (to_rtx))
+ {
+ /* If to_rtx is a promoted subreg, this must be a store to the
+ whole variable, otherwise to_rtx would need to be MEM.
+ We need to zero or sign extend the value afterwards. */
+ gcc_assert (known_eq (bitpos, 0)
+ && known_eq (bitsize,
+ GET_MODE_BITSIZE (GET_MODE (to_rtx))));
+ if (TREE_CODE (to) == MEM_REF && !REF_REVERSE_STORAGE_ORDER (to))
+ result = store_expr (from, to_rtx, 0, nontemporal, false);
+ else
+ {
+ result = store_field (to_rtx, bitsize, bitpos,
+ bitregion_start, bitregion_end,
+ mode1, from, get_alias_set (to),
+ nontemporal, reversep);
+ rtx to_rtx1
+ = lowpart_subreg (subreg_unpromoted_mode (to_rtx),
+ SUBREG_REG (to_rtx),
+ subreg_promoted_mode (to_rtx));
+ to_rtx1 = convert_to_mode (subreg_promoted_mode (to_rtx),
+ to_rtx1,
+ SUBREG_PROMOTED_SIGN (to_rtx));
+ emit_move_insn (SUBREG_REG (to_rtx), to_rtx1);
+ }
+ }
else
result = store_field (to_rtx, bitsize, bitpos,
bitregion_start, bitregion_end,
then? As in, if store_expr can handle it, use that, otherwise perform the
extension at the end.
As for BIT_INSERT_EXPR, I'm not sure if SSA_NAMEs can get promoted SUBREGs or
not, but in any case it wouldn't be this code path, it would be store_expr
which handles the promoted SUBREGs already, because destination would not be a
MEM_REF with non-mem decl or reversed order, nor handled_component_p, nor
ARRAY_TYPE destination.
^ permalink raw reply [flat|nested] 19+ messages in thread
* [Bug middle-end/98190] [11 Regression] GCC11 miscompiles code using _Bool when inlining: bfxil instruction misused since r11-165
2020-12-08 0:22 [Bug c/98190] New: GCC 11.0 miscompiles code using _Bool when inlining: bfxil instruction misused? vstinner at redhat dot com
` (11 preceding siblings ...)
2020-12-09 9:41 ` jakub at gcc dot gnu.org
@ 2020-12-09 15:03 ` rguenther at suse dot de
2020-12-09 16:30 ` rsandifo at gcc dot gnu.org
` (4 subsequent siblings)
17 siblings, 0 replies; 19+ messages in thread
From: rguenther at suse dot de @ 2020-12-09 15:03 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98190
--- Comment #13 from rguenther at suse dot de <rguenther at suse dot de> ---
On Wed, 9 Dec 2020, jakub at gcc dot gnu.org wrote:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98190
>
> --- Comment #12 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
> (In reply to rsandifo@gcc.gnu.org from comment #10)
> > If we can't assert, I guess the rule is that we need to extend
> > whenever we're storing to the MSB of the inner register. We can
> > do that either by extending the source value and the range to
> > the outer register, or by assigning to the inner register and
> > then extending it separately.
>
> So perhaps:
> --- gcc/expr.c.jj 2020-12-09 00:00:08.622548080 +0100
> +++ gcc/expr.c 2020-12-09 10:36:12.198801940 +0100
> @@ -5451,6 +5451,33 @@ expand_assignment (tree to, tree from, b
> mode1, to_rtx, to, from,
> reversep))
> result = NULL;
> + else if (SUBREG_P (to_rtx)
> + && SUBREG_PROMOTED_VAR_P (to_rtx))
> + {
> + /* If to_rtx is a promoted subreg, this must be a store to the
> + whole variable, otherwise to_rtx would need to be MEM.
Yes, that's true - all !DECL_NOT_GIMPLE_REG_P may not have partial defs.
> + We need to zero or sign extend the value afterwards. */
> + gcc_assert (known_eq (bitpos, 0)
> + && known_eq (bitsize,
> + GET_MODE_BITSIZE (GET_MODE (to_rtx))));
> + if (TREE_CODE (to) == MEM_REF && !REF_REVERSE_STORAGE_ORDER (to))
> + result = store_expr (from, to_rtx, 0, nontemporal, false);
> + else
> + {
> + result = store_field (to_rtx, bitsize, bitpos,
> + bitregion_start, bitregion_end,
> + mode1, from, get_alias_set (to),
> + nontemporal, reversep);
> + rtx to_rtx1
> + = lowpart_subreg (subreg_unpromoted_mode (to_rtx),
> + SUBREG_REG (to_rtx),
> + subreg_promoted_mode (to_rtx));
> + to_rtx1 = convert_to_mode (subreg_promoted_mode (to_rtx),
> + to_rtx1,
> + SUBREG_PROMOTED_SIGN (to_rtx));
> + emit_move_insn (SUBREG_REG (to_rtx), to_rtx1);
> + }
> + }
> else
> result = store_field (to_rtx, bitsize, bitpos,
> bitregion_start, bitregion_end,
>
> then? As in, if store_expr can handle it, use that, otherwise perform the
> extension at the end.
>
> As for BIT_INSERT_EXPR, I'm not sure if SSA_NAMEs can get promoted SUBREGs or
> not, but in any case it wouldn't be this code path, it would be store_expr
> which handles the promoted SUBREGs already, because destination would not be a
> MEM_REF with non-mem decl or reversed order, nor handled_component_p, nor
> ARRAY_TYPE destination.
True.
^ permalink raw reply [flat|nested] 19+ messages in thread
* [Bug middle-end/98190] [11 Regression] GCC11 miscompiles code using _Bool when inlining: bfxil instruction misused since r11-165
2020-12-08 0:22 [Bug c/98190] New: GCC 11.0 miscompiles code using _Bool when inlining: bfxil instruction misused? vstinner at redhat dot com
` (12 preceding siblings ...)
2020-12-09 15:03 ` rguenther at suse dot de
@ 2020-12-09 16:30 ` rsandifo at gcc dot gnu.org
2020-12-10 8:22 ` jakub at gcc dot gnu.org
` (3 subsequent siblings)
17 siblings, 0 replies; 19+ messages in thread
From: rsandifo at gcc dot gnu.org @ 2020-12-09 16:30 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98190
--- Comment #14 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #12)
> (In reply to rsandifo@gcc.gnu.org from comment #10)
> > If we can't assert, I guess the rule is that we need to extend
> > whenever we're storing to the MSB of the inner register. We can
> > do that either by extending the source value and the range to
> > the outer register, or by assigning to the inner register and
> > then extending it separately.
>
> So perhaps:
> --- gcc/expr.c.jj 2020-12-09 00:00:08.622548080 +0100
> +++ gcc/expr.c 2020-12-09 10:36:12.198801940 +0100
> @@ -5451,6 +5451,33 @@ expand_assignment (tree to, tree from, b
> mode1, to_rtx, to, from,
> reversep))
> result = NULL;
> + else if (SUBREG_P (to_rtx)
> + && SUBREG_PROMOTED_VAR_P (to_rtx))
> + {
> + /* If to_rtx is a promoted subreg, this must be a store to the
> + whole variable, otherwise to_rtx would need to be MEM.
> + We need to zero or sign extend the value afterwards. */
> + gcc_assert (known_eq (bitpos, 0)
> + && known_eq (bitsize,
> + GET_MODE_BITSIZE (GET_MODE (to_rtx))));
> + if (TREE_CODE (to) == MEM_REF && !REF_REVERSE_STORAGE_ORDER (to))
> + result = store_expr (from, to_rtx, 0, nontemporal, false);
> + else
> + {
> + result = store_field (to_rtx, bitsize, bitpos,
> + bitregion_start, bitregion_end,
> + mode1, from, get_alias_set (to),
> + nontemporal, reversep);
> + rtx to_rtx1
> + = lowpart_subreg (subreg_unpromoted_mode (to_rtx),
> + SUBREG_REG (to_rtx),
> + subreg_promoted_mode (to_rtx));
> + to_rtx1 = convert_to_mode (subreg_promoted_mode (to_rtx),
> + to_rtx1,
> + SUBREG_PROMOTED_SIGN (to_rtx));
> + emit_move_insn (SUBREG_REG (to_rtx), to_rtx1);
> + }
> + }
> else
> result = store_field (to_rtx, bitsize, bitpos,
> bitregion_start, bitregion_end,
>
> then? As in, if store_expr can handle it, use that, otherwise perform the
> extension at the end.
LGTM FWIW, although I shouldn't be the one to review. I'm not
sure off-hand whether it's OK to store an unpromoted value to
a promoted subreg lhs, with the promoted subreg being temporarily
invalid. If that's a problem, it might be safer to pass to_rtx1
to store_field instead.
^ permalink raw reply [flat|nested] 19+ messages in thread
* [Bug middle-end/98190] [11 Regression] GCC11 miscompiles code using _Bool when inlining: bfxil instruction misused since r11-165
2020-12-08 0:22 [Bug c/98190] New: GCC 11.0 miscompiles code using _Bool when inlining: bfxil instruction misused? vstinner at redhat dot com
` (13 preceding siblings ...)
2020-12-09 16:30 ` rsandifo at gcc dot gnu.org
@ 2020-12-10 8:22 ` jakub at gcc dot gnu.org
2020-12-10 11:39 ` rguenther at suse dot de
` (2 subsequent siblings)
17 siblings, 0 replies; 19+ messages in thread
From: jakub at gcc dot gnu.org @ 2020-12-10 8:22 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98190
--- Comment #15 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 49727
--> https://gcc.gnu.org/bugzilla/attachment.cgi?id=49727&action=edit
gcc11-pr98190.patch
So, I have bootstrapped/regtested this patch last night on x86_64, i686,
aarch64, armv7hl, powerpc64le (and s390x still pending) linux.
Unfortunately, on aarch64 it regresses:
gcc.c-torture/execute/pr93213.c
and on powerpc64le that test plus:
g++.dg/warn/Wstrict-aliasing-bogus-char-1.C
gcc.dg/pr87273.c
gcc.dg/torture/pr91656-1.c
gcc.dg/tree-ssa/pr92085-2.c
gcc.dg/tree-ssa/pr94703.c
Seems the assumption that for promoted SUBREG to_rtx the store is always to all
the bits is incorrect, e.g. on pr93213.c the memcpy is copying just half of
the bits. So, shall we check the bitpos 0 bitsize all to_rtx bits for the
store_rtx case and otherwise check depending on endianity if the most
significant bit of to_rtx is overwritten and extend in that case?
^ permalink raw reply [flat|nested] 19+ messages in thread
* [Bug middle-end/98190] [11 Regression] GCC11 miscompiles code using _Bool when inlining: bfxil instruction misused since r11-165
2020-12-08 0:22 [Bug c/98190] New: GCC 11.0 miscompiles code using _Bool when inlining: bfxil instruction misused? vstinner at redhat dot com
` (14 preceding siblings ...)
2020-12-10 8:22 ` jakub at gcc dot gnu.org
@ 2020-12-10 11:39 ` rguenther at suse dot de
2020-12-11 10:12 ` cvs-commit at gcc dot gnu.org
2020-12-11 11:50 ` jakub at gcc dot gnu.org
17 siblings, 0 replies; 19+ messages in thread
From: rguenther at suse dot de @ 2020-12-10 11:39 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98190
--- Comment #16 from rguenther at suse dot de <rguenther at suse dot de> ---
On Thu, 10 Dec 2020, jakub at gcc dot gnu.org wrote:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98190
>
> --- Comment #15 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
> Created attachment 49727
> --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=49727&action=edit
> gcc11-pr98190.patch
>
> So, I have bootstrapped/regtested this patch last night on x86_64, i686,
> aarch64, armv7hl, powerpc64le (and s390x still pending) linux.
> Unfortunately, on aarch64 it regresses:
> gcc.c-torture/execute/pr93213.c
> and on powerpc64le that test plus:
> g++.dg/warn/Wstrict-aliasing-bogus-char-1.C
> gcc.dg/pr87273.c
> gcc.dg/torture/pr91656-1.c
> gcc.dg/tree-ssa/pr92085-2.c
> gcc.dg/tree-ssa/pr94703.c
>
> Seems the assumption that for promoted SUBREG to_rtx the store is always to all
> the bits is incorrect, e.g. on pr93213.c the memcpy is copying just half of
> the bits. So, shall we check the bitpos 0 bitsize all to_rtx bits for the
> store_rtx case and otherwise check depending on endianity if the most
> significant bit of to_rtx is overwritten and extend in that case?
in foo() you mean? For
__builtin_memmove (&u16_1, &u128_1, 1);
? So that's a parameter destination - does it at least have correctly
DECL_NOT_GIMPLE_REG_P set? Did expansion really do sth different
when we had it TREE_ADDRESSABLE?
^ permalink raw reply [flat|nested] 19+ messages in thread
* [Bug middle-end/98190] [11 Regression] GCC11 miscompiles code using _Bool when inlining: bfxil instruction misused since r11-165
2020-12-08 0:22 [Bug c/98190] New: GCC 11.0 miscompiles code using _Bool when inlining: bfxil instruction misused? vstinner at redhat dot com
` (15 preceding siblings ...)
2020-12-10 11:39 ` rguenther at suse dot de
@ 2020-12-11 10:12 ` cvs-commit at gcc dot gnu.org
2020-12-11 11:50 ` jakub at gcc dot gnu.org
17 siblings, 0 replies; 19+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2020-12-11 10:12 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98190
--- Comment #17 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:
https://gcc.gnu.org/g:3e60ddeb8220ed388819bb3f14e8caa9309fd3c2
commit r11-5927-g3e60ddeb8220ed388819bb3f14e8caa9309fd3c2
Author: Jakub Jelinek <jakub@redhat.com>
Date: Fri Dec 11 11:10:17 2020 +0100
expansion: Sign or zero extend on MEM_REF stores into SUBREG with
SUBREG_PROMOTED_VAR_P [PR98190]
Some targets decide to promote certain scalar variables to wider mode,
so their DECL_RTL is a SUBREG with SUBREG_PROMOTED_VAR_P.
When storing to such vars, store_expr takes care of sign or zero extending,
but if we store e.g. through MEM_REF into them, no sign or zero extension
happens and that leads to wrong-code e.g. on the following testcase on
aarch64-linux.
The following patch uses store_expr if we overwrite all the bits and it is
not reversed storage order, i.e. something that store_expr handles
normally,
and otherwise (if the most significant bit is (or for pdp11 might be, but
pdp11 doesn't promote) being modified), the code extends manually.
2020-12-11 Jakub Jelinek <jakub@redhat.com>
PR middle-end/98190
* expr.c (expand_assignment): If to_rtx is a promoted SUBREG,
ensure sign or zero extension either through use of store_expr
or by extending manually.
* gcc.dg/pr98190.c: New test.
^ permalink raw reply [flat|nested] 19+ messages in thread
* [Bug middle-end/98190] [11 Regression] GCC11 miscompiles code using _Bool when inlining: bfxil instruction misused since r11-165
2020-12-08 0:22 [Bug c/98190] New: GCC 11.0 miscompiles code using _Bool when inlining: bfxil instruction misused? vstinner at redhat dot com
` (16 preceding siblings ...)
2020-12-11 10:12 ` cvs-commit at gcc dot gnu.org
@ 2020-12-11 11:50 ` jakub at gcc dot gnu.org
17 siblings, 0 replies; 19+ messages in thread
From: jakub at gcc dot gnu.org @ 2020-12-11 11:50 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98190
Jakub Jelinek <jakub at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |RESOLVED
Resolution|--- |FIXED
--- Comment #18 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed.
^ permalink raw reply [flat|nested] 19+ messages in thread