* [PATCH] Fix PR 91605 @ 2019-09-01 10:36 Bernd Edlinger 2019-09-02 7:50 ` Richard Biener 2019-09-03 18:40 ` Jeff Law 0 siblings, 2 replies; 7+ messages in thread From: Bernd Edlinger @ 2019-09-01 10:36 UTC (permalink / raw) To: gcc-patches, Richard Biener, Jeff Law [-- Attachment #1: Type: text/plain, Size: 300 bytes --] 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. [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: patch-pr91605.diff --] [-- Type: text/x-patch; name="patch-pr91605.diff", Size: 1373 bytes --] 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; +} ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Fix PR 91605 2019-09-01 10:36 [PATCH] Fix PR 91605 Bernd Edlinger @ 2019-09-02 7:50 ` Richard Biener 2019-09-02 14:02 ` Bernd Edlinger 2019-09-03 18:40 ` Jeff Law 1 sibling, 1 reply; 7+ messages in thread From: Richard Biener @ 2019-09-02 7:50 UTC (permalink / raw) To: Bernd Edlinger; +Cc: gcc-patches, Jeff Law [-- Attachment #1: Type: text/plain, Size: 1135 bytes --] 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. > -- Richard Biener <rguenther@suse.de> SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany; GF: Felix Imendörffer; HRB 247165 (AG München) ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Fix PR 91605 2019-09-02 7:50 ` Richard Biener @ 2019-09-02 14:02 ` Bernd Edlinger 2019-09-02 14:04 ` Richard Biener 0 siblings, 1 reply; 7+ messages in thread From: Bernd Edlinger @ 2019-09-02 14:02 UTC (permalink / raw) To: Richard Biener; +Cc: gcc-patches, Jeff Law [-- Attachment #1: Type: text/plain, Size: 1145 bytes --] 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. [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: patch-pr91605.diff --] [-- Type: text/x-patch; name="patch-pr91605.diff", Size: 3203 bytes --] 2019-09-01 Bernd Edlinger <bernd.edlinger@hotmail.de> PR middle-end/91605 * expr.c (addr_expr_of_non_mem_decl_p_1): Refactor into... (non_mem_decl_p): ...this. (mem_ref_refers_to_non_mem_p): Handle DECL_P as well ase MEM_REF. (expand_assignment): Call mem_ref_referes_to_non_mem_p unconditionally as before. 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 275279) +++ gcc/expr.c (working copy) @@ -4942,18 +4942,13 @@ get_bit_range (poly_uint64_pod *bitstart, poly_uin *bitend = *bitstart + tree_to_poly_uint64 (DECL_SIZE (repr)) - 1; } -/* Returns true if ADDR is an ADDR_EXPR of a DECL that does not reside - in memory and has non-BLKmode. DECL_RTL must not be a MEM; if - DECL_RTL was not set yet, return NORTL. */ +/* Returns true if BASE is a DECL that does not reside in memory and + has non-BLKmode. DECL_RTL must not be a MEM; if + DECL_RTL was not set yet, return false. */ static inline bool -addr_expr_of_non_mem_decl_p_1 (tree addr, bool nortl) +non_mem_decl_p (tree base) { - if (TREE_CODE (addr) != ADDR_EXPR) - return false; - - tree base = TREE_OPERAND (addr, 0); - if (!DECL_P (base) || TREE_ADDRESSABLE (base) || DECL_MODE (base) == BLKmode) @@ -4960,19 +4955,33 @@ static inline bool return false; if (!DECL_RTL_SET_P (base)) - return nortl; + return false; return (!MEM_P (DECL_RTL (base))); } -/* Returns true if the MEM_REF REF refers to an object that does not +/* Returns true if REF refers to an object that does not reside in memory and has non-BLKmode. */ static inline bool mem_ref_refers_to_non_mem_p (tree ref) { - tree base = TREE_OPERAND (ref, 0); - return addr_expr_of_non_mem_decl_p_1 (base, false); + tree base; + + if (TREE_CODE (ref) == MEM_REF + || TREE_CODE (ref) == TARGET_MEM_REF) + { + tree addr = TREE_OPERAND (ref, 0); + + if (TREE_CODE (addr) != ADDR_EXPR) + return false; + + base = TREE_OPERAND (addr, 0); + } + else + base = ref; + + return non_mem_decl_p (base); } /* Expand an assignment that stores the value of FROM into TO. If NONTEMPORAL @@ -5004,7 +5013,7 @@ 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)) + && !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; +} ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Fix PR 91605 2019-09-02 14:02 ` Bernd Edlinger @ 2019-09-02 14:04 ` Richard Biener 0 siblings, 0 replies; 7+ messages in thread From: Richard Biener @ 2019-09-02 14:04 UTC (permalink / raw) To: Bernd Edlinger; +Cc: gcc-patches, Jeff Law 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. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Fix PR 91605 2019-09-01 10:36 [PATCH] Fix PR 91605 Bernd Edlinger 2019-09-02 7:50 ` Richard Biener @ 2019-09-03 18:40 ` Jeff Law 2019-09-03 18:46 ` Bernd Edlinger 1 sibling, 1 reply; 7+ messages in thread From: Jeff Law @ 2019-09-03 18:40 UTC (permalink / raw) To: Bernd Edlinger, gcc-patches, Richard Biener 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Fix PR 91605 2019-09-03 18:40 ` Jeff Law @ 2019-09-03 18:46 ` Bernd Edlinger 2019-09-03 18:47 ` Jeff Law 0 siblings, 1 reply; 7+ messages in thread From: Bernd Edlinger @ 2019-09-03 18:46 UTC (permalink / raw) To: Jeff Law, gcc-patches, Richard Biener 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. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Fix PR 91605 2019-09-03 18:46 ` Bernd Edlinger @ 2019-09-03 18:47 ` Jeff Law 0 siblings, 0 replies; 7+ messages in thread From: Jeff Law @ 2019-09-03 18:47 UTC (permalink / raw) To: Bernd Edlinger, gcc-patches, Richard Biener 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-09-03 18:47 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-09-01 10:36 [PATCH] Fix PR 91605 Bernd Edlinger 2019-09-02 7:50 ` Richard Biener 2019-09-02 14:02 ` Bernd Edlinger 2019-09-02 14:04 ` Richard Biener 2019-09-03 18:40 ` Jeff Law 2019-09-03 18:46 ` Bernd Edlinger 2019-09-03 18:47 ` Jeff Law
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).