From: "Martin Liška" <mliska@suse.cz>
To: Segher Boessenkool <segher@kernel.crashing.org>
Cc: Jeff Law <law@redhat.com>, gcc-patches@gcc.gnu.org, dvlasenk@redhat.com
Subject: Re: [PATCH 3/3] Extend -falign-FOO=N to N[:M[:N2[:M2]]]
Date: Tue, 03 Jul 2018 10:16:00 -0000 [thread overview]
Message-ID: <504decd9-4f47-d467-2ace-77e028b6ed79@suse.cz> (raw)
In-Reply-To: <20180703095500.GC16221@gate.crashing.org>
On 07/03/2018 11:55 AM, Segher Boessenkool wrote:
> On Tue, Jul 03, 2018 at 10:53:20AM +0200, Martin Liška wrote:
>> On 06/29/2018 09:04 PM, Jeff Law wrote:
>>> I think this is fine for the trunk.
>>>
>>> jeff
>>
>> Thank you Jeff.
>>
>> I found some issues when doing build of all targets (contrib/config-list.mk).
>> I'll update patch and test that affected cross-compilers still produce same output.
>>
>> However I noticed one ppc64 issue:
>>
>> $ cat -n gcc/config/powerpcspe/powerpcspe.c
>>
>> 5401 /* Set branch target alignment, if not optimizing for size. */
>> 5402 if (!optimize_size)
>> 5403 {
>> 5404 /* Cell wants to be aligned 8byte for dual issue. Titan wants to be
>> 5405 aligned 8byte to avoid misprediction by the branch predictor. */
>> 5406 if (rs6000_cpu == PROCESSOR_TITAN
>> 5407 || rs6000_cpu == PROCESSOR_CELL)
>> 5408 {
>> 5409 if (align_functions <= 0)
>> 5410 align_functions = 8;
>> 5411 if (align_jumps <= 0)
>> 5412 align_jumps = 8;
>> 5413 if (align_loops <= 0)
>> 5414 align_loops = 8;
>> 5415 }
>> 5416 if (rs6000_align_branch_targets)
>> 5417 {
>> 5418 if (align_functions <= 0)
>> 5419 align_functions = 16;
>> 5420 if (align_jumps <= 0)
>> 5421 align_jumps = 16;
>> 5422 if (align_loops <= 0)
>> 5423 {
>> 5424 can_override_loop_align = 1;
>> 5425 align_loops = 16;
>> 5426 }
>> 5427 }
>> 5428 if (align_jumps_max_skip <= 0)
>> 5429 align_jumps_max_skip = 15;
>> 5430 if (align_loops_max_skip <= 0)
>> 5431 align_loops_max_skip = 15;
>>
>> Note that at line 5429 there's set of align_jumps_max_skip to 15 if not set by default.
>> At line 5412 align_jumps is set to 8, and align_jumps_max_skip should be equal align_jumps - 1.
>> That's a discrepancy. Segher can you please take a look?
>
> This is powerpcspe, that's not mine.
>
> But rs6000 has the same code, sure.
Right, that why I wrote to you.
> Why do you say "align_jumps_max_skip
> should be equal align_jumps - 1"? If that were true, why does it exist
> at all?
>
> toplev.c already has (in init_alignments):
>
> if (align_jumps_max_skip > align_jumps)
> align_jumps_max_skip = align_jumps - 1;
I'm rewriting this logic in the patch set. Issue is that
checking for value of align_jumps_max_skip is done
in rs6000_option_override_internal, which is place before
align_jumps_max_skip is parsed.
That said, 'align_jumps_max_skip <= 0' is always true.
Martin
>
> so why would targets duplicate that logic? (The target override is called
> before init_alignments).
>
>
> Segher
>
next prev parent reply other threads:[~2018-07-03 10:16 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-05-25 11:04 [PATCH 0/3] Extend -falign-FOO=N to N[,M[,N2[,M2]]] version 9 marxin
2018-05-25 11:04 ` [PATCH 3/3] Extend -falign-FOO=N to N[:M[:N2[:M2]]] marxin
2018-06-29 19:05 ` Jeff Law
2018-07-03 8:53 ` Martin Liška
2018-07-03 9:55 ` Segher Boessenkool
2018-07-03 10:16 ` Martin Liška [this message]
2018-07-03 10:58 ` Segher Boessenkool
2018-07-03 12:51 ` Martin Liška
2018-07-03 13:23 ` Segher Boessenkool
2018-07-03 19:12 ` Martin Liška
2018-07-04 0:20 ` Jeff Law
2018-05-25 11:04 ` [PATCH 1/3] Add vec::reverse marxin
2018-05-28 12:13 ` Richard Biener
2018-06-13 1:54 ` Jeff Law
2018-05-25 11:08 ` [PATCH 2/3] Temporary remove "at least 8 byte alignment" code from x86 marxin
2018-06-13 2:02 ` Jeff Law
2018-07-03 19:20 ` Martin Liška
-- strict thread matches above, loose matches on Subject: below --
2017-04-18 18:30 [PATCH 0/3] Extend -falign-FOO=N to N[,M[,N2[,M2]]] version 8 Denys Vlasenko
2017-04-18 18:46 ` [PATCH 3/3] Extend -falign-FOO=N to N[,M[,N2[,M2]]] Denys Vlasenko
2017-04-18 19:12 ` Sandra Loosemore
2017-04-17 15:57 [PATCH 0/3] Extend -falign-FOO=N to N[,M[,N2[,M2]]] version 7 Denys Vlasenko
2017-04-17 16:20 ` [PATCH 3/3] Extend -falign-FOO=N to N[,M[,N2[,M2]]] Denys Vlasenko
2017-04-17 20:02 ` Sandra Loosemore
2017-04-18 18:30 ` Denys Vlasenko
2016-10-12 20:53 [PATCH 0/3] Extend -falign-FOO=N to N[,M[,N2[,M2]]] version 4 Denys Vlasenko
2016-10-12 20:53 ` [PATCH 3/3] Extend -falign-FOO=N to N[,M[,N2[,M2]]] Denys Vlasenko
2016-09-30 17:55 [PATCH 0/3] Extend -falign-FOO=N to N[,M[,N2[,M2]]] version 4 Denys Vlasenko
2016-09-30 17:58 ` [PATCH 3/3] Extend -falign-FOO=N to N[,M[,N2[,M2]]] Denys Vlasenko
2016-10-06 9:47 ` Bernd Schmidt
2016-10-06 18:43 ` Denys Vlasenko
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=504decd9-4f47-d467-2ace-77e028b6ed79@suse.cz \
--to=mliska@suse.cz \
--cc=dvlasenk@redhat.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=law@redhat.com \
--cc=segher@kernel.crashing.org \
/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).