On Wed, 2020-07-01 at 21:48 +0200, Ilya Leoshkevich wrote: > On Wed, 2020-07-01 at 11:57 -0600, Jeff Law wrote: > > On Wed, 2020-07-01 at 14:29 +0200, Ilya Leoshkevich via Gcc-patches > > wrote: > > > gcc/ChangeLog: > > > > > > 2020-06-30 Ilya Leoshkevich <i...@linux.ibm.com> > > > > > > * asan.c (asan_emit_stack_protection): Use CODE_LABEL_BOUNDARY. > > > * defaults.h (CODE_LABEL_BOUNDARY): New macro. > > > * doc/tm.texi: Document CODE_LABEL_BOUNDARY. > > > * doc/tm.texi.in: Likewise. > > Don't we already have the ability to set label alignments? See > > LABEL_ALIGN. > > The following works with -falign-labels=2: > > --- a/gcc/asan.c > +++ b/gcc/asan.c > @@ -1524,7 +1524,7 @@ asan_emit_stack_protection (rtx base, rtx > pbase, > unsigned int alignb, > DECL_INITIAL (decl) = decl; > TREE_ASM_WRITTEN (decl) = 1; > TREE_ASM_WRITTEN (id) = 1; > - SET_DECL_ALIGN (decl, CODE_LABEL_BOUNDARY); > + SET_DECL_ALIGN (decl, (1 << LABEL_ALIGN (gen_label_rtx ())) * > BITS_PER_UNIT); > emit_move_insn (mem, expand_normal (build_fold_addr_expr (decl))); > shadow_base = expand_binop (Pmode, lshr_optab, base, > gen_int_shift_amount (Pmode, > ASAN_SHADOW_SHIFT), > > In order to go this way, we would need to raise `-falign-labels=` > default to 2 for s390, which is not incorrect, but would > unnecessarily > clutter asm with `.align 2` before each label. So IMHO it would be > nicer to simply ask the backend "what is your target's instruction > alignment?".
Besides that it would clutter asm with .align 2, another argument against using LABEL_ALIGN here is that it's semantically different from what is needed: -falign-labels value, which it returns, is specified by user for optimization purposes, whereas here we need to query the architecture's property. In practical terms, if user specifies -falign-labels=4096, this would affect how the code is generated here. However, this would be completely unnecessary: we never jump to decl, its address is only saved for reporting. Best regards, Ilya
