[Bug target/98681] [8/9 Regression] aarch64: Invalid ubfiz instruction rejected by assembler

2021-04-22 Thread jakub at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98681

Jakub Jelinek  changed:

   What|Removed |Added

 Resolution|--- |FIXED
 Status|ASSIGNED|RESOLVED

--- Comment #15 from Jakub Jelinek  ---
Fixed.

[Bug target/98681] [8/9 Regression] aarch64: Invalid ubfiz instruction rejected by assembler

2021-04-22 Thread cvs-commit at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98681

--- Comment #14 from CVS Commits  ---
The releases/gcc-8 branch has been updated by Jakub Jelinek
:

https://gcc.gnu.org/g:4e4a5809586a345e12aed7a65e89551d6bdfbda3

commit r8-10877-g4e4a5809586a345e12aed7a65e89551d6bdfbda3
Author: Jakub Jelinek 
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) & 0xe000
in RTL we represent SImode constants with MSB set as negative
HOST_WIDE_INT,
so it will actually be HOST_WIDE_INT_C (0xe000), 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  

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)

[Bug target/98681] [8/9 Regression] aarch64: Invalid ubfiz instruction rejected by assembler

2021-04-20 Thread cvs-commit at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98681

--- Comment #13 from CVS Commits  ---
The releases/gcc-9 branch has been updated by Jakub Jelinek
:

https://gcc.gnu.org/g:e3dc765eb4556b78fe52d32be9858f2805b4488d

commit r9-9410-ge3dc765eb4556b78fe52d32be9858f2805b4488d
Author: Jakub Jelinek 
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) & 0xe000
in RTL we represent SImode constants with MSB set as negative
HOST_WIDE_INT,
so it will actually be HOST_WIDE_INT_C (0xe000), 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  

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)

[Bug target/98681] [8/9 Regression] aarch64: Invalid ubfiz instruction rejected by assembler

2021-01-29 Thread jakub at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98681

Jakub Jelinek  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  ---
Fixed for 10.3+ too.