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 >