On Thu, 2020-07-09 at 14:07 +0200, Ilya Leoshkevich wrote: > 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 <[email protected]> > > > > > > > > * 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.
Hi Jeff, Could you please have another look at this one? Best regards, Ilya
