Ping^3 : [PATCH] [gcc, combine] PR46164: Don't combine the insns if a volatile register is contained.
Terry Guo
flameroc@gmail.com
Wed Apr 22 02:21:00 GMT 2015
On Wed, Apr 22, 2015 at 9:44 AM, Segher Boessenkool
<segher@kernel.crashing.org> wrote:
> On Tue, Apr 21, 2015 at 03:13:38PM +0800, Terry Guo wrote:
>> > Did you fix the comment? REG_USERVAR_P and HARD_REGISTER_P can be
>> > set for more than just register asm.
>>
>> Sorry for missing the patch. I believe that I addressed your patch.
>> Please review it again to make sure my understanding is correct.
>
>> + /* Use REG_USERVAR_P and HARD_REGISTER_P to check whether DEST is a user
>> + specified register, and do not eliminate such register if it is in an
>> + asm input. Otherwise if allow such elimination, we may break the
>> + register asm usage defined in GCC manual. */
>> + if (REG_P (dest) && REG_USERVAR_P (dest) && HARD_REGISTER_P (dest)
>> + && extract_asm_operands (PATTERN (i3)))
>> + return 0;
>
> The "to check whether DEST is a user-specified register" part is not
> correct; this check can for example also match for function arguments
> (which are hard regs) that were combined into any "normal" user var.
> I don't see how we would do a better check, and disallowing combination
> in this case is harmless (or even good); but the comment is misleading.
>
>
> Segher
Thanks for reviewing. Patch is updated per you suggestion. The
ChangeLog is also updated as below:
gcc/ChangeLog:
2015-04-22 Hale Wang <hale.wang@arm.com>
Terry Guo <terry.guo@arm.com>
PR rtl-optimization/64818
* combine.c (can_combine_p): Don't combine user-specified register if
it is in an asm input.
gcc/testsuite/ChangeLog:
2015-04-22 Hale Wang <hale.wang@arm.com>
Terry Guo <terry.guo@arm.com>
PR rtl-optimization/64818
* gcc.target/arm/pr64818.c: New.
-------------- next part --------------
diff --git a/gcc/combine.c b/gcc/combine.c
index 6f0007a..6cd55dd 100644
--- a/gcc/combine.c
+++ b/gcc/combine.c
@@ -1910,6 +1910,15 @@ can_combine_p (rtx_insn *insn, rtx_insn *i3, rtx_insn *pred ATTRIBUTE_UNUSED,
set = expand_field_assignment (set);
src = SET_SRC (set), dest = SET_DEST (set);
+ /* Do not eliminate user-specified register if it is in an
+ asm input because we may break the register asm usage defined
+ in GCC manual if allow to do so.
+ Be aware that this may cover more cases than we expect but this
+ should be harmless. */
+ if (REG_P (dest) && REG_USERVAR_P (dest) && HARD_REGISTER_P (dest)
+ && extract_asm_operands (PATTERN (i3)))
+ return 0;
+
/* Don't eliminate a store in the stack pointer. */
if (dest == stack_pointer_rtx
/* Don't combine with an insn that sets a register to itself if it has
diff --git a/gcc/testsuite/gcc.target/arm/pr64818.c b/gcc/testsuite/gcc.target/arm/pr64818.c
new file mode 100644
index 0000000..bddd846
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/pr64818.c
@@ -0,0 +1,30 @@
+/* { dg-do compile } */
+/* { dg-options "-O1" } */
+
+char temp[16];
+extern int foo1 (void);
+
+void foo (void)
+{
+ int i;
+ int len;
+
+ while (1)
+ {
+ len = foo1 ();
+ register int a asm ("r0") = 5;
+ register char *b asm ("r1") = temp;
+ register int c asm ("r2") = len;
+ asm volatile ("mov %[r0], %[r0]\n mov %[r1], %[r1]\n mov %[r2], %[r2]\n"
+ : "+m"(*b)
+ : [r0]"r"(a), [r1]"r"(b), [r2]"r"(c));
+
+ for (i = 0; i < len; i++)
+ {
+ if (temp[i] == 10)
+ return;
+ }
+ }
+}
+
+/* { dg-final { scan-assembler "\[\\t \]+mov\ r1,\ r1" } } */
More information about the Gcc-patches
mailing list