This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [c++ patch] fix for mi-thunk emission


> The problem at hand is the insn_length array used in final.c by
> get_attr_length.  It is set up by shorten_branches to reflect the real
> length per insn.  If it is initialized get_attr_length() will use that
> array, otherwise it will call ultimately insn_default_length() (and
> attribute defined by insn-attrtab.c).
>
> So, what happens is the following:
>   mi-thunk is emitted
>     shorten_branch is called --> insn_length[] set up
>     final and friends are called
>   mi-thunk done
>   next normal function is compiled
>     ... different passes run ...
>     pass_delay_slots is run
>       dbr_schedule
>         requests certain attributes, some of them depend on attr_length
>         on sparc --> get_attr_length is called, returns wrong cached
>         length from the mi-thunk *
>     pass_shorten_branches is run
>       setup insn_length[] correctly for current function
>     ...
>     pass_clean_state is run
>       init_insn_length called, which clears insn_length[]
>
> At * this results in different code depending on if get_attr_length() is
> actually called for the other attributes (like eligible_for_delay in
> sparc's case), or if that call is optimized away by genattrtab (to the
> value insn_default_length would return).  They would agree on the value if
> insn_length[] wouldn't be incorrectly initialized.
>
> Note that this is just _different_, not wrong code.  Just the filling of
> delay slots is different, it's not incorrect.  Of course it could result
> also in wrong code under other circumstances.

Nice explanation.  Out of curiosity, how did you catch that?  By visual 
inspection?

> I fixed this by just adding a call to init_insn_length() at the right
> place in cp/method.c .

Thanks for taking care of the SPARC here.

> But conceptually how mi-thunks are emitted currently is a mess, and could
> possibly also result in other funny effects from not running clean_state
> afterwards. 

Seconded, it's really, really hacky.

-- 
Eric Botcazou


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]