Message ID | AM6PR10MB2566FFE1880C451C49F3DD08E4BF0@AM6PR10MB2566.EURPRD10.PROD.OUTLOOK.COM |
---|---|
State | New |
Headers | show |
Series | Fix PR 91605 | expand |
On Sun, 1 Sep 2019, Bernd Edlinger wrote: > Hi, > > this fixes an oversight in r274986. > We need to avoid using movmisalign on DECL_P which are not in memory, > similar to the !mem_ref_refers_to_non_mem_p which unfortunately can't > handle DECL_P. > But - && (DECL_P (to) || !mem_ref_refers_to_non_mem_p (to)) + && (DECL_P (to) ? MEM_P (DECL_RTL (to)) + : !mem_ref_refers_to_non_mem_p (to)) and in mem_ref_refers_to_non_mem_p we do if (!DECL_RTL_SET_P (base)) return nortl; return (!MEM_P (DECL_RTL (base))); so when !DECL_RTL_SET_P (t) we can go full speed ahead? That said, can we refactor addr_expr_of_non_mem_decl_p_1 to put if (TREE_CODE (addr) != ADDR_EXPR) return false; tree base = TREE_OPERAND (addr, 0); into the single caller and re-use it then also for the DECL_P case? Thanks, Richard. > Bootstrapped and reg-tested on x86_64-pc-linux-gnu. > Is it OK for trunk? > > > Thanks > Bernd. >
On 9/2/19 9:50 AM, Richard Biener wrote: > On Sun, 1 Sep 2019, Bernd Edlinger wrote: > >> Hi, >> >> this fixes an oversight in r274986. >> We need to avoid using movmisalign on DECL_P which are not in memory, >> similar to the !mem_ref_refers_to_non_mem_p which unfortunately can't >> handle DECL_P. >> > > But > > - && (DECL_P (to) || !mem_ref_refers_to_non_mem_p (to)) > + && (DECL_P (to) ? MEM_P (DECL_RTL (to)) > + : !mem_ref_refers_to_non_mem_p (to)) > > and in mem_ref_refers_to_non_mem_p we do > > if (!DECL_RTL_SET_P (base)) > return nortl; > > return (!MEM_P (DECL_RTL (base))); > > so when !DECL_RTL_SET_P (t) we can go full speed ahead? That said, > can we refactor addr_expr_of_non_mem_decl_p_1 to put > Ah, I was not aware that DECL_RTL has a side-effect if !DECL_RTL_SET_P. > if (TREE_CODE (addr) != ADDR_EXPR) > return false; > > tree base = TREE_OPERAND (addr, 0); > > into the single caller and re-use it then also for the DECL_P case? > Yes, that is probably better then. So how about this? Is it OK? Thanks Bernd.
On Mon, 2 Sep 2019, Bernd Edlinger wrote: > On 9/2/19 9:50 AM, Richard Biener wrote: > > On Sun, 1 Sep 2019, Bernd Edlinger wrote: > > > >> Hi, > >> > >> this fixes an oversight in r274986. > >> We need to avoid using movmisalign on DECL_P which are not in memory, > >> similar to the !mem_ref_refers_to_non_mem_p which unfortunately can't > >> handle DECL_P. > >> > > > > But > > > > - && (DECL_P (to) || !mem_ref_refers_to_non_mem_p (to)) > > + && (DECL_P (to) ? MEM_P (DECL_RTL (to)) > > + : !mem_ref_refers_to_non_mem_p (to)) > > > > and in mem_ref_refers_to_non_mem_p we do > > > > if (!DECL_RTL_SET_P (base)) > > return nortl; > > > > return (!MEM_P (DECL_RTL (base))); > > > > so when !DECL_RTL_SET_P (t) we can go full speed ahead? That said, > > can we refactor addr_expr_of_non_mem_decl_p_1 to put > > > > Ah, I was not aware that DECL_RTL has a side-effect if !DECL_RTL_SET_P. > > > > if (TREE_CODE (addr) != ADDR_EXPR) > > return false; > > > > tree base = TREE_OPERAND (addr, 0); > > > > into the single caller and re-use it then also for the DECL_P case? > > > > Yes, that is probably better then. > > So how about this? > Is it OK? OK. Thanks, Richard.
On 9/1/19 4:36 AM, Bernd Edlinger wrote: > Hi, > > this fixes an oversight in r274986. > We need to avoid using movmisalign on DECL_P which are not in memory, > similar to the !mem_ref_refers_to_non_mem_p which unfortunately can't > handle DECL_P. > > > Bootstrapped and reg-tested on x86_64-pc-linux-gnu. > Is it OK for trunk? > > > Thanks > Bernd. > > > patch-pr91605.diff > > 2019-09-01 Bernd Edlinger <bernd.edlinger@hotmail.de> > > PR middle-end/91605 > * expr.c (expand_assignment): Avoid DECL_P with DECL_RTL > not being MEM_P. > > testsuite: > 2019-09-01 Bernd Edlinger <bernd.edlinger@hotmail.de> > > PR middle-end/91605 > * g++.target/i386/pr91605.C: New test. OK. Jeff
On 9/3/19 8:40 PM, Jeff Law wrote: > On 9/1/19 4:36 AM, Bernd Edlinger wrote: >> Hi, >> >> this fixes an oversight in r274986. >> We need to avoid using movmisalign on DECL_P which are not in memory, >> similar to the !mem_ref_refers_to_non_mem_p which unfortunately can't >> handle DECL_P. >> >> >> Bootstrapped and reg-tested on x86_64-pc-linux-gnu. >> Is it OK for trunk? >> >> >> Thanks >> Bernd. >> >> >> patch-pr91605.diff >> >> 2019-09-01 Bernd Edlinger <bernd.edlinger@hotmail.de> >> >> PR middle-end/91605 >> * expr.c (expand_assignment): Avoid DECL_P with DECL_RTL >> not being MEM_P. >> >> testsuite: >> 2019-09-01 Bernd Edlinger <bernd.edlinger@hotmail.de> >> >> PR middle-end/91605 >> * g++.target/i386/pr91605.C: New test. > OK. > Jeff > Thanks Jeff, but I have already applied a slightly improved patch which has approved by Richi: https://gcc.gnu.org/ml/gcc-patches/2019-09/msg00059.html Bernd.
On 9/3/19 12:46 PM, Bernd Edlinger wrote: > On 9/3/19 8:40 PM, Jeff Law wrote: >> On 9/1/19 4:36 AM, Bernd Edlinger wrote: >>> Hi, >>> >>> this fixes an oversight in r274986. >>> We need to avoid using movmisalign on DECL_P which are not in memory, >>> similar to the !mem_ref_refers_to_non_mem_p which unfortunately can't >>> handle DECL_P. >>> >>> >>> Bootstrapped and reg-tested on x86_64-pc-linux-gnu. >>> Is it OK for trunk? >>> >>> >>> Thanks >>> Bernd. >>> >>> >>> patch-pr91605.diff >>> >>> 2019-09-01 Bernd Edlinger <bernd.edlinger@hotmail.de> >>> >>> PR middle-end/91605 >>> * expr.c (expand_assignment): Avoid DECL_P with DECL_RTL >>> not being MEM_P. >>> >>> testsuite: >>> 2019-09-01 Bernd Edlinger <bernd.edlinger@hotmail.de> >>> >>> PR middle-end/91605 >>> * g++.target/i386/pr91605.C: New test. >> OK. >> Jeff >> > > Thanks Jeff, > > but I have already applied a slightly improved patch which > has approved by Richi: > > https://gcc.gnu.org/ml/gcc-patches/2019-09/msg00059.html OK. Go with that one instead :-) I'd looked at that one too, noticed the ChangeLog didn't seem to match the patch well and set it aside to dig further into. jeff
2019-09-01 Bernd Edlinger <bernd.edlinger@hotmail.de> PR middle-end/91605 * expr.c (expand_assignment): Avoid DECL_P with DECL_RTL not being MEM_P. testsuite: 2019-09-01 Bernd Edlinger <bernd.edlinger@hotmail.de> PR middle-end/91605 * g++.target/i386/pr91605.C: New test. Index: gcc/expr.c =================================================================== --- gcc/expr.c (revision 275063) +++ gcc/expr.c (working copy) @@ -5004,7 +5004,8 @@ expand_assignment (tree to, tree from, bool nontem || TREE_CODE (to) == TARGET_MEM_REF || DECL_P (to)) && mode != BLKmode - && (DECL_P (to) || !mem_ref_refers_to_non_mem_p (to)) + && (DECL_P (to) ? MEM_P (DECL_RTL (to)) + : !mem_ref_refers_to_non_mem_p (to)) && ((align = get_object_alignment (to)) < GET_MODE_ALIGNMENT (mode)) && (((icode = optab_handler (movmisalign_optab, mode)) Index: gcc/testsuite/g++.target/i386/pr91605.C =================================================================== --- gcc/testsuite/g++.target/i386/pr91605.C (revision 0) +++ gcc/testsuite/g++.target/i386/pr91605.C (working copy) @@ -0,0 +1,13 @@ +/* { dg-do compile } */ +/* { dg-options "-fpack-struct -mavx" } */ + +struct A { + __attribute__((__vector_size__(4 * sizeof(double)))) double data; +}; +struct B { + A operator*(B); +}; +void fn1() { + B x, y; + x *y; +}