From: Kevin O'Connor <kevin@koconnor.net>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: Peter Maydell <peter.maydell@linaro.org>,
Richard Henderson <rth@twiddle.net>,
Laszlo Ersek <lersek@redhat.com>,
QEMU Developers <qemu-devel@nongnu.org>,
Eduardo Habkost <ehabkost@redhat.com>
Subject: Re: [Qemu-devel] [PATCH 1/2] target-i386: Use 1UL for bit shift
Date: Sat, 3 Oct 2015 22:34:17 -0400 [thread overview]
Message-ID: <20151004023417.GA13062@morn.lan> (raw)
In-Reply-To: <560E7384.9000901@redhat.com>
On Fri, Oct 02, 2015 at 02:07:32PM +0200, Paolo Bonzini wrote:
> On 02/10/2015 13:14, Laszlo Ersek wrote:
> > On 10/02/15 10:34, Paolo Bonzini wrote:
> >> On 01/10/2015 21:17, Laszlo Ersek wrote:
> >>> - In the firmware, allocate an array of bytes, dynamically. This array
> >>> will have no declared type.
> >>>
> >>> - Populate the array byte-wise, from fw_cfg. Because the stores happen
> >>> through character-typed lvalues, they do not "imbue" the target
> >>> object with any effective type, for further accesses that do not
> >>> modify the value. (I.e., for further reads.)
> >>>
> >>> - Get a (uint8_t*) into the array somewhere, and cast it to
> >>> (struct acpi_table_hdr *). Read fields through the cast pointer.
> >>> Assuming no out-of-bounds situation (considering the entire
> >>> pointed to acpi_table_hdr struct), and assuming no alignment
> >>> violations for the fields (which is implementation-defined), these
> >>> accesses will be fine.
> >>>
> >>> *However*. If in point 2 you populate the array with uint64_t accesses,
> >>> that *does* imbue the array elements with an effective type that is
> >>> binding for further read accesses.
> >>
> >> Then don't do it. Use memcpy from uint64_t to the array.
> >
> > It won't work; memcpy() propagates the effective type.
>
> Doh. I guess that's another "not in practice" case. Saying "memcpy to
> {,u}int8_t doesn't propagate the effective type" would probably go to
> great lengths towards fixing this.
Just to be pedantic, uint8_t/int8_t are not the same as 'char' wrt
aliasing rules. (The standard defines writes to a char array/pointer
as being allowed to alias with other types, but does not say that
about int8_t.) Gcc currently treats them as the same; I actually
tried to get gcc to change that a few months ago:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66110#c13
FWIW, I think the aliasing rules allow for very useful optimizations
and I wouldn't want to turn them off for programs where performance is
important.
The test case in the bug link above (which the gcc developers
thankfully did address!) is a good example of the utility of alias
detection. This function:
void func(struct s2 *p)
{
p->p1->f2 = 9;
p->p1->f2 = 10;
}
can't be optimized without -fstrict-aliasing. Indeed, even if the
code was changed to p->p1->f3 = 11; p->p1->f4 = 12; then gcc would
still need to reload p->p1 after every store. That's just silly.
-Kevin
next prev parent reply other threads:[~2015-10-04 2:34 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-09-29 20:34 [Qemu-devel] [PATCH 0/2] target-i386: Fix undefined behavior on bit shifts Eduardo Habkost
2015-09-29 20:34 ` [Qemu-devel] [PATCH 1/2] target-i386: Use 1UL for bit shift Eduardo Habkost
2015-09-30 13:27 ` Paolo Bonzini
2015-09-30 20:24 ` Richard Henderson
2015-10-01 8:29 ` Paolo Bonzini
2015-10-01 9:24 ` Peter Maydell
2015-10-01 13:52 ` Paolo Bonzini
2015-10-01 17:07 ` Laszlo Ersek
2015-10-01 17:30 ` Paolo Bonzini
2015-10-01 17:38 ` Peter Maydell
2015-10-01 19:17 ` Laszlo Ersek
2015-10-02 8:34 ` Paolo Bonzini
2015-10-02 11:14 ` Laszlo Ersek
2015-10-02 12:07 ` Paolo Bonzini
2015-10-04 2:34 ` Kevin O'Connor [this message]
2015-10-01 20:35 ` Markus Armbruster
2015-10-01 18:40 ` Laszlo Ersek
2015-10-02 8:48 ` Paolo Bonzini
2015-09-29 20:34 ` [Qemu-devel] [PATCH 2/2] target-i386: Don't left shift negative constant Eduardo Habkost
2015-10-01 1:35 ` Richard Henderson
2015-10-01 17:06 ` Eduardo Habkost
2015-10-23 15:07 ` Eduardo Habkost
2015-10-23 18:20 ` Richard Henderson
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=20151004023417.GA13062@morn.lan \
--to=kevin@koconnor.net \
--cc=ehabkost@redhat.com \
--cc=lersek@redhat.com \
--cc=pbonzini@redhat.com \
--cc=peter.maydell@linaro.org \
--cc=qemu-devel@nongnu.org \
--cc=rth@twiddle.net \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.