diff mbox series

[LRA] Never reload fixed form constraints memory operand

Message ID CAKnkMGvXi16uEfXRfHe2WPHkoCMLsoCBmHm8op=FmAHcN3nLkg@mail.gmail.com
State New
Headers show
Series [LRA] Never reload fixed form constraints memory operand | expand

Commit Message

Thomas Preudhomme Oct. 3, 2018, 4:47 p.m. UTC 
Hi,

The unconditional reload of address operand for recognized instruction
in process_address_1 prevent the patch for fixing "PR85434: Address of
stack protector guard spilled to stack on ARM" proposed at [1]. The code
in this patch attempt to control which registers are used to make PIC
access but the reload performed by process_address_1 will use generic
PIC access. This patch removes the test for the instruction to be
unrecognized to do the reload, thus always avoiding to reload address
operand for fixed constraints (such as "X" used in the patch).

[1] https://gcc.gnu.org/ml/gcc-patches/2018-08/msg01838.html

ChangeLog entry is as follows:

*** gcc/ChangeLog ***

2018-10-03  Thomas Preud'homme  <thomas.preudhomme@linaro.org>

    * lra-constraints.c (process_address_1): Bail out for all
    satisfied fixed constraints.

Testing: Successfully bootstrapped and regtested on:
- arm-linux-gnueabihf (both Arm and Thumb2 mode)
- aarch64-linux-gnu
- x86_64-linux-gnu
- i386-linux-gnu
- sparc64-linux-gnu (gcc202)
- powerpc64le-linux-gnu (gcc112)

Is this ok for trunk?

Best regards,

Thomas

Comments

Vladimir Makarov Oct. 4, 2018, 3:12 a.m. UTC | #1 
On 10/03/2018 12:47 PM, Thomas Preudhomme wrote:
> Best regards,

>

> Thomas

>

> never_reload_fixed_address_operand.patch

>

>

>  From 2831d8b886d92513c2d30d43a6a989d2bbd0ceee Mon Sep 17 00:00:00 2001

> From: Thomas Preud'homme<thomas.preudhomme@linaro.org>

> Date: Thu, 27 Sep 2018 09:50:12 +0100

> Subject: [PATCH] [PATCH, LRA] Never reload fixed form constraints memory

>   operand

>

> Hi,

>

> The unconditional reload of address operand for recognized instruction

> in process_address_1 prevent the patch for fixing "PR85434: Address of

> stack protector guard spilled to stack on ARM" proposed at [1]. The code

> in this patch attempt to control which registers are used to make PIC

> access but the reload performed by process_address_1 will use generic

> PIC access. This patch removes the test for the instruction to be

> unrecognized to do the reload, thus always avoiding to reload address

> operand for fixed constraints (such as "X" used in the patch).

>

> [1]https://gcc.gnu.org/ml/gcc-patches/2018-08/msg01838.html

>

> ChangeLog entry is as follows:

>

> *** gcc/ChangeLog ***

>

> 2018-10-03  Thomas Preud'homme<thomas.preudhomme@linaro.org>

>

> 	* lra-constraints.c (process_address_1): Bail out for all

> 	satisfied fixed constraints.

>

> Testing: Successfully bootstrapped and regtested on:

> - arm-linux-gnueabihf (both Arm and Thumb2 mode)

> - aarch64-linux-gnu

> - x86_64-linux-gnu

> - i386-linux-gnu

> - sparc64-linux-gnu (gcc202)

> - powerpc64le-linux-gnu (gcc112)

>

> Is this ok for trunk?

>

OK. Thank you for testing all these targets, Thomas.
H.J. Lu Oct. 4, 2018, 11:30 a.m. UTC | #2 
On Wed, Oct 3, 2018 at 8:12 PM Vladimir Makarov <vmakarov@redhat.com> wrote:
>

> On 10/03/2018 12:47 PM, Thomas Preudhomme wrote:

> > Best regards,

> >

> > Thomas

> >

> > never_reload_fixed_address_operand.patch

> >

> >

> >  From 2831d8b886d92513c2d30d43a6a989d2bbd0ceee Mon Sep 17 00:00:00 2001

> > From: Thomas Preud'homme<thomas.preudhomme@linaro.org>

> > Date: Thu, 27 Sep 2018 09:50:12 +0100

> > Subject: [PATCH] [PATCH, LRA] Never reload fixed form constraints memory

> >   operand

> >

> > Hi,

> >

> > The unconditional reload of address operand for recognized instruction

> > in process_address_1 prevent the patch for fixing "PR85434: Address of

> > stack protector guard spilled to stack on ARM" proposed at [1]. The code

> > in this patch attempt to control which registers are used to make PIC

> > access but the reload performed by process_address_1 will use generic

> > PIC access. This patch removes the test for the instruction to be

> > unrecognized to do the reload, thus always avoiding to reload address

> > operand for fixed constraints (such as "X" used in the patch).

> >

> > [1]https://gcc.gnu.org/ml/gcc-patches/2018-08/msg01838.html

> >

> > ChangeLog entry is as follows:

> >

> > *** gcc/ChangeLog ***

> >

> > 2018-10-03  Thomas Preud'homme<thomas.preudhomme@linaro.org>

> >

> >       * lra-constraints.c (process_address_1): Bail out for all

> >       satisfied fixed constraints.

> >

> > Testing: Successfully bootstrapped and regtested on:

> > - arm-linux-gnueabihf (both Arm and Thumb2 mode)

> > - aarch64-linux-gnu

> > - x86_64-linux-gnu

> > - i386-linux-gnu

> > - sparc64-linux-gnu (gcc202)

> > - powerpc64le-linux-gnu (gcc112)

> >

> > Is this ok for trunk?

> >

> OK. Thank you for testing all these targets, Thomas.

>


This caused:

FAIL: gcc.target/i386/pr83317.c (internal compiler error)
FAIL: gcc.target/i386/pr83317.c (test for excess errors)

[hjl@gnu-4 gcc]$
/export/build/gnu/tools-build/gcc-debug/build-x86_64-linux/gcc/xgcc
-B/export/build/gnu/tools-build/gcc-debug/build-x86_64-linux/gcc/
/export/gnu/import/git/sources/gcc/gcc/testsuite/gcc.target/i386/pr83317.c
 -m32   -fno-diagnostics-show-caret -fno-diagnostics-show-line-numbers
-fdiagnostics-color=never   -O1 -fPIC -msse2 -mfpmath=sse -S -o
pr83317.s
during RTL pass: reload
/export/gnu/import/git/sources/gcc/gcc/testsuite/gcc.target/i386/pr83317.c:
In function \u2018foo\u2019:
/export/gnu/import/git/sources/gcc/gcc/testsuite/gcc.target/i386/pr83317.c:21:1:
internal compiler error: in lra_eliminate_reg_if_possible, at
lra-eliminations.c:1393
0xea4875 lra_eliminate_reg_if_possible(rtx_def**)
/export/gnu/import/git/sources/gcc/gcc/lra-eliminations.c:1393
0xe8a94c address_eliminator
/export/gnu/import/git/sources/gcc/gcc/lra-constraints.c:362
0xe8aaf7 satisfies_memory_constraint_p
/export/gnu/import/git/sources/gcc/gcc/lra-constraints.c:401
0xe8f947 process_alt_operands
/export/gnu/import/git/sources/gcc/gcc/lra-constraints.c:2248
0xe93d1f curr_insn_transform
/export/gnu/import/git/sources/gcc/gcc/lra-constraints.c:3861
0xe975f2 lra_constraints(bool)
/export/gnu/import/git/sources/gcc/gcc/lra-constraints.c:4878
0xe7c458 lra(_IO_FILE*)
/export/gnu/import/git/sources/gcc/gcc/lra.c:2446
0xe111ff do_reload
/export/gnu/import/git/sources/gcc/gcc/ira.c:5469
0xe116f2 execute
/export/gnu/import/git/sources/gcc/gcc/ira.c:5653
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See <https://gcc.gnu.org/bugs/> for instructions.
[hjl@gnu-4 gcc]$


-- 
H.J.
Thomas Preudhomme Oct. 4, 2018, 12:27 p.m. UTC | #3 
My bad, I used dg-cmp-results without verbosity which didn't show the
problem It starts to show it with -v -v, I'm not sure why. I'll have a
look right now and revert by the end of today if I cannot come up with
a fix. Does that sound ok?

Best regards,

Thomas
On Thu, 4 Oct 2018 at 12:31, H.J. Lu <hjl.tools@gmail.com> wrote:
>

> On Wed, Oct 3, 2018 at 8:12 PM Vladimir Makarov <vmakarov@redhat.com> wrote:

> >

> > On 10/03/2018 12:47 PM, Thomas Preudhomme wrote:

> > > Best regards,

> > >

> > > Thomas

> > >

> > > never_reload_fixed_address_operand.patch

> > >

> > >

> > >  From 2831d8b886d92513c2d30d43a6a989d2bbd0ceee Mon Sep 17 00:00:00 2001

> > > From: Thomas Preud'homme<thomas.preudhomme@linaro.org>

> > > Date: Thu, 27 Sep 2018 09:50:12 +0100

> > > Subject: [PATCH] [PATCH, LRA] Never reload fixed form constraints memory

> > >   operand

> > >

> > > Hi,

> > >

> > > The unconditional reload of address operand for recognized instruction

> > > in process_address_1 prevent the patch for fixing "PR85434: Address of

> > > stack protector guard spilled to stack on ARM" proposed at [1]. The code

> > > in this patch attempt to control which registers are used to make PIC

> > > access but the reload performed by process_address_1 will use generic

> > > PIC access. This patch removes the test for the instruction to be

> > > unrecognized to do the reload, thus always avoiding to reload address

> > > operand for fixed constraints (such as "X" used in the patch).

> > >

> > > [1]https://gcc.gnu.org/ml/gcc-patches/2018-08/msg01838.html

> > >

> > > ChangeLog entry is as follows:

> > >

> > > *** gcc/ChangeLog ***

> > >

> > > 2018-10-03  Thomas Preud'homme<thomas.preudhomme@linaro.org>

> > >

> > >       * lra-constraints.c (process_address_1): Bail out for all

> > >       satisfied fixed constraints.

> > >

> > > Testing: Successfully bootstrapped and regtested on:

> > > - arm-linux-gnueabihf (both Arm and Thumb2 mode)

> > > - aarch64-linux-gnu

> > > - x86_64-linux-gnu

> > > - i386-linux-gnu

> > > - sparc64-linux-gnu (gcc202)

> > > - powerpc64le-linux-gnu (gcc112)

> > >

> > > Is this ok for trunk?

> > >

> > OK. Thank you for testing all these targets, Thomas.

> >

>

> This caused:

>

> FAIL: gcc.target/i386/pr83317.c (internal compiler error)

> FAIL: gcc.target/i386/pr83317.c (test for excess errors)

>

> [hjl@gnu-4 gcc]$

> /export/build/gnu/tools-build/gcc-debug/build-x86_64-linux/gcc/xgcc

> -B/export/build/gnu/tools-build/gcc-debug/build-x86_64-linux/gcc/

> /export/gnu/import/git/sources/gcc/gcc/testsuite/gcc.target/i386/pr83317.c

>  -m32   -fno-diagnostics-show-caret -fno-diagnostics-show-line-numbers

> -fdiagnostics-color=never   -O1 -fPIC -msse2 -mfpmath=sse -S -o

> pr83317.s

> during RTL pass: reload

> /export/gnu/import/git/sources/gcc/gcc/testsuite/gcc.target/i386/pr83317.c:

> In function \u2018foo\u2019:

> /export/gnu/import/git/sources/gcc/gcc/testsuite/gcc.target/i386/pr83317.c:21:1:

> internal compiler error: in lra_eliminate_reg_if_possible, at

> lra-eliminations.c:1393

> 0xea4875 lra_eliminate_reg_if_possible(rtx_def**)

> /export/gnu/import/git/sources/gcc/gcc/lra-eliminations.c:1393

> 0xe8a94c address_eliminator

> /export/gnu/import/git/sources/gcc/gcc/lra-constraints.c:362

> 0xe8aaf7 satisfies_memory_constraint_p

> /export/gnu/import/git/sources/gcc/gcc/lra-constraints.c:401

> 0xe8f947 process_alt_operands

> /export/gnu/import/git/sources/gcc/gcc/lra-constraints.c:2248

> 0xe93d1f curr_insn_transform

> /export/gnu/import/git/sources/gcc/gcc/lra-constraints.c:3861

> 0xe975f2 lra_constraints(bool)

> /export/gnu/import/git/sources/gcc/gcc/lra-constraints.c:4878

> 0xe7c458 lra(_IO_FILE*)

> /export/gnu/import/git/sources/gcc/gcc/lra.c:2446

> 0xe111ff do_reload

> /export/gnu/import/git/sources/gcc/gcc/ira.c:5469

> 0xe116f2 execute

> /export/gnu/import/git/sources/gcc/gcc/ira.c:5653

> Please submit a full bug report,

> with preprocessed source if appropriate.

> Please include the complete backtrace with any bug report.

> See <https://gcc.gnu.org/bugs/> for instructions.

> [hjl@gnu-4 gcc]$

>

>

> --

> H.J.
Vladimir Makarov Oct. 4, 2018, 2:35 p.m. UTC | #4 
On 10/04/2018 08:27 AM, Thomas Preudhomme wrote:
> My bad, I used dg-cmp-results without verbosity which didn't show the

> problem It starts to show it with -v -v, I'm not sure why. I'll have a

> look right now and revert by the end of today if I cannot come up with

> a fix. Does that sound ok?

>

>

Yes, that is ok.  It is pretty normal situation with LRA bugs. Sometimes 
a fix needs several iterations as one person can not test all the targets.
diff mbox series

Patch

From 2831d8b886d92513c2d30d43a6a989d2bbd0ceee Mon Sep 17 00:00:00 2001
From: Thomas Preud'homme <thomas.preudhomme@linaro.org>
Date: Thu, 27 Sep 2018 09:50:12 +0100
Subject: [PATCH] [PATCH, LRA] Never reload fixed form constraints memory
 operand

Hi,

The unconditional reload of address operand for recognized instruction
in process_address_1 prevent the patch for fixing "PR85434: Address of
stack protector guard spilled to stack on ARM" proposed at [1]. The code
in this patch attempt to control which registers are used to make PIC
access but the reload performed by process_address_1 will use generic
PIC access. This patch removes the test for the instruction to be
unrecognized to do the reload, thus always avoiding to reload address
operand for fixed constraints (such as "X" used in the patch).

[1] https://gcc.gnu.org/ml/gcc-patches/2018-08/msg01838.html

ChangeLog entry is as follows:

*** gcc/ChangeLog ***

2018-10-03  Thomas Preud'homme  <thomas.preudhomme@linaro.org>

	* lra-constraints.c (process_address_1): Bail out for all
	satisfied fixed constraints.

Testing: Successfully bootstrapped and regtested on:
- arm-linux-gnueabihf (both Arm and Thumb2 mode)
- aarch64-linux-gnu
- x86_64-linux-gnu
- i386-linux-gnu
- sparc64-linux-gnu (gcc202)
- powerpc64le-linux-gnu (gcc112)

Is this ok for trunk?

Best regards,

Thomas
---
 gcc/lra-constraints.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/gcc/lra-constraints.c b/gcc/lra-constraints.c
index 774d1ff3aaa..c3edd9ef45d 100644
--- a/gcc/lra-constraints.c
+++ b/gcc/lra-constraints.c
@@ -3243,8 +3243,7 @@  process_address_1 (int nop, bool check_only_p,
   /* Do not attempt to decompose arbitrary addresses generated by combine
      for asm operands with loose constraints, e.g 'X'.  */
   else if (MEM_P (op)
-	   && !(INSN_CODE (curr_insn) < 0
-		&& get_constraint_type (cn) == CT_FIXED_FORM
+	   && !(get_constraint_type (cn) == CT_FIXED_FORM
 	        && constraint_satisfied_p (op, cn)))
     decompose_mem_address (&ad, op);
   else if (GET_CODE (op) == SUBREG
-- 
2.19.0