This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
[COMMITTED][GCC7][GCC8] Backport PR89222 (ARM thumb-2 misoptimisation of func ptr call with -O2 or -Os)
- From: Wilco Dijkstra <Wilco dot Dijkstra at arm dot com>
- To: GCC Patches <gcc-patches at gcc dot gnu dot org>
- Cc: nd <nd at arm dot com>
- Date: Tue, 16 Jul 2019 13:31:00 +0000
- Subject: [COMMITTED][GCC7][GCC8] Backport PR89222 (ARM thumb-2 misoptimisation of func ptr call with -O2 or -Os)
- Arc-authentication-results: i=1; mx.microsoft.com 1;spf=pass smtp.mailfrom=arm.com;dmarc=pass action=none header.from=arm.com;dkim=pass header.d=arm.com;arc=none
- Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=ZcgJbCOoYhVBVHm+yyGajo9ZHOLHWjGODYxQLcNoo1k=; b=cyLhB6zn1hBL0MRBlalhlsPCP34lUAlI2A+GyUxEODuMO4j9cvxzVKDUpneMSW0SKyTB3oiVRxW2fynhxObG6wSJlWcqbTP17lEYYB2MFzE5sQoao76cX0SmVwDaHbBX9BHk+F8cGjndbe9a/rnc8Fr1n5Kp3Xq0TdCuQVpFxfI3Y2kphhIRD6d/yvSlw4t3iAYezq8iEQcUI5bT9AmbQI+CwhHPri8KF/AnfP09dR27r4fJGvgPNmdoH8ZwtjMAZ2DTyuMpYY8VUjcxXjK/4OihGvxpHBfm0dDta7DiqqovhlyDVoyZNKeQrjrL3e/ZVqLYrf/xD4kTOV4LQhMCgg==
- Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=R3UkzssCwqR4gT5zdk8bSJ1dUUgMCxdDZzcvGyetAK/o9oFU75+a/hRhTC84ctV/zJVcERqHSPXW7p+1XWsFWPpjucgB6hxSySxjnQUR6s031wgtexT5DKf/+5SsQTrTRh6ukKy7uYgDoeJ4Z68Sdvbn+OZBdkqNlBzeuyWaOniH9JKFPmF6uci6k1mcaEQ3L+hCyyhsszehGhr6QiDXRJZPebQRK2xYfWo8S4W2RQSUdAlW8nPRZjNulA/lZhTZYWX9f3Dy/93KNcL1D7cCF2c7ImWZI71+BQSzcH92Ojpthv9caoC3Tw8YZq1v9t/7wnOfr3G26IRXmlxwN45MJQ==
- Original-authentication-results: spf=none (sender IP is ) smtp.mailfrom=Wilco dot Dijkstra at arm dot com;
Hi,
I've backported PR89222 to GCC7 and GCC8 branches so it's now fixed on all
active branches.
Wilco
[ARM] Fix PR89222
The GCC optimizer can generate symbols with non-zero offset from simple
if-statements. Bit zero is used for the Arm/Thumb state bit, so relocations
with offsets fail if it changes bit zero and the relocation forces bit zero
to true. The fix is to disable offsets on function pointer symbols.
gcc/
PR target/89222
* config/arm/arm.md (movsi): Use targetm.cannot_force_const_mem
to decide when to split off a non-zero offset from a symbol.
* config/arm/arm.c (arm_cannot_force_const_mem): Disallow offsets
in function symbols.
testsuite/
PR target/89222
* gcc.target/arm/pr89222.c: Add new test.
---
diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index f07f4cc47b6cfcea8f44960bf4760ea9a46b8f87..69b74a237a5f10b4137aa995ad43b77d6ecd04db 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -8940,11 +8940,16 @@ static bool
arm_cannot_force_const_mem (machine_mode mode ATTRIBUTE_UNUSED, rtx x)
{
rtx base, offset;
+ split_const (x, &base, &offset);
- if (ARM_OFFSETS_MUST_BE_WITHIN_SECTIONS_P)
+ if (SYMBOL_REF_P (base))
{
- split_const (x, &base, &offset);
- if (GET_CODE (base) == SYMBOL_REF
+ /* Function symbols cannot have an offset due to the Thumb bit. */
+ if ((SYMBOL_REF_FLAGS (base) & SYMBOL_FLAG_FUNCTION)
+ && INTVAL (offset) != 0)
+ return true;
+
+ if (ARM_OFFSETS_MUST_BE_WITHIN_SECTIONS_P
&& !offset_within_block_p (base, INTVAL (offset)))
return true;
}
diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md
index aa759624f8f617576773aa75fd6239d6e06e8a13..88110cb732b52866d4fdcad70fd5a202aa62bd03 100644
--- a/gcc/config/arm/arm.md
+++ b/gcc/config/arm/arm.md
@@ -5981,53 +5981,29 @@ (define_expand "movsi"
}
}
- if (ARM_OFFSETS_MUST_BE_WITHIN_SECTIONS_P)
+ split_const (operands[1], &base, &offset);
+ if (INTVAL (offset) != 0
+ && targetm.cannot_force_const_mem (SImode, operands[1]))
{
- split_const (operands[1], &base, &offset);
- if (GET_CODE (base) == SYMBOL_REF
- && !offset_within_block_p (base, INTVAL (offset)))
- {
- tmp = can_create_pseudo_p () ? gen_reg_rtx (SImode) : operands[0];
- emit_move_insn (tmp, base);
- emit_insn (gen_addsi3 (operands[0], tmp, offset));
- DONE;
- }
+ tmp = can_create_pseudo_p () ? gen_reg_rtx (SImode) : operands[0];
+ emit_move_insn (tmp, base);
+ emit_insn (gen_addsi3 (operands[0], tmp, offset));
+ DONE;
}
+ tmp = can_create_pseudo_p () ? NULL_RTX : operands[0];
+
/* Recognize the case where operand[1] is a reference to thread-local
- data and load its address to a register. */
+ data and load its address to a register. Offsets have been split off
+ already. */
if (arm_tls_referenced_p (operands[1]))
- {
- rtx tmp = operands[1];
- rtx addend = NULL;
-
- if (GET_CODE (tmp) == CONST && GET_CODE (XEXP (tmp, 0)) == PLUS)
- {
- addend = XEXP (XEXP (tmp, 0), 1);
- tmp = XEXP (XEXP (tmp, 0), 0);
- }
-
- gcc_assert (GET_CODE (tmp) == SYMBOL_REF);
- gcc_assert (SYMBOL_REF_TLS_MODEL (tmp) != 0);
-
- tmp = legitimize_tls_address (tmp,
- !can_create_pseudo_p () ? operands[0] : 0);
- if (addend)
- {
- tmp = gen_rtx_PLUS (SImode, tmp, addend);
- tmp = force_operand (tmp, operands[0]);
- }
- operands[1] = tmp;
- }
+ operands[1] = legitimize_tls_address (operands[1], tmp);
else if (flag_pic
&& (CONSTANT_P (operands[1])
|| symbol_mentioned_p (operands[1])
|| label_mentioned_p (operands[1])))
- operands[1] = legitimize_pic_address (operands[1], SImode,
- (!can_create_pseudo_p ()
- ? operands[0]
- : NULL_RTX), NULL_RTX,
- false /*compute_now*/);
+ operands[1] =
+ legitimize_pic_address (operands[1], SImode, tmp);
}
"
)
diff --git a/gcc/testsuite/gcc.target/arm/pr89222.c b/gcc/testsuite/gcc.target/arm/pr89222.c
new file mode 100644
index 0000000000000000000000000000000000000000..d26d7df17544db8426331e67b9a36d749ec6c6d1
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/pr89222.c
@@ -0,0 +1,32 @@
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+
+void g (void);
+
+void f1 (int x)
+{
+ if (x != (int) g + 3)
+ return;
+ g();
+}
+
+void (*a2)(void);
+
+void f2 (void)
+{
+ a2 = &g + 3;
+}
+
+typedef void (*__sighandler_t)(int);
+void handler (int);
+
+void f3 (int x)
+{
+ __sighandler_t h = &handler;
+ if (h != (__sighandler_t) 2 && h != (__sighandler_t) 1)
+ h (x);
+}
+
+/* { dg-final { scan-assembler-times {add(?:s)?\tr[0-9]+, r[0-9]+, #3} 2 } } */
+/* { dg-final { scan-assembler-not {.word\tg\+3} } } */
+/* { dg-final { scan-assembler-not {.word\thandler-1} } } */