From: Bernd Edlinger <bernd.edlinger@hotmail.de>
To: Eric Botcazou <ebotcazou@adacore.com>
Cc: Richard Biener <rguenther@suse.de>,
"gcc-patches@gcc.gnu.org" <gcc-patches@gcc.gnu.org>,
Jeff Law <law@redhat.com>, Jakub Jelinek <jakub@redhat.com>
Subject: AW: [PATCH] Fix unaligned access when predictive commoning (PR 71083)
Date: Wed, 10 Aug 2016 08:47:00 -0000 [thread overview]
Message-ID: <AM4PR0701MB2162AF8FCA628D85B77F25C2E41D0@AM4PR0701MB2162.eurprd07.prod.outlook.com> (raw)
In-Reply-To: <AM4PR0701MB21629444A982758619D3B0C6E41C0@AM4PR0701MB2162.eurprd07.prod.outlook.com>
On 08/10/16, Bernd Edlinger wrote:
>On 08/09/16 22:48, Eric Botcazou wrote:
>>> I think from Eric's comment in get_inner_ref it can be possible
>>> in Ada that the outer object is accidentally byte-aligned
>>> but the inner reference is not. In that case I think it is
>>> better to generate a BIT_FIELD_REF instead of a COMPONENT_REF.
>>
I actually meant to say get_bit_range.
I tried to translate the c-test case to an equivalent ada test case with
my not-so-fluent Ada speak...
So here is a test case that *actually* hits the if ((boff % BITS_PER_UNIT) != 0)
code path.
Before my patch there was an unaligned SImode access, and with
the patch we have 3 QImode accesses here.
cat pr71083_pkg.ads
package Pr71083_pkg is
type Nibble is mod 2**4;
type Int24 is mod 2**24;
type StructA is record
a : Nibble;
b : Int24;
end record;
pragma Pack(StructA);
type StructB is record
a : Nibble;
b : StructA;
end record;
pragma Pack(StructB);
type ArrayOfStructB is array(0..100) of StructB;
procedure Foo (X : in out ArrayOfStructB);
end Pr71083_pkg;
cat pr71083_pkg.adb
-- { dg-do compile }
-- { dg-options "-O3" }
package body Pr71083_pkg is
procedure Foo (X : in out ArrayOfStructB) is
begin
for K in 0..99 loop
X (K+1).b.b := X (K).b.b;
end loop;
end Foo;
end Pr71083_pkg;
cat pr71083.adb
-- { dg-do run }
-- { dg-options "-O3" }
with Pr71083_pkg;
use Pr71083_pkg;
procedure Pr71083 is
Test : ArrayOfStructB;
begin
Test (0).b.b := 9999;
Foo (Test);
if Test (100).b.b /= 9999 then
raise Program_Error;
end if;
end;
I was not sure which name to choose,
so I used the same convention as in the c-torture.
How do you like that?
I would like to add that to the gnat.dg because
it seems that there is zero testing for the predictive
commoning in the gnat.dg test suite.
Bernd.
>>> The patch says get_bit_range instead... The comment therein means that
>>> bitfields can be nested in Ada: you can have a bitfield in a record which is
>>> itself a bitfield in another record.
>>>
>>>> Eric do you agree? Are there any Ada test cases where the
>>>> pcom optimization jumps in?
>>>
>>> According to the comment, data-ref analysis punts on bit offsets so I'm not
>>> sure how boff can be not byte-aligned.
>>>
>
> I mean in dr_analyze_innermost, we have:
>
> base = get_inner_reference (ref, &pbitsize, &pbitpos, &poffset, &pmode,
> &punsignedp, &preversep, &pvolatilep);
> gcc_assert (base != NULL_TREE);
>
> if (pbitpos % BITS_PER_UNIT != 0)
> {
> if (dump_file && (dump_flags & TDF_DETAILS))
> fprintf (dump_file, "failed: bit offset alignment.\n");
> return false;
> }
>
> if (preversep)
> {
> if (dump_file && (dump_flags & TDF_DETAILS))
> fprintf (dump_file, "failed: reverse storage order.\n");
> return false;
> }
>
>
> and that means that get_inner_reference on the outermost ref
> returns a byte-aligned bit-offset, and from there I would
> think it has the knowledge, when to punt on reversep and
> when the offset is not byte-aligned.
>
> But in get_bit_range we have a bit-field with arbitrary
> bit-offset, but surprisingly the
> get_inner_reference (TREE_OPERAND (exp, 0)) did not return
> byte-aligned bit-offset. I doubt that data-ref ananlysis
> ever cares about the byte-alignment of intermediate
> refs.
>
> We know that the difference between the innermost ref
> and the outer ref is byte-aligned, but we do not know
> that the same is true for offset between the COMPONENT_REF
> and the outer ref.
>
> I mean boff is essentially the difference between
> bitpos of get_inner_reference(exp) and
> bitpos of get_inner_reference(TREE_OPERAND (exp, 0))
>
> This would be exposed by my patch, because previously
> we always generated BIT_FIELD_REFS, with bit-offset 0,
> but the MEM_REF at the byte-offset there is in all likelihood
> pretty much unaligned, the MEM_REF at the COMPONENT_REF
> is likely more aligned and allows better code for ARM processors,
> but only if the MEM_REF is at a byte-aligned offset at all,
> otherwise the whole transformation would be wrong.
>
>
>
> Thanks
> Bernd.
next prev parent reply other threads:[~2016-08-10 8:47 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-08-08 19:57 Bernd Edlinger
2016-08-09 7:29 ` Richard Biener
2016-08-09 17:31 ` Bernd Edlinger
2016-08-09 20:48 ` Eric Botcazou
2016-08-09 22:23 ` Bernd Edlinger
2016-08-10 8:47 ` Bernd Edlinger [this message]
2016-08-10 12:19 ` AW: " Eric Botcazou
2016-08-10 12:29 ` Richard Biener
2016-08-10 16:24 ` Bernd Edlinger
2016-08-11 7:07 ` Richard Biener
2016-08-11 10:09 ` Bernd Edlinger
2016-08-11 10:30 ` Richard Biener
2016-08-11 19:47 ` [PATCH] Increase alignment for bit-field " Bernd Edlinger
2016-08-12 7:13 ` Richard Biener
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=AM4PR0701MB2162AF8FCA628D85B77F25C2E41D0@AM4PR0701MB2162.eurprd07.prod.outlook.com \
--to=bernd.edlinger@hotmail.de \
--cc=ebotcazou@adacore.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=jakub@redhat.com \
--cc=law@redhat.com \
--cc=rguenther@suse.de \
/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).