| Message ID | 1557803319-91146-1-git-send-email-linkw@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2,1/3] Move prepare_decl_rtl to expr.[ch] as extern | expand |
On Tue, May 14, 2019 at 5:09 AM <linkw@linux.ibm.com> wrote: > > From: Kewen Lin <linkw@linux.ibm.com> > > Previous version link: > https://gcc.gnu.org/ml/gcc-patches/2019-04/msg00912.html > > This is a NFC (no functional change) patch. Ivopts has > some codes to expand gimple to RTL seq, but before call > expanding, we should call a preparation funciton > prepare_decl_rtl. This patch is to change it and its > dependents to non-static, can be shared with other passes. > > Bootstrapped and regression testing passed on powerpc64le. > > Is OK for trunk? Hum. The function is somewhat of a hack, trying to produce "reasonable" DECL_RTL, exposing it in expr.[ch] with this name is eventually misleading. Also you fail to "outline" the most important part: FOR_EACH_VEC_ELT (decl_rtl_to_reset, i, obj) SET_DECL_RTL (obj, NULL_RTX); which IMHO would warrant making this machinery a class with the above done in its destructor? Maybe name the functions prepare_guessed_decl_rtl () and the new class guessed_decl_rtl? Now looking how you'll end up using this... Richard. > gcc/ChangeLog > > 2019-05-13 Kewen Lin <linkw@gcc.gnu.org> > > PR middle-end/80791 > * expr.c (produce_memory_decl_rtl): New function. > (prepare_decl_rtl): Likewise. > * expr.h (produce_memory_decl_rtl): New declaration. > (prepare_decl_rtl): Likewise. > * tree-ssa-loop-ivopts.c (produce_memory_decl_rtl): Remove. > (prepare_decl_rtl): Likewise. > (computation_cost): Updated to call refactored prepare_decl_rtl. > > --- > gcc/expr.c | 91 +++++++++++++++++++++++++++++++++++++++++++++ > gcc/expr.h | 16 +++++++- > gcc/tree-ssa-loop-ivopts.c | 93 ++-------------------------------------------- > 3 files changed, 110 insertions(+), 90 deletions(-) > > diff --git a/gcc/expr.c b/gcc/expr.c > index 9ff5e5f..1f2ad45 100644 > --- a/gcc/expr.c > +++ b/gcc/expr.c > @@ -12539,3 +12539,94 @@ int_expr_size (tree exp) > > return tree_to_shwi (size); > } > + > +/* Produce DECL_RTL for object obj so it looks like it is stored in memory. */ > + > +rtx > +produce_memory_decl_rtl (tree obj, int *regno) > +{ > + addr_space_t as = TYPE_ADDR_SPACE (TREE_TYPE (obj)); > + machine_mode address_mode = targetm.addr_space.address_mode (as); > + rtx x; > + > + gcc_assert (obj); > + if (TREE_STATIC (obj) || DECL_EXTERNAL (obj)) > + { > + const char *name = IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (obj)); > + x = gen_rtx_SYMBOL_REF (address_mode, name); > + SET_SYMBOL_REF_DECL (x, obj); > + x = gen_rtx_MEM (DECL_MODE (obj), x); > + set_mem_addr_space (x, as); > + targetm.encode_section_info (obj, x, true); > + } > + else > + { > + x = gen_raw_REG (address_mode, (*regno)++); > + x = gen_rtx_MEM (DECL_MODE (obj), x); > + set_mem_addr_space (x, as); > + } > + > + return x; > +} > + > +/* Prepares decl_rtl for variables referred in *EXPR_P. Callback for > + walk_tree. DATA contains the actual fake register number. */ > + > +tree > +prepare_decl_rtl (tree *expr_p, int *ws, void *data) > +{ > + tree obj = NULL_TREE; > + rtx x = NULL_RTX; > + decl_rtl_data *info = (decl_rtl_data *) data; > + int *regno = info->regno; > + vec<tree> *treevec = info->treevec; > + > + switch (TREE_CODE (*expr_p)) > + { > + case ADDR_EXPR: > + for (expr_p = &TREE_OPERAND (*expr_p, 0); handled_component_p (*expr_p); > + expr_p = &TREE_OPERAND (*expr_p, 0)) > + continue; > + obj = *expr_p; > + if (DECL_P (obj) && HAS_RTL_P (obj) && !DECL_RTL_SET_P (obj)) > + x = produce_memory_decl_rtl (obj, regno); > + break; > + > + case SSA_NAME: > + *ws = 0; > + obj = SSA_NAME_VAR (*expr_p); > + /* Defer handling of anonymous SSA_NAMEs to the expander. */ > + if (!obj) > + return NULL_TREE; > + if (!DECL_RTL_SET_P (obj)) > + x = gen_raw_REG (DECL_MODE (obj), (*regno)++); > + break; > + > + case VAR_DECL: > + case PARM_DECL: > + case RESULT_DECL: > + *ws = 0; > + obj = *expr_p; > + > + if (DECL_RTL_SET_P (obj)) > + break; > + > + if (DECL_MODE (obj) == BLKmode) > + x = produce_memory_decl_rtl (obj, regno); > + else > + x = gen_raw_REG (DECL_MODE (obj), (*regno)++); > + > + break; > + > + default: > + break; > + } > + > + if (x) > + { > + treevec->safe_push (obj); > + SET_DECL_RTL (obj, x); > + } > + > + return NULL_TREE; > +} > diff --git a/gcc/expr.h b/gcc/expr.h > index 17c3962..b1894a6b 100644 > --- a/gcc/expr.h > +++ b/gcc/expr.h > @@ -53,7 +53,21 @@ typedef struct separate_ops > tree type; > tree op0, op1, op2; > } *sepops; > - > + > +/* This structure is used to pass information to tree walker function > + prepare_decl_rtl. */ > +typedef struct data_for_decl_rtl > +{ > + int *regno; > + vec<tree> *treevec; > +} decl_rtl_data; > + > +/* Produce decl_rtl for object so it looks like it is stored in memory. */ > +rtx produce_memory_decl_rtl (tree, int *); > + > +/* Prepares decl_rtl for variables referred. Callback for walk_tree. */ > +tree prepare_decl_rtl (tree *, int *, void *); > + > /* This is run during target initialization to set up which modes can be > used directly in memory and to initialize the block move optab. */ > extern void init_expr_target (void); > diff --git a/gcc/tree-ssa-loop-ivopts.c b/gcc/tree-ssa-loop-ivopts.c > index a44b4cb..885c8e8 100644 > --- a/gcc/tree-ssa-loop-ivopts.c > +++ b/gcc/tree-ssa-loop-ivopts.c > @@ -3687,94 +3687,6 @@ get_group_iv_cost (struct ivopts_data *data, struct iv_group *group, > return NULL; > } > > -/* Produce DECL_RTL for object obj so it looks like it is stored in memory. */ > -static rtx > -produce_memory_decl_rtl (tree obj, int *regno) > -{ > - addr_space_t as = TYPE_ADDR_SPACE (TREE_TYPE (obj)); > - machine_mode address_mode = targetm.addr_space.address_mode (as); > - rtx x; > - > - gcc_assert (obj); > - if (TREE_STATIC (obj) || DECL_EXTERNAL (obj)) > - { > - const char *name = IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (obj)); > - x = gen_rtx_SYMBOL_REF (address_mode, name); > - SET_SYMBOL_REF_DECL (x, obj); > - x = gen_rtx_MEM (DECL_MODE (obj), x); > - set_mem_addr_space (x, as); > - targetm.encode_section_info (obj, x, true); > - } > - else > - { > - x = gen_raw_REG (address_mode, (*regno)++); > - x = gen_rtx_MEM (DECL_MODE (obj), x); > - set_mem_addr_space (x, as); > - } > - > - return x; > -} > - > -/* Prepares decl_rtl for variables referred in *EXPR_P. Callback for > - walk_tree. DATA contains the actual fake register number. */ > - > -static tree > -prepare_decl_rtl (tree *expr_p, int *ws, void *data) > -{ > - tree obj = NULL_TREE; > - rtx x = NULL_RTX; > - int *regno = (int *) data; > - > - switch (TREE_CODE (*expr_p)) > - { > - case ADDR_EXPR: > - for (expr_p = &TREE_OPERAND (*expr_p, 0); > - handled_component_p (*expr_p); > - expr_p = &TREE_OPERAND (*expr_p, 0)) > - continue; > - obj = *expr_p; > - if (DECL_P (obj) && HAS_RTL_P (obj) && !DECL_RTL_SET_P (obj)) > - x = produce_memory_decl_rtl (obj, regno); > - break; > - > - case SSA_NAME: > - *ws = 0; > - obj = SSA_NAME_VAR (*expr_p); > - /* Defer handling of anonymous SSA_NAMEs to the expander. */ > - if (!obj) > - return NULL_TREE; > - if (!DECL_RTL_SET_P (obj)) > - x = gen_raw_REG (DECL_MODE (obj), (*regno)++); > - break; > - > - case VAR_DECL: > - case PARM_DECL: > - case RESULT_DECL: > - *ws = 0; > - obj = *expr_p; > - > - if (DECL_RTL_SET_P (obj)) > - break; > - > - if (DECL_MODE (obj) == BLKmode) > - x = produce_memory_decl_rtl (obj, regno); > - else > - x = gen_raw_REG (DECL_MODE (obj), (*regno)++); > - > - break; > - > - default: > - break; > - } > - > - if (x) > - { > - decl_rtl_to_reset.safe_push (obj); > - SET_DECL_RTL (obj, x); > - } > - > - return NULL_TREE; > -} > > /* Determines cost of the computation of EXPR. */ > > @@ -3792,7 +3704,10 @@ computation_cost (tree expr, bool speed) > > node->frequency = NODE_FREQUENCY_NORMAL; > crtl->maybe_hot_insn_p = speed; > - walk_tree (&expr, prepare_decl_rtl, ®no, NULL); > + decl_rtl_data data; > + data.regno = ®no; > + data.treevec = &decl_rtl_to_reset; > + walk_tree (&expr, prepare_decl_rtl, &data, NULL); > start_sequence (); > rslt = expand_expr (expr, NULL_RTX, TYPE_MODE (type), EXPAND_NORMAL); > seq = get_insns (); > -- > 2.7.4 >
on 2019/5/14 下午3:18, Richard Biener wrote: > Hum. The function is somewhat of a hack, trying to produce > "reasonable" DECL_RTL, exposing it in expr.[ch] with this > name is eventually misleading. Also you fail to "outline" > the most important part: > > FOR_EACH_VEC_ELT (decl_rtl_to_reset, i, obj) > SET_DECL_RTL (obj, NULL_RTX); > > which IMHO would warrant making this machinery a class > with the above done in its destructor? > Good suggestion! In the IVOPTS implementation, it has one interface free_loop_data to clean up some data structures including this decl_rtl_to_reset. While for the use in "PATCH v2 2/3", we have to clean it artificially once expanding finishes. It's better to make it as a class and ensure the clean of the vector in its destructor. > Maybe name the functions prepare_guessed_decl_rtl () > and the new class guessed_decl_rtl? > OK. Or "tmp" instead of "guessed"? Thanks, Kewen > Now looking how you'll end up using this... > > Richard. >
on 2019/5/15 上午10:11, Kewen.Lin wrote: > > on 2019/5/14 下午3:18, Richard Biener wrote: >> Hum. The function is somewhat of a hack, trying to produce >> "reasonable" DECL_RTL, exposing it in expr.[ch] with this >> name is eventually misleading. Also you fail to "outline" >> the most important part: >> >> FOR_EACH_VEC_ELT (decl_rtl_to_reset, i, obj) >> SET_DECL_RTL (obj, NULL_RTX); >> >> which IMHO would warrant making this machinery a class >> with the above done in its destructor? >> > > Good suggestion! In the IVOPTS implementation, it has one > interface free_loop_data to clean up some data structures > including this decl_rtl_to_reset. While for the use in > "PATCH v2 2/3", we have to clean it artificially once > expanding finishes. > > It's better to make it as a class and ensure the clean of > the vector in its destructor. > Hi Bin, Could you kindly comment this? For the use in "PATCH v2 2/3", it's still the same with destructor change. But for the use in IVOPTs, I don't know the historical reason why to clean up in free_loop_data instead of in place? If it's possible to reuse, for example some object was prepared and we don't need to prepare for it again during the same loop handling, then the destructor proposal will clean it up too early and inefficient. If so, one flag in constructor to control this might be required. Thanks a lot in advance! Kewen >> Maybe name the functions prepare_guessed_decl_rtl () >> and the new class guessed_decl_rtl? >> > OK. Or "tmp" instead of "guessed"? > > > Thanks, > Kewen > >> Now looking how you'll end up using this... >> >> Richard. >> >
On Wed, 15 May 2019, Kewen.Lin wrote: > > on 2019/5/14 下午3:18, Richard Biener wrote: > > Hum. The function is somewhat of a hack, trying to produce > > "reasonable" DECL_RTL, exposing it in expr.[ch] with this > > name is eventually misleading. Also you fail to "outline" > > the most important part: > > > > FOR_EACH_VEC_ELT (decl_rtl_to_reset, i, obj) > > SET_DECL_RTL (obj, NULL_RTX); > > > > which IMHO would warrant making this machinery a class > > with the above done in its destructor? > > > > Good suggestion! In the IVOPTS implementation, it has one > interface free_loop_data to clean up some data structures > including this decl_rtl_to_reset. While for the use in > "PATCH v2 2/3", we have to clean it artificially once > expanding finishes. > > It's better to make it as a class and ensure the clean of > the vector in its destructor. > > > Maybe name the functions prepare_guessed_decl_rtl () > > and the new class guessed_decl_rtl? > > > OK. Or "tmp" instead of "guessed"? Also works for me. Richard.
On Wed, May 15, 2019 at 4:38 PM Kewen.Lin <linkw@linux.ibm.com> wrote: > > on 2019/5/15 上午10:11, Kewen.Lin wrote: > > > > on 2019/5/14 下午3:18, Richard Biener wrote: > >> Hum. The function is somewhat of a hack, trying to produce > >> "reasonable" DECL_RTL, exposing it in expr.[ch] with this > >> name is eventually misleading. Also you fail to "outline" > >> the most important part: > >> > >> FOR_EACH_VEC_ELT (decl_rtl_to_reset, i, obj) > >> SET_DECL_RTL (obj, NULL_RTX); > >> > >> which IMHO would warrant making this machinery a class > >> with the above done in its destructor? > >> > > > > Good suggestion! In the IVOPTS implementation, it has one > > interface free_loop_data to clean up some data structures > > including this decl_rtl_to_reset. While for the use in > > "PATCH v2 2/3", we have to clean it artificially once > > expanding finishes. > > > > It's better to make it as a class and ensure the clean of > > the vector in its destructor. > > > > Hi Bin, > > Could you kindly comment this? For the use in "PATCH v2 2/3", > it's still the same with destructor change. But for the use > in IVOPTs, I don't know the historical reason why to clean > up in free_loop_data instead of in place? If it's possible to This is quite old code in ivopts, I suppose it's for caching results as you guessed. I didn't measure if how much the caching affects performance. It can be simply removed if not as useful as expected. Of course we need experiment data here. Please correct me if I am wrong. This is factored out of ivopts in order to reuse it in the new target hook? As discussed in another thread, most target hook code can be moved to ivopts, so this refactoring and in-place freeing would be unnecessary, and the new code can also take advantage of caching? Thanks, bin > reuse, for example some object was prepared and we don't need > to prepare for it again during the same loop handling, then > the destructor proposal will clean it up too early and > inefficient. If so, one flag in constructor to control this > might be required. > > Thanks a lot in advance! > > Kewen > > > >> Maybe name the functions prepare_guessed_decl_rtl () > >> and the new class guessed_decl_rtl? > >> > > OK. Or "tmp" instead of "guessed"? > > > > > > Thanks, > > Kewen > > > >> Now looking how you'll end up using this... > >> > >> Richard. > >> > > >
On Wed, 15 May 2019, Bin.Cheng wrote: > On Wed, May 15, 2019 at 4:38 PM Kewen.Lin <linkw@linux.ibm.com> wrote: > > > > on 2019/5/15 上午10:11, Kewen.Lin wrote: > > > > > > on 2019/5/14 下午3:18, Richard Biener wrote: > > >> Hum. The function is somewhat of a hack, trying to produce > > >> "reasonable" DECL_RTL, exposing it in expr.[ch] with this > > >> name is eventually misleading. Also you fail to "outline" > > >> the most important part: > > >> > > >> FOR_EACH_VEC_ELT (decl_rtl_to_reset, i, obj) > > >> SET_DECL_RTL (obj, NULL_RTX); > > >> > > >> which IMHO would warrant making this machinery a class > > >> with the above done in its destructor? > > >> > > > > > > Good suggestion! In the IVOPTS implementation, it has one > > > interface free_loop_data to clean up some data structures > > > including this decl_rtl_to_reset. While for the use in > > > "PATCH v2 2/3", we have to clean it artificially once > > > expanding finishes. > > > > > > It's better to make it as a class and ensure the clean of > > > the vector in its destructor. > > > > > > > Hi Bin, > > > > Could you kindly comment this? For the use in "PATCH v2 2/3", > > it's still the same with destructor change. But for the use > > in IVOPTs, I don't know the historical reason why to clean > > up in free_loop_data instead of in place? If it's possible to > This is quite old code in ivopts, I suppose it's for caching results > as you guessed. I didn't measure if how much the caching affects > performance. It can be simply removed if not as useful as expected. > Of course we need experiment data here. > > Please correct me if I am wrong. This is factored out of ivopts in > order to reuse it in the new target hook? As discussed in another > thread, most target hook code can be moved to ivopts, so this > refactoring and in-place freeing would be unnecessary, and the new > code can also take advantage of caching? You can use a pointer to the class in place of the global vector used right now and allocate/deallocate with new/delete in the exactly same places than before? Cleaning things up can be done independently and that would preserve current behavior. Richard. > Thanks, > bin > > reuse, for example some object was prepared and we don't need > > to prepare for it again during the same loop handling, then > > the destructor proposal will clean it up too early and > > inefficient. If so, one flag in constructor to control this > > might be required. > > > > Thanks a lot in advance! > > > > Kewen > > > > > > >> Maybe name the functions prepare_guessed_decl_rtl () > > >> and the new class guessed_decl_rtl? > > >> > > > OK. Or "tmp" instead of "guessed"? > > > > > > > > > Thanks, > > > Kewen > > > > > >> Now looking how you'll end up using this... > > >> > > >> Richard. > > >> > > > > > >
on 2019/5/15 下午4:50, Bin.Cheng wrote: > On Wed, May 15, 2019 at 4:38 PM Kewen.Lin <linkw@linux.ibm.com> wrote: >> >> on 2019/5/15 上午10:11, Kewen.Lin wrote: >>> >>> on 2019/5/14 下午3:18, Richard Biener wrote: >>>> Hum. The function is somewhat of a hack, trying to produce >>>> "reasonable" DECL_RTL, exposing it in expr.[ch] with this >>>> name is eventually misleading. Also you fail to "outline" >>>> the most important part: >>>> >>>> FOR_EACH_VEC_ELT (decl_rtl_to_reset, i, obj) >>>> SET_DECL_RTL (obj, NULL_RTX); >>>> >>>> which IMHO would warrant making this machinery a class >>>> with the above done in its destructor? >>>> >>> >>> Good suggestion! In the IVOPTS implementation, it has one >>> interface free_loop_data to clean up some data structures >>> including this decl_rtl_to_reset. While for the use in >>> "PATCH v2 2/3", we have to clean it artificially once >>> expanding finishes. >>> >>> It's better to make it as a class and ensure the clean of >>> the vector in its destructor. >>> >> >> Hi Bin, >> >> Could you kindly comment this? For the use in "PATCH v2 2/3", >> it's still the same with destructor change. But for the use >> in IVOPTs, I don't know the historical reason why to clean >> up in free_loop_data instead of in place? If it's possible to > This is quite old code in ivopts, I suppose it's for caching results > as you guessed. I didn't measure if how much the caching affects > performance. It can be simply removed if not as useful as expected. > Of course we need experiment data here. Got it, thanks! > > Please correct me if I am wrong. This is factored out of ivopts in > order to reuse it in the new target hook? As discussed in another > thread, most target hook code can be moved to ivopts, so this > refactoring and in-place freeing would be unnecessary, and the new > code can also take advantage of caching? > Yes, you are absolutely right. If we move the generic part ( especially costly_iter_for_doloop_p) to IVOPTs, the factoring isn't necessary any more. I thought it may be still nice to refactor it even if we will go with that moving of generic part. But the code exists for a long time but there is no requests to factor it out all the time, it looks useless to expose it speculatively? OK, I'll cancel factoring it then. Thanks, Kewen > Thanks, > bin >> reuse, for example some object was prepared and we don't need >> to prepare for it again during the same loop handling, then >> the destructor proposal will clean it up too early and >> inefficient. If so, one flag in constructor to control this >> might be required. >> >> Thanks a lot in advance! >> >> Kewen >> >>
On Wed, May 15, 2019 at 5:51 PM Kewen.Lin <linkw@linux.ibm.com> wrote: > > on 2019/5/15 下午4:50, Bin.Cheng wrote: > > On Wed, May 15, 2019 at 4:38 PM Kewen.Lin <linkw@linux.ibm.com> wrote: > >> > >> on 2019/5/15 上午10:11, Kewen.Lin wrote: > >>> > >>> on 2019/5/14 下午3:18, Richard Biener wrote: > >>>> Hum. The function is somewhat of a hack, trying to produce > >>>> "reasonable" DECL_RTL, exposing it in expr.[ch] with this > >>>> name is eventually misleading. Also you fail to "outline" > >>>> the most important part: > >>>> > >>>> FOR_EACH_VEC_ELT (decl_rtl_to_reset, i, obj) > >>>> SET_DECL_RTL (obj, NULL_RTX); > >>>> > >>>> which IMHO would warrant making this machinery a class > >>>> with the above done in its destructor? > >>>> > >>> > >>> Good suggestion! In the IVOPTS implementation, it has one > >>> interface free_loop_data to clean up some data structures > >>> including this decl_rtl_to_reset. While for the use in > >>> "PATCH v2 2/3", we have to clean it artificially once > >>> expanding finishes. > >>> > >>> It's better to make it as a class and ensure the clean of > >>> the vector in its destructor. > >>> > >> > >> Hi Bin, > >> > >> Could you kindly comment this? For the use in "PATCH v2 2/3", > >> it's still the same with destructor change. But for the use > >> in IVOPTs, I don't know the historical reason why to clean > >> up in free_loop_data instead of in place? If it's possible to > > This is quite old code in ivopts, I suppose it's for caching results > > as you guessed. I didn't measure if how much the caching affects > > performance. It can be simply removed if not as useful as expected. > > Of course we need experiment data here. > > Got it, thanks! > > > > > Please correct me if I am wrong. This is factored out of ivopts in > > order to reuse it in the new target hook? As discussed in another > > thread, most target hook code can be moved to ivopts, so this > > refactoring and in-place freeing would be unnecessary, and the new > > code can also take advantage of caching? > > > > Yes, you are absolutely right. If we move the generic part ( > especially costly_iter_for_doloop_p) to IVOPTs, the factoring isn't > necessary any more. > > I thought it may be still nice to refactor it even if we will go > with that moving of generic part. But the code exists for a long > time but there is no requests to factor it out all the time, it > looks useless to expose it speculatively? Just do it if you think necessary. The criteria is simple: whether it results in better code. You don't need any approval to improve GCC. :-) Thanks, bin > > OK, I'll cancel factoring it then. > > > Thanks, > Kewen > > > Thanks, > > bin > > > >> reuse, for example some object was prepared and we don't need > >> to prepare for it again during the same loop handling, then > >> the destructor proposal will clean it up too early and > >> inefficient. If so, one flag in constructor to control this > >> might be required. > >> > >> Thanks a lot in advance! > >> > >> Kewen > >> > >> >
diff --git a/gcc/expr.c b/gcc/expr.c index 9ff5e5f..1f2ad45 100644 --- a/gcc/expr.c +++ b/gcc/expr.c @@ -12539,3 +12539,94 @@ int_expr_size (tree exp) return tree_to_shwi (size); } + +/* Produce DECL_RTL for object obj so it looks like it is stored in memory. */ + +rtx +produce_memory_decl_rtl (tree obj, int *regno) +{ + addr_space_t as = TYPE_ADDR_SPACE (TREE_TYPE (obj)); + machine_mode address_mode = targetm.addr_space.address_mode (as); + rtx x; + + gcc_assert (obj); + if (TREE_STATIC (obj) || DECL_EXTERNAL (obj)) + { + const char *name = IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (obj)); + x = gen_rtx_SYMBOL_REF (address_mode, name); + SET_SYMBOL_REF_DECL (x, obj); + x = gen_rtx_MEM (DECL_MODE (obj), x); + set_mem_addr_space (x, as); + targetm.encode_section_info (obj, x, true); + } + else + { + x = gen_raw_REG (address_mode, (*regno)++); + x = gen_rtx_MEM (DECL_MODE (obj), x); + set_mem_addr_space (x, as); + } + + return x; +} + +/* Prepares decl_rtl for variables referred in *EXPR_P. Callback for + walk_tree. DATA contains the actual fake register number. */ + +tree +prepare_decl_rtl (tree *expr_p, int *ws, void *data) +{ + tree obj = NULL_TREE; + rtx x = NULL_RTX; + decl_rtl_data *info = (decl_rtl_data *) data; + int *regno = info->regno; + vec<tree> *treevec = info->treevec; + + switch (TREE_CODE (*expr_p)) + { + case ADDR_EXPR: + for (expr_p = &TREE_OPERAND (*expr_p, 0); handled_component_p (*expr_p); + expr_p = &TREE_OPERAND (*expr_p, 0)) + continue; + obj = *expr_p; + if (DECL_P (obj) && HAS_RTL_P (obj) && !DECL_RTL_SET_P (obj)) + x = produce_memory_decl_rtl (obj, regno); + break; + + case SSA_NAME: + *ws = 0; + obj = SSA_NAME_VAR (*expr_p); + /* Defer handling of anonymous SSA_NAMEs to the expander. */ + if (!obj) + return NULL_TREE; + if (!DECL_RTL_SET_P (obj)) + x = gen_raw_REG (DECL_MODE (obj), (*regno)++); + break; + + case VAR_DECL: + case PARM_DECL: + case RESULT_DECL: + *ws = 0; + obj = *expr_p; + + if (DECL_RTL_SET_P (obj)) + break; + + if (DECL_MODE (obj) == BLKmode) + x = produce_memory_decl_rtl (obj, regno); + else + x = gen_raw_REG (DECL_MODE (obj), (*regno)++); + + break; + + default: + break; + } + + if (x) + { + treevec->safe_push (obj); + SET_DECL_RTL (obj, x); + } + + return NULL_TREE; +} diff --git a/gcc/expr.h b/gcc/expr.h index 17c3962..b1894a6b 100644 --- a/gcc/expr.h +++ b/gcc/expr.h @@ -53,7 +53,21 @@ typedef struct separate_ops tree type; tree op0, op1, op2; } *sepops; - + +/* This structure is used to pass information to tree walker function + prepare_decl_rtl. */ +typedef struct data_for_decl_rtl +{ + int *regno; + vec<tree> *treevec; +} decl_rtl_data; + +/* Produce decl_rtl for object so it looks like it is stored in memory. */ +rtx produce_memory_decl_rtl (tree, int *); + +/* Prepares decl_rtl for variables referred. Callback for walk_tree. */ +tree prepare_decl_rtl (tree *, int *, void *); + /* This is run during target initialization to set up which modes can be used directly in memory and to initialize the block move optab. */ extern void init_expr_target (void); diff --git a/gcc/tree-ssa-loop-ivopts.c b/gcc/tree-ssa-loop-ivopts.c index a44b4cb..885c8e8 100644 --- a/gcc/tree-ssa-loop-ivopts.c +++ b/gcc/tree-ssa-loop-ivopts.c @@ -3687,94 +3687,6 @@ get_group_iv_cost (struct ivopts_data *data, struct iv_group *group, return NULL; } -/* Produce DECL_RTL for object obj so it looks like it is stored in memory. */ -static rtx -produce_memory_decl_rtl (tree obj, int *regno) -{ - addr_space_t as = TYPE_ADDR_SPACE (TREE_TYPE (obj)); - machine_mode address_mode = targetm.addr_space.address_mode (as); - rtx x; - - gcc_assert (obj); - if (TREE_STATIC (obj) || DECL_EXTERNAL (obj)) - { - const char *name = IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (obj)); - x = gen_rtx_SYMBOL_REF (address_mode, name); - SET_SYMBOL_REF_DECL (x, obj); - x = gen_rtx_MEM (DECL_MODE (obj), x); - set_mem_addr_space (x, as); - targetm.encode_section_info (obj, x, true); - } - else - { - x = gen_raw_REG (address_mode, (*regno)++); - x = gen_rtx_MEM (DECL_MODE (obj), x); - set_mem_addr_space (x, as); - } - - return x; -} - -/* Prepares decl_rtl for variables referred in *EXPR_P. Callback for - walk_tree. DATA contains the actual fake register number. */ - -static tree -prepare_decl_rtl (tree *expr_p, int *ws, void *data) -{ - tree obj = NULL_TREE; - rtx x = NULL_RTX; - int *regno = (int *) data; - - switch (TREE_CODE (*expr_p)) - { - case ADDR_EXPR: - for (expr_p = &TREE_OPERAND (*expr_p, 0); - handled_component_p (*expr_p); - expr_p = &TREE_OPERAND (*expr_p, 0)) - continue; - obj = *expr_p; - if (DECL_P (obj) && HAS_RTL_P (obj) && !DECL_RTL_SET_P (obj)) - x = produce_memory_decl_rtl (obj, regno); - break; - - case SSA_NAME: - *ws = 0; - obj = SSA_NAME_VAR (*expr_p); - /* Defer handling of anonymous SSA_NAMEs to the expander. */ - if (!obj) - return NULL_TREE; - if (!DECL_RTL_SET_P (obj)) - x = gen_raw_REG (DECL_MODE (obj), (*regno)++); - break; - - case VAR_DECL: - case PARM_DECL: - case RESULT_DECL: - *ws = 0; - obj = *expr_p; - - if (DECL_RTL_SET_P (obj)) - break; - - if (DECL_MODE (obj) == BLKmode) - x = produce_memory_decl_rtl (obj, regno); - else - x = gen_raw_REG (DECL_MODE (obj), (*regno)++); - - break; - - default: - break; - } - - if (x) - { - decl_rtl_to_reset.safe_push (obj); - SET_DECL_RTL (obj, x); - } - - return NULL_TREE; -} /* Determines cost of the computation of EXPR. */ @@ -3792,7 +3704,10 @@ computation_cost (tree expr, bool speed) node->frequency = NODE_FREQUENCY_NORMAL; crtl->maybe_hot_insn_p = speed; - walk_tree (&expr, prepare_decl_rtl, ®no, NULL); + decl_rtl_data data; + data.regno = ®no; + data.treevec = &decl_rtl_to_reset; + walk_tree (&expr, prepare_decl_rtl, &data, NULL); start_sequence (); rslt = expand_expr (expr, NULL_RTX, TYPE_MODE (type), EXPAND_NORMAL); seq = get_insns ();
From: Kewen Lin <linkw@linux.ibm.com> Previous version link: https://gcc.gnu.org/ml/gcc-patches/2019-04/msg00912.html This is a NFC (no functional change) patch. Ivopts has some codes to expand gimple to RTL seq, but before call expanding, we should call a preparation funciton prepare_decl_rtl. This patch is to change it and its dependents to non-static, can be shared with other passes. Bootstrapped and regression testing passed on powerpc64le. Is OK for trunk? gcc/ChangeLog 2019-05-13 Kewen Lin <linkw@gcc.gnu.org> PR middle-end/80791 * expr.c (produce_memory_decl_rtl): New function. (prepare_decl_rtl): Likewise. * expr.h (produce_memory_decl_rtl): New declaration. (prepare_decl_rtl): Likewise. * tree-ssa-loop-ivopts.c (produce_memory_decl_rtl): Remove. (prepare_decl_rtl): Likewise. (computation_cost): Updated to call refactored prepare_decl_rtl. --- gcc/expr.c | 91 +++++++++++++++++++++++++++++++++++++++++++++ gcc/expr.h | 16 +++++++- gcc/tree-ssa-loop-ivopts.c | 93 ++-------------------------------------------- 3 files changed, 110 insertions(+), 90 deletions(-)