This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH 3/3] Extend -falign-FOO=N to N[:M[:N2[:M2]]]
- From: Segher Boessenkool <segher at kernel dot crashing dot org>
- To: Martin Liška <mliska at suse dot cz>
- Cc: Jeff Law <law at redhat dot com>, gcc-patches at gcc dot gnu dot org, dvlasenk at redhat dot com
- Date: Tue, 3 Jul 2018 04:55:00 -0500
- Subject: Re: [PATCH 3/3] Extend -falign-FOO=N to N[:M[:N2[:M2]]]
- References: <cover.1527245880.git.mliska@suse.cz> <afb1c0abe1094cbe50a77c02a343d8e5b0b467a0.1527245880.git.mliska@suse.cz> <9bb1dbf4-b0ef-544c-fb02-b0262a39b87d@redhat.com> <e7326830-9116-b8ba-7e60-7fa5cc3a440d@suse.cz>
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. 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;
so why would targets duplicate that logic? (The target override is called
before init_alignments).
Segher