* PATCH [2/n]: Prepare x32: Convert pointer to TLS symbol if needed @ 2011-06-11 15:56 H.J. Lu 2011-06-29 8:55 ` Richard Sandiford 0 siblings, 1 reply; 5+ messages in thread From: H.J. Lu @ 2011-06-11 15:56 UTC (permalink / raw) To: gcc-patches Hi, Backend may promote pointers to Pmode. Before we force a TLS symbol to a pseudo, we may need to convert it to proper mode. OK for trunk? Thanks. H.J. ---- 2011-06-11 H.J. Lu <hongjiu.lu@intel.com> * calls.c (precompute_register_parameters): Convert pointer to TLS symbol if needed. diff --git a/gcc/calls.c b/gcc/calls.c index feb98d2..de98267 100644 --- a/gcc/calls.c +++ b/gcc/calls.c @@ -706,7 +706,13 @@ precompute_register_parameters (int num_actuals, struct arg_data *args, pseudo now. TLS symbols sometimes need a call to resolve. */ if (CONSTANT_P (args[i].value) && !targetm.legitimate_constant_p (args[i].mode, args[i].value)) - args[i].value = force_reg (args[i].mode, args[i].value); + { + if (GET_MODE (args[i].value) != args[i].mode) + args[i].value = convert_to_mode (args[i].mode, + args[i].value, + args[i].unsignedp); + args[i].value = force_reg (args[i].mode, args[i].value); + } /* If we are to promote the function arg to a wider mode, do it now. */ ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: PATCH [2/n]: Prepare x32: Convert pointer to TLS symbol if needed 2011-06-11 15:56 PATCH [2/n]: Prepare x32: Convert pointer to TLS symbol if needed H.J. Lu @ 2011-06-29 8:55 ` Richard Sandiford 2011-06-29 14:39 ` H.J. Lu 0 siblings, 1 reply; 5+ messages in thread From: Richard Sandiford @ 2011-06-29 8:55 UTC (permalink / raw) To: H.J. Lu; +Cc: gcc-patches "H.J. Lu" <hongjiu.lu@intel.com> writes: > @@ -706,7 +706,13 @@ precompute_register_parameters (int num_actuals, struct arg_data *args, > pseudo now. TLS symbols sometimes need a call to resolve. */ > if (CONSTANT_P (args[i].value) > && !targetm.legitimate_constant_p (args[i].mode, args[i].value)) > - args[i].value = force_reg (args[i].mode, args[i].value); > + { > + if (GET_MODE (args[i].value) != args[i].mode) > + args[i].value = convert_to_mode (args[i].mode, > + args[i].value, > + args[i].unsignedp); > + args[i].value = force_reg (args[i].mode, args[i].value); > + } But if GET_MODE (args[i].value) != args[i].mode, then the call to targetm.legitimate_constant_p looks wrong. The mode passed in the first argument is supposed to the mode of the second argument. Is there any reason why this and the following: /* If we are to promote the function arg to a wider mode, do it now. */ if (args[i].mode != TYPE_MODE (TREE_TYPE (args[i].tree_value))) args[i].value = convert_modes (args[i].mode, TYPE_MODE (TREE_TYPE (args[i].tree_value)), args[i].value, args[i].unsignedp); need to be done in the current order? I can't think of any off-hand. If not, would swapping them also fix the bug? (I can't review this either way, of course.) Richard ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: PATCH [2/n]: Prepare x32: Convert pointer to TLS symbol if needed 2011-06-29 8:55 ` Richard Sandiford @ 2011-06-29 14:39 ` H.J. Lu 2011-06-29 17:36 ` H.J. Lu 0 siblings, 1 reply; 5+ messages in thread From: H.J. Lu @ 2011-06-29 14:39 UTC (permalink / raw) To: gcc-patches, richard.sandiford On Wed, Jun 29, 2011 at 1:45 AM, Richard Sandiford <richard.sandiford@linaro.org> wrote: > "H.J. Lu" <hongjiu.lu@intel.com> writes: >> @@ -706,7 +706,13 @@ precompute_register_parameters (int num_actuals, struct arg_data *args, >> pseudo now. TLS symbols sometimes need a call to resolve. */ >> if (CONSTANT_P (args[i].value) >> && !targetm.legitimate_constant_p (args[i].mode, args[i].value)) >> - args[i].value = force_reg (args[i].mode, args[i].value); >> + { >> + if (GET_MODE (args[i].value) != args[i].mode) >> + args[i].value = convert_to_mode (args[i].mode, >> + args[i].value, >> + args[i].unsignedp); >> + args[i].value = force_reg (args[i].mode, args[i].value); >> + } > > But if GET_MODE (args[i].value) != args[i].mode, then the call to > targetm.legitimate_constant_p looks wrong. The mode passed in the > first argument is supposed to the mode of the second argument. > > Is there any reason why this and the following: > > /* If we are to promote the function arg to a wider mode, > do it now. */ > > if (args[i].mode != TYPE_MODE (TREE_TYPE (args[i].tree_value))) > args[i].value > = convert_modes (args[i].mode, > TYPE_MODE (TREE_TYPE (args[i].tree_value)), > args[i].value, args[i].unsignedp); > > need to be done in the current order? I can't think of any off-hand. > If not, would swapping them also fix the bug? > > (I can't review this either way, of course.) It works on the testcase. I will do a full test. Thanks. -- H.J. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: PATCH [2/n]: Prepare x32: Convert pointer to TLS symbol if needed 2011-06-29 14:39 ` H.J. Lu @ 2011-06-29 17:36 ` H.J. Lu 2011-07-01 16:38 ` Richard Sandiford 0 siblings, 1 reply; 5+ messages in thread From: H.J. Lu @ 2011-06-29 17:36 UTC (permalink / raw) To: gcc-patches, richard.sandiford [-- Attachment #1: Type: text/plain, Size: 2136 bytes --] On Wed, Jun 29, 2011 at 7:06 AM, H.J. Lu <hjl.tools@gmail.com> wrote: > On Wed, Jun 29, 2011 at 1:45 AM, Richard Sandiford > <richard.sandiford@linaro.org> wrote: >> "H.J. Lu" <hongjiu.lu@intel.com> writes: >>> @@ -706,7 +706,13 @@ precompute_register_parameters (int num_actuals, struct arg_data *args, >>> pseudo now. TLS symbols sometimes need a call to resolve. */ >>> if (CONSTANT_P (args[i].value) >>> && !targetm.legitimate_constant_p (args[i].mode, args[i].value)) >>> - args[i].value = force_reg (args[i].mode, args[i].value); >>> + { >>> + if (GET_MODE (args[i].value) != args[i].mode) >>> + args[i].value = convert_to_mode (args[i].mode, >>> + args[i].value, >>> + args[i].unsignedp); >>> + args[i].value = force_reg (args[i].mode, args[i].value); >>> + } >> >> But if GET_MODE (args[i].value) != args[i].mode, then the call to >> targetm.legitimate_constant_p looks wrong. The mode passed in the >> first argument is supposed to the mode of the second argument. >> >> Is there any reason why this and the following: >> >> /* If we are to promote the function arg to a wider mode, >> do it now. */ >> >> if (args[i].mode != TYPE_MODE (TREE_TYPE (args[i].tree_value))) >> args[i].value >> = convert_modes (args[i].mode, >> TYPE_MODE (TREE_TYPE (args[i].tree_value)), >> args[i].value, args[i].unsignedp); >> >> need to be done in the current order? I can't think of any off-hand. >> If not, would swapping them also fix the bug? >> >> (I can't review this either way, of course.) > > It works on the testcase. I will do a full test. > It works. There are no regressions on Linux/x86-64. OK for trunk? Thanks. -- H.J. ---- 2011-06-29 H.J. Lu <hongjiu.lu@intel.com> PR middle-end/47715 * calls.c (precompute_register_parameters): Promote the function argument before checking non-legitimate constant. [-- Attachment #2: gcc-x32-pr47715-1.patch --] [-- Type: text/plain, Size: 1452 bytes --] 2011-06-29 H.J. Lu <hongjiu.lu@intel.com> PR middle-end/47715 * calls.c (precompute_register_parameters): Promote the function argument before checking non-legitimate constant. diff --git a/gcc/calls.c b/gcc/calls.c index bba477c..7538e4e 100644 --- a/gcc/calls.c +++ b/gcc/calls.c @@ -702,12 +702,6 @@ precompute_register_parameters (int num_actuals, struct arg_data *args, pop_temp_slots (); } - /* If the value is a non-legitimate constant, force it into a - pseudo now. TLS symbols sometimes need a call to resolve. */ - if (CONSTANT_P (args[i].value) - && !targetm.legitimate_constant_p (args[i].mode, args[i].value)) - args[i].value = force_reg (args[i].mode, args[i].value); - /* If we are to promote the function arg to a wider mode, do it now. */ @@ -717,6 +711,12 @@ precompute_register_parameters (int num_actuals, struct arg_data *args, TYPE_MODE (TREE_TYPE (args[i].tree_value)), args[i].value, args[i].unsignedp); + /* If the value is a non-legitimate constant, force it into a + pseudo now. TLS symbols sometimes need a call to resolve. */ + if (CONSTANT_P (args[i].value) + && !targetm.legitimate_constant_p (args[i].mode, args[i].value)) + args[i].value = force_reg (args[i].mode, args[i].value); + /* If we're going to have to load the value by parts, pull the parts into pseudos. The part extraction process can involve non-trivial computation. */ ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: PATCH [2/n]: Prepare x32: Convert pointer to TLS symbol if needed 2011-06-29 17:36 ` H.J. Lu @ 2011-07-01 16:38 ` Richard Sandiford 0 siblings, 0 replies; 5+ messages in thread From: Richard Sandiford @ 2011-07-01 16:38 UTC (permalink / raw) To: H.J. Lu; +Cc: gcc-patches "H.J. Lu" <hjl.tools@gmail.com> writes: > On Wed, Jun 29, 2011 at 7:06 AM, H.J. Lu <hjl.tools@gmail.com> wrote: >> On Wed, Jun 29, 2011 at 1:45 AM, Richard Sandiford >> <richard.sandiford@linaro.org> wrote: >>> "H.J. Lu" <hongjiu.lu@intel.com> writes: >>>> @@ -706,7 +706,13 @@ precompute_register_parameters (int num_actuals, struct arg_data *args, >>>> pseudo now. TLS symbols sometimes need a call to resolve. */ >>>> if (CONSTANT_P (args[i].value) >>>> && !targetm.legitimate_constant_p (args[i].mode, args[i].value)) >>>> - args[i].value = force_reg (args[i].mode, args[i].value); >>>> + { >>>> + if (GET_MODE (args[i].value) != args[i].mode) >>>> + args[i].value = convert_to_mode (args[i].mode, >>>> + args[i].value, >>>> + args[i].unsignedp); >>>> + args[i].value = force_reg (args[i].mode, args[i].value); >>>> + } >>> >>> But if GET_MODE (args[i].value) != args[i].mode, then the call to >>> targetm.legitimate_constant_p looks wrong. The mode passed in the >>> first argument is supposed to the mode of the second argument. >>> >>> Is there any reason why this and the following: >>> >>> /* If we are to promote the function arg to a wider mode, >>> do it now. */ >>> >>> if (args[i].mode != TYPE_MODE (TREE_TYPE (args[i].tree_value))) >>> args[i].value >>> = convert_modes (args[i].mode, >>> TYPE_MODE (TREE_TYPE (args[i].tree_value)), >>> args[i].value, args[i].unsignedp); >>> >>> need to be done in the current order? I can't think of any off-hand. >>> If not, would swapping them also fix the bug? >>> >>> (I can't review this either way, of course.) >> >> It works on the testcase. I will do a full test. >> > > It works. There are no regressions on Linux/x86-64. Great! I can't approve it, but FWIW, it looks good to me. The new order seems to make more conceptual sense: coerce the value into the right mode, then coerce it into the right type of rtx. Richard ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2011-07-01 16:38 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2011-06-11 15:56 PATCH [2/n]: Prepare x32: Convert pointer to TLS symbol if needed H.J. Lu 2011-06-29 8:55 ` Richard Sandiford 2011-06-29 14:39 ` H.J. Lu 2011-06-29 17:36 ` H.J. Lu 2011-07-01 16:38 ` Richard Sandiford
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).