public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Jakub Jelinek <jakub@redhat.com>
To: Jason Merrill <jason@redhat.com>
Cc: Bernd Schmidt <bschmidt@redhat.com>,
	Eric Botcazou <ebotcazou@adacore.com>,
	       gcc-patches@gcc.gnu.org
Subject: Re: [PATCH] Fix expansion of TREE_ADDRESSABLE bitwise copies (PR c++/69851)
Date: Fri, 19 Feb 2016 18:41:00 -0000	[thread overview]
Message-ID: <20160219184122.GV3017@tucnak.redhat.com> (raw)
In-Reply-To: <56C75F5C.6010307@redhat.com>

On Fri, Feb 19, 2016 at 01:30:52PM -0500, Jason Merrill wrote:
> On 02/19/2016 09:03 AM, Jakub Jelinek wrote:
> >As described in the PR, in C++ we can have assignments
> >where both the lhs and rhs are COMPONENT_REFs with TREE_ADDRESSABLE types,
> >including padding, but the FIELD_DECLs are artificial fields that have
> >narrower bit sizes.
> >store_field in this case takes the path of bit-field handling (even when
> >it has bitpos and bitsize multiples of BITS_PER_UNIT (I think that is
> >necessarily true for the TREE_ADDRESSABLE types), which is incorrect,
> >because the rhs is expanded in that case through expand_normal, which
> >for a result type wider than the FIELD_DECL with forces it into a temporary.
> >In older GCCs that just generated inefficient code (copy the rhs into a
> >stack temporary, then copy that to lhs), but GCC trunk ICEs on that.
> >Fixed by not taking the bit-field path in that case after verifying
> >we'll be able to expand it properly using the normal store_expr.
> 
> Won't store_expr clobber tail padding because it doesn't know about bitsize?

It doesn't clobber it, because it uses get_inner_reference, expands the
inner reference (which is necessarily for something TREE_ADDRESSABLE either
a MEM_REF or some decl that lives in memory), and get_inner_reference in
that case gives it the bitsize/bitpos from the FIELD_DECL.
Which is why in the patch I've posted there is the comparison of DECL_SIZE
of the FIELD_DECL against the bitsize that is passed to store_field.

> I would think that what we want is to use emit_block_move in the bit-field
> path whenever we're dealing with whole bytes in memory.

I'm afraid we'd need to duplicate too much code if we'd wanted to bypass all
the layers in there.

	Jakub

  reply	other threads:[~2016-02-19 18:41 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-02-19 14:03 Jakub Jelinek
2016-02-19 18:30 ` Jason Merrill
2016-02-19 18:41   ` Jakub Jelinek [this message]
2016-02-19 19:00     ` Jason Merrill
2016-02-19 19:07       ` Jakub Jelinek
2016-02-19 19:11         ` Jason Merrill
2016-02-19 18:42 Bernd Edlinger
2016-02-19 19:04 ` Jakub Jelinek
2016-02-19 20:04   ` Bernd Edlinger
2016-02-19 20:08     ` Jakub Jelinek
2016-02-19 20:18       ` Bernd Edlinger
2016-02-19 20:45       ` Bernd Edlinger
2016-02-19 20:57         ` Jakub Jelinek
2016-02-19 20:46       ` Bernd Edlinger

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160219184122.GV3017@tucnak.redhat.com \
    --to=jakub@redhat.com \
    --cc=bschmidt@redhat.com \
    --cc=ebotcazou@adacore.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jason@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).