> On Sat, Jan 13, 2018 at 7:56 AM, Jan Hubicka <hubi...@ucw.cz> wrote: > >> >> +/* Output a funtion with a call and return thunk for indirect branch. > >> >> + If BND_P is true, the BND prefix is needed. If REGNO != -1, the > >> >> + function address is in REGNO. Otherwise, the function address is > >> >> + on the top of stack. */ > >> >> + > >> >> +static void > >> >> +output_indirect_thunk_function (bool need_bnd_p, int regno) > >> >> +{ > >> >> + char name[32]; > >> >> + tree decl; > >> >> + > >> >> + /* Create __x86_indirect_thunk/__x86_indirect_thunk_bnd. */ > >> >> + indirect_thunk_name (name, regno, need_bnd_p); > >> >> + decl = build_decl (BUILTINS_LOCATION, FUNCTION_DECL, > >> >> + get_identifier (name), > >> >> + build_function_type_list (void_type_node, > >> >> NULL_TREE)); > >> >> + DECL_RESULT (decl) = build_decl (BUILTINS_LOCATION, RESULT_DECL, > >> >> + NULL_TREE, void_type_node); > >> >> + TREE_PUBLIC (decl) = 1; > >> >> + TREE_STATIC (decl) = 1; > >> >> + DECL_IGNORED_P (decl) = 1; > >> > > >> > DECL_ARTIFICIAL as well? > >> > >> This is done exactly the same way as PIC thunk. I don't think we > >> should change it here. > >> > > >> > Why do you need to output asm visibility directives by hand when you > >> > create > >> > symbol for the function anyway? > >> > >> This is done exactly the same way as PIC thunk. I don't think we > >> should change it here. > > > > I see, it is pretty ancient code. Perhaps you can at least commonize > > the uglness so we don't duplicate the ifdefs for MACHO? > > I don't think we should such surgery at such late stage. I prefer to > keep it for later cleanup.
OK, lets keep it as it is and clean up next stage1. Honza > > >> > >> > I would expect that you can just use similar code as > >> > cgraph_node::expand_thunk > >> > when calls output_mi_thunk and get this done in a way that is > >> > independent of > >> > target assembler. > >> > >> I took a look. I don't see an easy to do it. I'd like to keep it > >> exactly the > >> same as PIC thunk. And we change both thunks together later if needed. > > > > OK, lets keep it done same was as PIC thunk. > > Next stage1 we could clean up both. > >> > I suppose it may make sense to split the insn and at least explicitly > >> > represent > >> > the fact that we (sometimes) push the target to stack. > >> > >> Did you mean "split the function"? I will break it into > >> ix86_output_indirect_branch_via_reg and > >> ix86_output_indirect_branch_via_push. > > > > I mean define_split to insert push instruction into the instruction > > stream rather then printing it as part of the indirect jump insn. > > We don't want anything added between these instructions. > Split it may lead to trouble. > > >> > >> > Why the memory variant exists at first place? > >> > >> When the function address is in memory, we can push it onto stack > >> to save a register. Also it is needed to cover "call foo" to "call > >> [foo@GOT]" > >> conversion. > >> > > >> > I think you also want to update type to "many" when we do more than just > >> > indirect branch. > >> > >> Did you mean "multi"? I will change to "multi". > > > > Yes. > > > >> > >> > We do not care much about this, but it feels wrong to have attributes > >> > off reality. > >> >> diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi > >> >> index f3e4a63ab46..ddb6035be96 100644 > >> >> --- a/gcc/doc/extend.texi > >> >> +++ b/gcc/doc/extend.texi > >> >> @@ -5754,6 +5754,16 @@ Specify which floating-point unit to use. You > >> >> must specify the > >> >> @code{target("fpmath=sse+387")} because the comma would separate > >> >> different options. > >> >> > >> >> +@item indirect_branch("@var{choice}") > >> >> +@cindex @code{indirect_branch} function attribute, x86 > >> >> +On x86 targets, the @code{indirect_branch} attribute causes the > >> >> compiler > >> >> +to convert indirect call and jump with @var{choice}. @samp{keep} > >> >> +keeps indirect call and jump unmodified. @samp{thunk} converts > >> >> indirect > >> >> +call and jump to call and return thunk. @samp{thunk-inline} converts > >> >> +indirect call and jump to inlined call and return thunk. > >> >> +@samp{thunk-extern} converts indirect call and jump to external call > >> >> +and return thunk provided in a separate object file. > >> > > >> > Please expand the documentation in a way that random user who is not > >> > aware of the > >> > issue will understand that those are security features that come at a > >> > cost. > >> > > >> >> +@opindex -mindirect-branch > >> >> +Convert indirect call and jump with @var{choice}. The default is > >> >> +@samp{keep}, which keeps indirect call and jump unmodified. > >> >> +@samp{thunk} converts indirect call and jump to call and return thunk. > >> >> +@samp{thunk-inline} converts indirect call and jump to inlined call > >> >> +and return thunk. @samp{thunk-extern} converts indirect call and jump > >> >> +to external call and return thunk provided in a separate object file. > >> >> +You can control this behavior for a specific function by using the > >> >> +function attribute @code{indirect_branch}. @xref{Function Attributes}. > >> > > >> > Similarly here. > >> > >> I will update documentation with user guide info after Intel white > >> paper is published. > > > > OK, > > thanks! > > Honza > >> > >> > Rest of the patch seems OK. We may want incrementally represent more of > >> > the > >> > indirect jump/call seqeuence in RTL, but at this point probably keeping > >> > things > >> > simple and localized is not a bad idea. This can be done incrementally. > >> > > >> > Please make updated patch and I would like to give others chance to > >> > comment today. > >> > > >> > Honza > >> > >> > >> > >> -- > >> H.J. > > > > -- > H.J.