public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug target/98681] New: aarch64: Invalid ubfiz instruction rejected by assembler
@ 2021-01-14 13:31 acoplan at gcc dot gnu.org
2021-01-14 13:31 ` [Bug target/98681] " acoplan at gcc dot gnu.org
` (16 more replies)
0 siblings, 17 replies; 18+ messages in thread
From: acoplan at gcc dot gnu.org @ 2021-01-14 13:31 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98681
Bug ID: 98681
Summary: aarch64: Invalid ubfiz instruction rejected by
assembler
Product: gcc
Version: 11.0
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: target
Assignee: unassigned at gcc dot gnu.org
Reporter: acoplan at gcc dot gnu.org
Target Milestone: ---
AArch64 GCC generates an invalid ubfiz instruction for the following testcase:
extern unsigned char a[];
int b;
void c() {
for (int d; d < 120; d += 6)
a[d] = b << -64;
}
$ aarch64-elf-gcc -c test.c -O3
test.c: In function 'c':
test.c:5:14: warning: left shift count is negative [-Wshift-count-negative]
5 | a[d] = b << -64;
| ^~
/tmp/ccelsQle.s: Assembler messages:
/tmp/ccelsQle.s:14: Error: immediate value out of range 0 to 31 at operand 3 --
`ubfiz w1,w1,-64,8'
Although this testcase invokes UB, it was reduced from a testcase which
ostensibly does not. In any case, we should not generate invalid assembly here.
^ permalink raw reply [flat|nested] 18+ messages in thread
* [Bug target/98681] aarch64: Invalid ubfiz instruction rejected by assembler
2021-01-14 13:31 [Bug target/98681] New: aarch64: Invalid ubfiz instruction rejected by assembler acoplan at gcc dot gnu.org
@ 2021-01-14 13:31 ` acoplan at gcc dot gnu.org
2021-01-14 13:45 ` [Bug target/98681] [8/9/10/11 Regression] " ktkachov at gcc dot gnu.org
` (15 subsequent siblings)
16 siblings, 0 replies; 18+ messages in thread
From: acoplan at gcc dot gnu.org @ 2021-01-14 13:31 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98681
Alex Coplan <acoplan at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Target Milestone|--- |11.0
^ permalink raw reply [flat|nested] 18+ messages in thread
* [Bug target/98681] [8/9/10/11 Regression] aarch64: Invalid ubfiz instruction rejected by assembler
2021-01-14 13:31 [Bug target/98681] New: aarch64: Invalid ubfiz instruction rejected by assembler acoplan at gcc dot gnu.org
2021-01-14 13:31 ` [Bug target/98681] " acoplan at gcc dot gnu.org
@ 2021-01-14 13:45 ` ktkachov at gcc dot gnu.org
2021-01-14 13:56 ` rguenth at gcc dot gnu.org
` (14 subsequent siblings)
16 siblings, 0 replies; 18+ messages in thread
From: ktkachov at gcc dot gnu.org @ 2021-01-14 13:45 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98681
ktkachov at gcc dot gnu.org changed:
What |Removed |Added
----------------------------------------------------------------------------
Ever confirmed|0 |1
Status|UNCONFIRMED |NEW
CC| |ktkachov at gcc dot gnu.org
Summary|aarch64: Invalid ubfiz |[8/9/10/11 Regression]
|instruction rejected by |aarch64: Invalid ubfiz
|assembler |instruction rejected by
| |assembler
Priority|P3 |P2
Last reconfirmed| |2021-01-14
--- Comment #1 from ktkachov at gcc dot gnu.org ---
Confirmed. GCC 5 didn't generate the bogus assembly.
I suppose the predicate and/or constraint of *andim_ashift<mode>_bfiz needs
tightening
^ permalink raw reply [flat|nested] 18+ messages in thread
* [Bug target/98681] [8/9/10/11 Regression] aarch64: Invalid ubfiz instruction rejected by assembler
2021-01-14 13:31 [Bug target/98681] New: aarch64: Invalid ubfiz instruction rejected by assembler acoplan at gcc dot gnu.org
2021-01-14 13:31 ` [Bug target/98681] " acoplan at gcc dot gnu.org
2021-01-14 13:45 ` [Bug target/98681] [8/9/10/11 Regression] " ktkachov at gcc dot gnu.org
@ 2021-01-14 13:56 ` rguenth at gcc dot gnu.org
2021-01-15 2:26 ` pinskia at gcc dot gnu.org
` (13 subsequent siblings)
16 siblings, 0 replies; 18+ messages in thread
From: rguenth at gcc dot gnu.org @ 2021-01-14 13:56 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98681
Richard Biener <rguenth at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Target Milestone|11.0 |8.5
^ permalink raw reply [flat|nested] 18+ messages in thread
* [Bug target/98681] [8/9/10/11 Regression] aarch64: Invalid ubfiz instruction rejected by assembler
2021-01-14 13:31 [Bug target/98681] New: aarch64: Invalid ubfiz instruction rejected by assembler acoplan at gcc dot gnu.org
` (2 preceding siblings ...)
2021-01-14 13:56 ` rguenth at gcc dot gnu.org
@ 2021-01-15 2:26 ` pinskia at gcc dot gnu.org
2021-01-22 15:35 ` jakub at gcc dot gnu.org
` (12 subsequent siblings)
16 siblings, 0 replies; 18+ messages in thread
From: pinskia at gcc dot gnu.org @ 2021-01-15 2:26 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98681
Andrew Pinski <pinskia at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
See Also| |https://gcc.gnu.org/bugzill
| |a/show_bug.cgi?id=87511
--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Related to https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87511 .
^ permalink raw reply [flat|nested] 18+ messages in thread
* [Bug target/98681] [8/9/10/11 Regression] aarch64: Invalid ubfiz instruction rejected by assembler
2021-01-14 13:31 [Bug target/98681] New: aarch64: Invalid ubfiz instruction rejected by assembler acoplan at gcc dot gnu.org
` (3 preceding siblings ...)
2021-01-15 2:26 ` pinskia at gcc dot gnu.org
@ 2021-01-22 15:35 ` jakub at gcc dot gnu.org
2021-01-22 15:51 ` jakub at gcc dot gnu.org
` (11 subsequent siblings)
16 siblings, 0 replies; 18+ messages in thread
From: jakub at gcc dot gnu.org @ 2021-01-22 15:35 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98681
Jakub Jelinek <jakub at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |jakub at gcc dot gnu.org
--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Adjusted testcase (-O2):
__attribute__((noipa)) int
foo (int x)
{
if (x > 32)
return (x << -64) & 255;
else
return x;
}
int
main ()
{
if (foo (32) != 32 || foo (-150) != -150)
__builtin_abort ();
return 0;
}
This doesn't invoke UB.
^ permalink raw reply [flat|nested] 18+ messages in thread
* [Bug target/98681] [8/9/10/11 Regression] aarch64: Invalid ubfiz instruction rejected by assembler
2021-01-14 13:31 [Bug target/98681] New: aarch64: Invalid ubfiz instruction rejected by assembler acoplan at gcc dot gnu.org
` (4 preceding siblings ...)
2021-01-22 15:35 ` jakub at gcc dot gnu.org
@ 2021-01-22 15:51 ` jakub at gcc dot gnu.org
2021-01-22 16:37 ` rearnsha at gcc dot gnu.org
` (10 subsequent siblings)
16 siblings, 0 replies; 18+ messages in thread
From: jakub at gcc dot gnu.org @ 2021-01-22 15:51 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98681
Jakub Jelinek <jakub at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Assignee|unassigned at gcc dot gnu.org |jakub at gcc dot gnu.org
Status|NEW |ASSIGNED
--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 50035
--> https://gcc.gnu.org/bugzilla/attachment.cgi?id=50035&action=edit
gcc11-pr98681.patch
Untested fix.
^ permalink raw reply [flat|nested] 18+ messages in thread
* [Bug target/98681] [8/9/10/11 Regression] aarch64: Invalid ubfiz instruction rejected by assembler
2021-01-14 13:31 [Bug target/98681] New: aarch64: Invalid ubfiz instruction rejected by assembler acoplan at gcc dot gnu.org
` (5 preceding siblings ...)
2021-01-22 15:51 ` jakub at gcc dot gnu.org
@ 2021-01-22 16:37 ` rearnsha at gcc dot gnu.org
2021-01-22 16:47 ` jakub at gcc dot gnu.org
` (9 subsequent siblings)
16 siblings, 0 replies; 18+ messages in thread
From: rearnsha at gcc dot gnu.org @ 2021-01-22 16:37 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98681
--- Comment #5 from Richard Earnshaw <rearnsha at gcc dot gnu.org> ---
Patch looks generally sensible, but I think all the INTVALs in that expression
should be converted to UINTVAL. The mask, in particular, is unsigned and it is
weird that one moment we're using a unsigned value and the next we're using a
signed value for shift_amt.
^ permalink raw reply [flat|nested] 18+ messages in thread
* [Bug target/98681] [8/9/10/11 Regression] aarch64: Invalid ubfiz instruction rejected by assembler
2021-01-14 13:31 [Bug target/98681] New: aarch64: Invalid ubfiz instruction rejected by assembler acoplan at gcc dot gnu.org
` (6 preceding siblings ...)
2021-01-22 16:37 ` rearnsha at gcc dot gnu.org
@ 2021-01-22 16:47 ` jakub at gcc dot gnu.org
2021-01-22 17:36 ` jakub at gcc dot gnu.org
` (8 subsequent siblings)
16 siblings, 0 replies; 18+ messages in thread
From: jakub at gcc dot gnu.org @ 2021-01-22 16:47 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98681
--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
For shft_amnt that single UINTVAL check will make sure it is not negative, so
whether the rest uses INTVAL or UINTVAL is then irrelevant.
Up to you as maintainer what you want there.
As for mask, let me check the exact insn behavior.
^ permalink raw reply [flat|nested] 18+ messages in thread
* [Bug target/98681] [8/9/10/11 Regression] aarch64: Invalid ubfiz instruction rejected by assembler
2021-01-14 13:31 [Bug target/98681] New: aarch64: Invalid ubfiz instruction rejected by assembler acoplan at gcc dot gnu.org
` (7 preceding siblings ...)
2021-01-22 16:47 ` jakub at gcc dot gnu.org
@ 2021-01-22 17:36 ` jakub at gcc dot gnu.org
2021-01-22 17:56 ` jakub at gcc dot gnu.org
` (7 subsequent siblings)
16 siblings, 0 replies; 18+ messages in thread
From: jakub at gcc dot gnu.org @ 2021-01-22 17:36 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98681
--- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
So, for the mask, the question is if we should or shouldn't handle e.g.
unsigned
foo (unsigned int x)
{
return (x << 13) & (-1U << 13);
}
or
unsigned
bar (unsigned int x)
{
return (x << 0) & -1U;
}
I think the instruction in theory can handle those corner-cases,
as lsb is [0, 31] and width [1, 32-lsb], but the optimizers (both GIMPLE and
simplify-rtx during combine) will optimize that away (the useless masking in
both cases and useless shift in the second case).
So the case where mask is negative is only theoretical whether it should or
shouldn't match, that
exact_log2 ((INTVAL (mask) >> INTVAL (shft_amnt)) + 1)
for negative INTVAL (mask) will result in -1 + 1 for the masks with bits
starting with shft_amnt set and smaller clear and exact_log2 (0) is -1, thus it
fail, while if UINTVAL it could succeed. E.g. that hypothetical -1U << 13 mask
is in CONST_INT_P -1 << 13 so in UINTVAL (0xffffffffffffe000 >> 13) + 1 is a
power of two. But e.g. for shift count 0, -1 mask would be (0xffffffffffffffff
>> 0) + 1,
so 0 and fail anyway.
On the other side, I'm not sure if the define_insn will work correctly when
mask is negative - consider again the (x << 13) & (-1U << 13).
The instruction uses %P3 which uses
case 'P':
if (!CONST_INT_P (x))
{
output_operand_lossage ("invalid operand for '%%%c'", code);
return;
}
asm_fprintf (f, "%u", popcount_hwi (INTVAL (x)));
but the constant is not -1U << 13, but -1 << 13 and so %P3 will not print
ubfiz w1, w0, 13, 19
that should be printed, but
ubfiz w1, w0, 13, 51
on which gas will fail again:
pr98681.s: Assembler messages:
pr98681.s:11: Error: immediate value out of range 1 to 32 at operand 4 --
`ubfiz w1,w0,13,51'
So, I think it is better to keep INTVAL (mask) as is.
^ permalink raw reply [flat|nested] 18+ messages in thread
* [Bug target/98681] [8/9/10/11 Regression] aarch64: Invalid ubfiz instruction rejected by assembler
2021-01-14 13:31 [Bug target/98681] New: aarch64: Invalid ubfiz instruction rejected by assembler acoplan at gcc dot gnu.org
` (8 preceding siblings ...)
2021-01-22 17:36 ` jakub at gcc dot gnu.org
@ 2021-01-22 17:56 ` jakub at gcc dot gnu.org
2021-01-26 13:50 ` cvs-commit at gcc dot gnu.org
` (6 subsequent siblings)
16 siblings, 0 replies; 18+ messages in thread
From: jakub at gcc dot gnu.org @ 2021-01-22 17:56 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98681
--- Comment #8 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 50037
--> https://gcc.gnu.org/bugzilla/attachment.cgi?id=50037&action=edit
gcc11-pr98681.patch
Therefore, if you want to use UINTVAL in that function wherever possible, we
can, but we need to add INTVAL (mask) > 0 check to catch the problems with
negative mask %P printing. I'm already bootstrapping/regtesting the earlier
patch, but can throw this one in too.
^ permalink raw reply [flat|nested] 18+ messages in thread
* [Bug target/98681] [8/9/10/11 Regression] aarch64: Invalid ubfiz instruction rejected by assembler
2021-01-14 13:31 [Bug target/98681] New: aarch64: Invalid ubfiz instruction rejected by assembler acoplan at gcc dot gnu.org
` (9 preceding siblings ...)
2021-01-22 17:56 ` jakub at gcc dot gnu.org
@ 2021-01-26 13:50 ` cvs-commit at gcc dot gnu.org
2021-01-26 14:00 ` [Bug target/98681] [8/9/10 " jakub at gcc dot gnu.org
` (5 subsequent siblings)
16 siblings, 0 replies; 18+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2021-01-26 13:50 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98681
--- Comment #9 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:17ad8cdebe65b47d257d85849747b806af0a85fd
commit r11-6914-g17ad8cdebe65b47d257d85849747b806af0a85fd
Author: Jakub Jelinek <jakub@redhat.com>
Date: Tue Jan 26 14:48:26 2021 +0100
aarch64: Tighten up checks for ubfix [PR98681]
The testcase in the patch doesn't assemble, because the instruction
requires
that the penultimate operand (lsb) range is [0, 32] (or [0, 64]) and the
last
operand's range is [1, 32 - lsb] (or [1, 64 - lsb]).
The INTVAL (shft_amnt) < GET_MODE_BITSIZE (mode) will accept the lsb
operand
to be in range [MIN, 32] (or [MIN, 64]) and then we invoke UB in the
compiler and sometimes it will make it through.
The patch changes all the INTVAL uses in that function to UINTVAL,
which isn't strictly necessary, but can be done (e.g. after the
UINTVAL (shft_amnt) < GET_MODE_BITSIZE (mode) check we know it is not
negative and thus INTVAL (shft_amnt) and UINTVAL (shft_amnt) then behave
the
same. But, I had to add INTVAL (mask) > 0 check in that case, otherwise we
risk (hypothetically) emitting instruction that doesn't assemble.
The problem is with masks that have the MSB bit set, while the instruction
can handle those, e.g.
ubfiz w1, w0, 13, 19
will do
(w0 << 13) & 0xffffe000
in RTL we represent SImode constants with MSB set as negative
HOST_WIDE_INT,
so it will actually be HOST_WIDE_INT_C (0xffffffffffffe000), and
the instruction uses %P3 to print the last operand, which calls
asm_fprintf (f, "%u", popcount_hwi (INTVAL (x)))
to print that. But that will not print 19, but 51 instead, will include
there also all the copies of the sign bit.
Not supporting those masks with MSB set isn't a big loss though, they
really
shouldn't appear normally, as both GIMPLE and RTL optimizations should
optimize those away (one isn't masking any bits off with such masks, so
just w0 << 13 will do too).
2021-01-26 Jakub Jelinek <jakub@redhat.com>
PR target/98681
* config/aarch64/aarch64.c (aarch64_mask_and_shift_for_ubfiz_p):
Use UINTVAL (shft_amnt) and UINTVAL (mask) instead of INTVAL
(shft_amnt)
and INTVAL (mask). Add && INTVAL (mask) > 0 condition.
* gcc.c-torture/execute/pr98681.c: New test.
^ permalink raw reply [flat|nested] 18+ messages in thread
* [Bug target/98681] [8/9/10 Regression] aarch64: Invalid ubfiz instruction rejected by assembler
2021-01-14 13:31 [Bug target/98681] New: aarch64: Invalid ubfiz instruction rejected by assembler acoplan at gcc dot gnu.org
` (10 preceding siblings ...)
2021-01-26 13:50 ` cvs-commit at gcc dot gnu.org
@ 2021-01-26 14:00 ` jakub at gcc dot gnu.org
2021-01-29 19:20 ` cvs-commit at gcc dot gnu.org
` (4 subsequent siblings)
16 siblings, 0 replies; 18+ messages in thread
From: jakub at gcc dot gnu.org @ 2021-01-26 14:00 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98681
Jakub Jelinek <jakub at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Summary|[8/9/10/11 Regression] |[8/9/10 Regression]
|aarch64: Invalid ubfiz |aarch64: Invalid ubfiz
|instruction rejected by |instruction rejected by
|assembler |assembler
--- Comment #10 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed on the trunk so far.
^ permalink raw reply [flat|nested] 18+ messages in thread
* [Bug target/98681] [8/9/10 Regression] aarch64: Invalid ubfiz instruction rejected by assembler
2021-01-14 13:31 [Bug target/98681] New: aarch64: Invalid ubfiz instruction rejected by assembler acoplan at gcc dot gnu.org
` (11 preceding siblings ...)
2021-01-26 14:00 ` [Bug target/98681] [8/9/10 " jakub at gcc dot gnu.org
@ 2021-01-29 19:20 ` cvs-commit at gcc dot gnu.org
2021-01-29 19:24 ` [Bug target/98681] [8/9 " jakub at gcc dot gnu.org
` (3 subsequent siblings)
16 siblings, 0 replies; 18+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2021-01-29 19:20 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98681
--- Comment #11 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-10 branch has been updated by Jakub Jelinek
<jakub@gcc.gnu.org>:
https://gcc.gnu.org/g:fb09d7242a25971b275292332337a56b86637f2c
commit r10-9321-gfb09d7242a25971b275292332337a56b86637f2c
Author: Jakub Jelinek <jakub@redhat.com>
Date: Tue Jan 26 14:48:26 2021 +0100
aarch64: Tighten up checks for ubfix [PR98681]
The testcase in the patch doesn't assemble, because the instruction
requires
that the penultimate operand (lsb) range is [0, 32] (or [0, 64]) and the
last
operand's range is [1, 32 - lsb] (or [1, 64 - lsb]).
The INTVAL (shft_amnt) < GET_MODE_BITSIZE (mode) will accept the lsb
operand
to be in range [MIN, 32] (or [MIN, 64]) and then we invoke UB in the
compiler and sometimes it will make it through.
The patch changes all the INTVAL uses in that function to UINTVAL,
which isn't strictly necessary, but can be done (e.g. after the
UINTVAL (shft_amnt) < GET_MODE_BITSIZE (mode) check we know it is not
negative and thus INTVAL (shft_amnt) and UINTVAL (shft_amnt) then behave
the
same. But, I had to add INTVAL (mask) > 0 check in that case, otherwise we
risk (hypothetically) emitting instruction that doesn't assemble.
The problem is with masks that have the MSB bit set, while the instruction
can handle those, e.g.
ubfiz w1, w0, 13, 19
will do
(w0 << 13) & 0xffffe000
in RTL we represent SImode constants with MSB set as negative
HOST_WIDE_INT,
so it will actually be HOST_WIDE_INT_C (0xffffffffffffe000), and
the instruction uses %P3 to print the last operand, which calls
asm_fprintf (f, "%u", popcount_hwi (INTVAL (x)))
to print that. But that will not print 19, but 51 instead, will include
there also all the copies of the sign bit.
Not supporting those masks with MSB set isn't a big loss though, they
really
shouldn't appear normally, as both GIMPLE and RTL optimizations should
optimize those away (one isn't masking any bits off with such masks, so
just w0 << 13 will do too).
2021-01-26 Jakub Jelinek <jakub@redhat.com>
PR target/98681
* config/aarch64/aarch64.c (aarch64_mask_and_shift_for_ubfiz_p):
Use UINTVAL (shft_amnt) and UINTVAL (mask) instead of INTVAL
(shft_amnt)
and INTVAL (mask). Add && INTVAL (mask) > 0 condition.
* gcc.c-torture/execute/pr98681.c: New test.
^ permalink raw reply [flat|nested] 18+ messages in thread
* [Bug target/98681] [8/9 Regression] aarch64: Invalid ubfiz instruction rejected by assembler
2021-01-14 13:31 [Bug target/98681] New: aarch64: Invalid ubfiz instruction rejected by assembler acoplan at gcc dot gnu.org
` (12 preceding siblings ...)
2021-01-29 19:20 ` cvs-commit at gcc dot gnu.org
@ 2021-01-29 19:24 ` jakub at gcc dot gnu.org
2021-04-20 23:31 ` cvs-commit at gcc dot gnu.org
` (2 subsequent siblings)
16 siblings, 0 replies; 18+ messages in thread
From: jakub at gcc dot gnu.org @ 2021-01-29 19:24 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98681
Jakub Jelinek <jakub at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Summary|[8/9/10 Regression] |[8/9 Regression] aarch64:
|aarch64: Invalid ubfiz |Invalid ubfiz instruction
|instruction rejected by |rejected by assembler
|assembler |
--- Comment #12 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed for 10.3+ too.
^ permalink raw reply [flat|nested] 18+ messages in thread
* [Bug target/98681] [8/9 Regression] aarch64: Invalid ubfiz instruction rejected by assembler
2021-01-14 13:31 [Bug target/98681] New: aarch64: Invalid ubfiz instruction rejected by assembler acoplan at gcc dot gnu.org
` (13 preceding siblings ...)
2021-01-29 19:24 ` [Bug target/98681] [8/9 " jakub at gcc dot gnu.org
@ 2021-04-20 23:31 ` cvs-commit at gcc dot gnu.org
2021-04-22 16:50 ` cvs-commit at gcc dot gnu.org
2021-04-22 17:11 ` jakub at gcc dot gnu.org
16 siblings, 0 replies; 18+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2021-04-20 23:31 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98681
--- Comment #13 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-9 branch has been updated by Jakub Jelinek
<jakub@gcc.gnu.org>:
https://gcc.gnu.org/g:e3dc765eb4556b78fe52d32be9858f2805b4488d
commit r9-9410-ge3dc765eb4556b78fe52d32be9858f2805b4488d
Author: Jakub Jelinek <jakub@redhat.com>
Date: Tue Jan 26 14:48:26 2021 +0100
aarch64: Tighten up checks for ubfix [PR98681]
The testcase in the patch doesn't assemble, because the instruction
requires
that the penultimate operand (lsb) range is [0, 32] (or [0, 64]) and the
last
operand's range is [1, 32 - lsb] (or [1, 64 - lsb]).
The INTVAL (shft_amnt) < GET_MODE_BITSIZE (mode) will accept the lsb
operand
to be in range [MIN, 32] (or [MIN, 64]) and then we invoke UB in the
compiler and sometimes it will make it through.
The patch changes all the INTVAL uses in that function to UINTVAL,
which isn't strictly necessary, but can be done (e.g. after the
UINTVAL (shft_amnt) < GET_MODE_BITSIZE (mode) check we know it is not
negative and thus INTVAL (shft_amnt) and UINTVAL (shft_amnt) then behave
the
same. But, I had to add INTVAL (mask) > 0 check in that case, otherwise we
risk (hypothetically) emitting instruction that doesn't assemble.
The problem is with masks that have the MSB bit set, while the instruction
can handle those, e.g.
ubfiz w1, w0, 13, 19
will do
(w0 << 13) & 0xffffe000
in RTL we represent SImode constants with MSB set as negative
HOST_WIDE_INT,
so it will actually be HOST_WIDE_INT_C (0xffffffffffffe000), and
the instruction uses %P3 to print the last operand, which calls
asm_fprintf (f, "%u", popcount_hwi (INTVAL (x)))
to print that. But that will not print 19, but 51 instead, will include
there also all the copies of the sign bit.
Not supporting those masks with MSB set isn't a big loss though, they
really
shouldn't appear normally, as both GIMPLE and RTL optimizations should
optimize those away (one isn't masking any bits off with such masks, so
just w0 << 13 will do too).
2021-01-26 Jakub Jelinek <jakub@redhat.com>
PR target/98681
* config/aarch64/aarch64.c (aarch64_mask_and_shift_for_ubfiz_p):
Use UINTVAL (shft_amnt) and UINTVAL (mask) instead of INTVAL
(shft_amnt)
and INTVAL (mask). Add && INTVAL (mask) > 0 condition.
* gcc.c-torture/execute/pr98681.c: New test.
(cherry picked from commit fb09d7242a25971b275292332337a56b86637f2c)
^ permalink raw reply [flat|nested] 18+ messages in thread
* [Bug target/98681] [8/9 Regression] aarch64: Invalid ubfiz instruction rejected by assembler
2021-01-14 13:31 [Bug target/98681] New: aarch64: Invalid ubfiz instruction rejected by assembler acoplan at gcc dot gnu.org
` (14 preceding siblings ...)
2021-04-20 23:31 ` cvs-commit at gcc dot gnu.org
@ 2021-04-22 16:50 ` cvs-commit at gcc dot gnu.org
2021-04-22 17:11 ` jakub at gcc dot gnu.org
16 siblings, 0 replies; 18+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2021-04-22 16:50 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98681
--- Comment #14 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-8 branch has been updated by Jakub Jelinek
<jakub@gcc.gnu.org>:
https://gcc.gnu.org/g:4e4a5809586a345e12aed7a65e89551d6bdfbda3
commit r8-10877-g4e4a5809586a345e12aed7a65e89551d6bdfbda3
Author: Jakub Jelinek <jakub@redhat.com>
Date: Tue Jan 26 14:48:26 2021 +0100
aarch64: Tighten up checks for ubfix [PR98681]
The testcase in the patch doesn't assemble, because the instruction
requires
that the penultimate operand (lsb) range is [0, 32] (or [0, 64]) and the
last
operand's range is [1, 32 - lsb] (or [1, 64 - lsb]).
The INTVAL (shft_amnt) < GET_MODE_BITSIZE (mode) will accept the lsb
operand
to be in range [MIN, 32] (or [MIN, 64]) and then we invoke UB in the
compiler and sometimes it will make it through.
The patch changes all the INTVAL uses in that function to UINTVAL,
which isn't strictly necessary, but can be done (e.g. after the
UINTVAL (shft_amnt) < GET_MODE_BITSIZE (mode) check we know it is not
negative and thus INTVAL (shft_amnt) and UINTVAL (shft_amnt) then behave
the
same. But, I had to add INTVAL (mask) > 0 check in that case, otherwise we
risk (hypothetically) emitting instruction that doesn't assemble.
The problem is with masks that have the MSB bit set, while the instruction
can handle those, e.g.
ubfiz w1, w0, 13, 19
will do
(w0 << 13) & 0xffffe000
in RTL we represent SImode constants with MSB set as negative
HOST_WIDE_INT,
so it will actually be HOST_WIDE_INT_C (0xffffffffffffe000), and
the instruction uses %P3 to print the last operand, which calls
asm_fprintf (f, "%u", popcount_hwi (INTVAL (x)))
to print that. But that will not print 19, but 51 instead, will include
there also all the copies of the sign bit.
Not supporting those masks with MSB set isn't a big loss though, they
really
shouldn't appear normally, as both GIMPLE and RTL optimizations should
optimize those away (one isn't masking any bits off with such masks, so
just w0 << 13 will do too).
2021-01-26 Jakub Jelinek <jakub@redhat.com>
PR target/98681
* config/aarch64/aarch64.c (aarch64_mask_and_shift_for_ubfiz_p):
Use UINTVAL (shft_amnt) and UINTVAL (mask) instead of INTVAL
(shft_amnt)
and INTVAL (mask). Add && INTVAL (mask) > 0 condition.
* gcc.c-torture/execute/pr98681.c: New test.
(cherry picked from commit fb09d7242a25971b275292332337a56b86637f2c)
^ permalink raw reply [flat|nested] 18+ messages in thread
* [Bug target/98681] [8/9 Regression] aarch64: Invalid ubfiz instruction rejected by assembler
2021-01-14 13:31 [Bug target/98681] New: aarch64: Invalid ubfiz instruction rejected by assembler acoplan at gcc dot gnu.org
` (15 preceding siblings ...)
2021-04-22 16:50 ` cvs-commit at gcc dot gnu.org
@ 2021-04-22 17:11 ` jakub at gcc dot gnu.org
16 siblings, 0 replies; 18+ messages in thread
From: jakub at gcc dot gnu.org @ 2021-04-22 17:11 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98681
Jakub Jelinek <jakub at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Resolution|--- |FIXED
Status|ASSIGNED |RESOLVED
--- Comment #15 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed.
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2021-04-22 17:11 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-14 13:31 [Bug target/98681] New: aarch64: Invalid ubfiz instruction rejected by assembler acoplan at gcc dot gnu.org
2021-01-14 13:31 ` [Bug target/98681] " acoplan at gcc dot gnu.org
2021-01-14 13:45 ` [Bug target/98681] [8/9/10/11 Regression] " ktkachov at gcc dot gnu.org
2021-01-14 13:56 ` rguenth at gcc dot gnu.org
2021-01-15 2:26 ` pinskia at gcc dot gnu.org
2021-01-22 15:35 ` jakub at gcc dot gnu.org
2021-01-22 15:51 ` jakub at gcc dot gnu.org
2021-01-22 16:37 ` rearnsha at gcc dot gnu.org
2021-01-22 16:47 ` jakub at gcc dot gnu.org
2021-01-22 17:36 ` jakub at gcc dot gnu.org
2021-01-22 17:56 ` jakub at gcc dot gnu.org
2021-01-26 13:50 ` cvs-commit at gcc dot gnu.org
2021-01-26 14:00 ` [Bug target/98681] [8/9/10 " jakub at gcc dot gnu.org
2021-01-29 19:20 ` cvs-commit at gcc dot gnu.org
2021-01-29 19:24 ` [Bug target/98681] [8/9 " jakub at gcc dot gnu.org
2021-04-20 23:31 ` cvs-commit at gcc dot gnu.org
2021-04-22 16:50 ` cvs-commit at gcc dot gnu.org
2021-04-22 17:11 ` jakub 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).