https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82214
Bug ID: 82214
Summary: [AArch64] Incorrect checking of LDP/STP offsets in
aarch64_print_operand
Product: gcc
Version: 8.0
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: target
Assignee: unassigned at gcc dot gnu.org
Reporter: jcw at gcc dot gnu.org
Target Milestone: ---
Hi,
Currently, aarch64_print_operand calls aarch64_print_operand_address
(indirectly) when used on a memory address.
For LDP/STP memory operands, the behavior is incorrect.
`aarch64_print_operand_address` checks `aarch64_classify_address` to see
whether the passed address is valid and to determine some classification
information.
However, aarch64_print_operand does not have enough information to distinguish
between a LDP/STP operand and a LDR/STR operand. This means that in the call
the aarch64_classify_address, the address is treated as a (differently sized)
LDR/STR operand. In some cases, the LDP/STP operand is not within the range for
LDR/STR and results in an ICE.
An example of this case is with a negative offset for a 8 byte wide mode (such
as a double). The offset for LDR/STR can be 12 bits unsigned adjusted, or 9
bits signed unadjusted, and the 7 bits signed adjusted of LDP/STP has a wider
range than either of those in this case.
I suggest that we add another format type to aarch64_print_operand for LDP/STP
addresses that does the correct checking for LDP/STP memory addresses.
Note that if this patch has gone in:
https://gcc.gnu.org/ml/gcc-patches/2017-09/msg00873.html
Then it can be updated to not disallow negative offsets for double and long int
modes by changing (in aarch64_gen_adjusted_ldpstp):
/* The base offset is optimally half way between the two stp/ldp offsets.
*/
if (msize <= 4)
base_off = (off_val_1 + off_val_3) / 2;
else
/* However, due to issues with negative LDP/STP offset generation for
larger modes, for DF and DI modes (and any vector modes that have
since been added) we must not use negative addresses smaller than
9 signed unadjusted bits can store. This provides the most range
in this case. */
base_off = off_val_1;
/* Adjust the base so that it is aligned with the addresses but still
optimal. */
if (base_off % msize != off_val_1 % msize)
/* Fix the offset, bearing in mind we want to make it bigger not
smaller. */
base_off += (((base_off % msize) - (off_val_1 % msize)) + msize) % msize;
else if (msize <= 4)
/* The negative range of LDP/STP is one larger than the positive range.
*/
base_off += msize;
to
/* The base offset is optimally half way between the two stp/ldp offsets.
*/
base_off = (off_val_1 + off_val_3) / 2;
/* Adjust the base so that it is aligned with the addresses but still
optimal. */
if (base_off % msize != off_val_1 % msize)
/* Fix the offset, bearing in mind we want to make it bigger not
smaller. */
base_off += (((base_off % msize) - (off_val_1 % msize)) + msize) % msize;
else
/* The negative range of LDP/STP is one larger than the positive range.
*/
base_off += msize;
I expect this to fix test case `gcc.target/aarch64/ldp_stp_11.c`
To reproduce:
The bug is hidden on current trunk as we don't generate negative offset LDP/STP
instructions on AArch64. If the above patch is installed, it is again hidden by
the extra checks.
To reproduce, make sure my patch is installed, remove the checks from the code
as I indicated above. Then testcase `gcc.target/aarch64/ldp_stp_11` should ICE.
Jackson.
