Re: [x32] PATCH: PR middle-end/47725: [x32] error: unable to find a register to spill in class DIREG
On Thu, Mar 24, 2011 at 8:51 AM, Eric Botcazou wrote:
>> Pointer is promoted to Pmode from ptr_mode.
>
> Indeed. However the problem is the 2 in assign_parm_setup_reg:
>
> /* Store the parm in a pseudoregister during the function, but we may
> need to do it in a wider mode. Using 2 here makes the result
> consistent with promote_decl_mode and thus expand_expr_real_1. */
> promoted_nominal_mode
> = promote_function_mode (data->nominal_type, data->nominal_mode, &unsignedp,
> TREE_TYPE (current_function_decl), 2);
>
> which is supposed to match the 2 in promote_decl_mode:
>
> if (TREE_CODE (decl) == RESULT_DECL
> || TREE_CODE (decl) == PARM_DECL)
> pmode = promote_function_mode (type, mode, &unsignedp,
> TREE_TYPE (current_function_decl), 2);
> else
> pmode = promote_mode (type, mode, &unsignedp);
>
> but doesn't match the 0 in assign_parm_find_data_types:
>
> promoted_mode = promote_function_mode (passed_type, passed_mode, &unsignedp,
> TREE_TYPE (current_function_decl), 0);
>
> so you get the redundant extension in the callee. The solution is to define
> the promote_function_mode hook for x86 to something like:
>
> static enum machine_mode
> ix86_promote_function_mode (const_tree type,
> enum machine_mode mode,
> int *punsignedp,
> const_tree fntype ATTRIBUTE_UNUSED,
> int for_return ATTRIBUTE_UNUSED)
> {
> if (POINTER_TYPE_P (type))
> {
> *punsignedp = POINTERS_EXTEND_UNSIGNED;
> return Pmode;
> }
>
> return mode;
> }
>
Here is the patch. precompute_register_parameters change is needed for
---
extern __thread int ttt;
extern void bar (void *);
void
foo1 ()
{
bar (&ttt);
}
---
I used
/* Pointer function arguments and return values are promoted to
Pmode. */
static enum machine_mode
ix86_promote_function_mode (const_tree type, enum machine_mode mode,
int *punsignedp, const_tree fntype,
int for_return)
{
if (for_return != 1 && POINTER_TYPE_P (type))
{
*punsignedp = POINTERS_EXTEND_UNSIGNED;
return Pmode;
}
return default_promote_function_mode (type, mode, punsignedp, fntype,
for_return);
}
since for_return == 1 has to match function_value, which I want to keep
as is. default_promote_function_mode handles i386 PROMOTE_MODE.
Tested it on Linux/ia32 and Linux/x86-64. OK for trunk?
Thanks.
--
H.J.
--
2011-03-29 H.J. Lu
PR middle-end/47725
PR target/48085
* calls.c (precompute_register_parameters): Convert pointer to
TLS symbol if needed.
* config/i386/i386.c (ix86_promote_function_mode): New.
(TARGET_PROMOTE_FUNCTION_MODE): Likewise.
diff --git a/gcc/ChangeLog.x32 b/gcc/ChangeLog.x32
index 303acd8..a7ff7e3 100644
--- a/gcc/ChangeLog.x32
+++ b/gcc/ChangeLog.x32
@@ -1,6 +1,16 @@
2011-03-29 H.J. Lu
PR middle-end/47725
+ PR target/48085
+ * calls.c (precompute_register_parameters): Convert pointer to
+ TLS symbol if needed.
+
+ * config/i386/i386.c (ix86_promote_function_mode): New.
+ (TARGET_PROMOTE_FUNCTION_MODE): Likewise.
+
+2011-03-29 H.J. Lu
+
+ PR middle-end/47725
* combine.c (cant_combine_insn_p): Don't check zero/sign extended
hard registers.
diff --git a/gcc/calls.c b/gcc/calls.c
index 2e79777..7357241 100644
--- a/gcc/calls.c
+++ b/gcc/calls.c
@@ -691,7 +691,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)
&& !LEGITIMATE_CONSTANT_P (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. */
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index d7a5c02..6cffab2 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -7636,6 +7636,23 @@ ix86_function_value (const_tree valtype, const_tree
fntype_or_decl,
return ix86_function_value_1 (valtype, fntype_or_decl, orig_mode, mode);
}
+/* Pointer function arguments and return values are promoted to
+ Pmode. */
+
+static enum machine_mode
+ix86_promote_function_mode (const_tree type, enum machine_mode mode,
+ int *punsignedp, const_tree fntype,
+ int for_return)
+{
+ i
Re: [x32] PATCH: PR middle-end/47725: [x32] error: unable to find a register to spill in class DIREG
> Pointer is promoted to Pmode from ptr_mode.
Indeed. However the problem is the 2 in assign_parm_setup_reg:
/* Store the parm in a pseudoregister during the function, but we may
need to do it in a wider mode. Using 2 here makes the result
consistent with promote_decl_mode and thus expand_expr_real_1. */
promoted_nominal_mode
= promote_function_mode (data->nominal_type, data->nominal_mode, &unsignedp,
TREE_TYPE (current_function_decl), 2);
which is supposed to match the 2 in promote_decl_mode:
if (TREE_CODE (decl) == RESULT_DECL
|| TREE_CODE (decl) == PARM_DECL)
pmode = promote_function_mode (type, mode, &unsignedp,
TREE_TYPE (current_function_decl), 2);
else
pmode = promote_mode (type, mode, &unsignedp);
but doesn't match the 0 in assign_parm_find_data_types:
promoted_mode = promote_function_mode (passed_type, passed_mode, &unsignedp,
TREE_TYPE (current_function_decl), 0);
so you get the redundant extension in the callee. The solution is to define
the promote_function_mode hook for x86 to something like:
static enum machine_mode
ix86_promote_function_mode (const_tree type,
enum machine_mode mode,
int *punsignedp,
const_tree fntype ATTRIBUTE_UNUSED,
int for_return ATTRIBUTE_UNUSED)
{
if (POINTER_TYPE_P (type))
{
*punsignedp = POINTERS_EXTEND_UNSIGNED;
return Pmode;
}
return mode;
}
--
Eric Botcazou
Re: [x32] PATCH: PR middle-end/47725: [x32] error: unable to find a register to spill in class DIREG
On Wed, Mar 23, 2011 at 1:22 AM, Eric Botcazou wrote:
>> It is:
>>
>> op0 = parmreg;
>> op1 = validated_mem;
>> if (icode != CODE_FOR_nothing
>> && insn_data[icode].operand[0].predicate (op0,
>> promoted_nominal_mode) && insn_data[icode].operand[1].predicate (op1,
>> data->passed_mode)) {
>> enum rtx_code code = unsignedp ? ZERO_EXTEND : SIGN_EXTEND;
>> rtx insn, insns;
>> HARD_REG_SET hardregs;
>>
>> start_sequence ();
>> insn = gen_extend_insn (op0, op1, promoted_nominal_mode,
>> data->passed_mode, unsignedp);
>> emit_insn (insn);
>> insns = get_insns ();
>
> Sure, but why is need_conversion set to true?
>
Pointer is promoted to Pmode from ptr_mode.
--
H.J.
---
#0 promote_mode (type=0x70e05540, mode=SImode, punsignedp=0x7fffd76c)
at /export/gnu/import/git/gcc-x32/gcc/explow.c:808
#1 0x009ef004 in default_promote_function_mode (type=0x70e05540,
mode=SImode, punsignedp=0x7fffd76c, funtype=0x70cec3f0,
for_return=2) at /export/gnu/import/git/gcc-x32/gcc/targhooks.c:128
#2 0x006d7155 in promote_function_mode (type=0x70e05540,
mode=SImode, punsignedp=0x7fffd76c, funtype=0x70cec3f0,
for_return=2) at /export/gnu/import/git/gcc-x32/gcc/explow.c:774
#3 0x007cf7de in assign_parm_setup_reg (all=0x7fffda10,
parm=0x70dec880, data=0x7fffd980)
at /export/gnu/import/git/gcc-x32/gcc/function.c:2941
Re: [x32] PATCH: PR middle-end/47725: [x32] error: unable to find a register to spill in class DIREG
> It is:
>
> op0 = parmreg;
> op1 = validated_mem;
> if (icode != CODE_FOR_nothing
> && insn_data[icode].operand[0].predicate (op0,
> promoted_nominal_mode) && insn_data[icode].operand[1].predicate (op1,
> data->passed_mode)) {
> enum rtx_code code = unsignedp ? ZERO_EXTEND : SIGN_EXTEND;
> rtx insn, insns;
> HARD_REG_SET hardregs;
>
> start_sequence ();
> insn = gen_extend_insn (op0, op1, promoted_nominal_mode,
> data->passed_mode, unsignedp);
> emit_insn (insn);
> insns = get_insns ();
Sure, but why is need_conversion set to true?
--
Eric Botcazou
Re: [x32] PATCH: PR middle-end/47725: [x32] error: unable to find a register to spill in class DIREG
On Tue, Mar 22, 2011 at 1:05 PM, Eric Botcazou wrote:
>> It leads 2 problems:
>>
>> 1. Redundant zero-extension at function entry.
>> 2. combine doesn't check zero-extension on hard register and leads to
>> internal compiler error.
>>
>> Is there a way to avoid redundant zero-extension at function entry to
>> solve both problems?
>
> Eliminating the redundant extension in the callee seems indeed to be
> appealing.
> You need to find out who decides that the incoming parameter needs to be zero-
> extended. Is that the call to promote_function_mode in assign_parm_setup_reg?
>
It is:
op0 = parmreg;
op1 = validated_mem;
if (icode != CODE_FOR_nothing
&& insn_data[icode].operand[0].predicate (op0, promoted_nominal_mode)
&& insn_data[icode].operand[1].predicate (op1, data->passed_mode))
{
enum rtx_code code = unsignedp ? ZERO_EXTEND : SIGN_EXTEND;
rtx insn, insns;
HARD_REG_SET hardregs;
start_sequence ();
insn = gen_extend_insn (op0, op1, promoted_nominal_mode,
data->passed_mode, unsignedp);
emit_insn (insn);
insns = get_insns ();
in assign_parm_setup_reg.
--
H.J.
Re: [x32] PATCH: PR middle-end/47725: [x32] error: unable to find a register to spill in class DIREG
> It leads 2 problems: > > 1. Redundant zero-extension at function entry. > 2. combine doesn't check zero-extension on hard register and leads to > internal compiler error. > > Is there a way to avoid redundant zero-extension at function entry to > solve both problems? Eliminating the redundant extension in the callee seems indeed to be appealing. You need to find out who decides that the incoming parameter needs to be zero- extended. Is that the call to promote_function_mode in assign_parm_setup_reg? -- Eric Botcazou
Re: [x32] PATCH: PR middle-end/47725: [x32] error: unable to find a register to spill in class DIREG
On Thu, Mar 17, 2011 at 9:00 PM, H.J. Lu wrote: > On Thu, Mar 17, 2011 at 5:30 PM, H.J. Lu wrote: >> On Tue, Feb 15, 2011 at 2:55 PM, Bernd Schmidt >> wrote: >>> On 02/14/2011 08:46 PM, Eric Botcazou wrote: > I agree with Jeff that combine would be the correct place to fix this. > At least it takes class_likely_spilled_p into account, so it will > restrict only those machines where extending the lifetime of hard regs > is dangerous. OK, but I don't see how copying to a new pseudo would interfere with that. >>> >>> For one thing, the set no longer matches the REG_EQUIV note we make >>> here, and that does seem to interfere with the optimization. I've tested >>> both patches on ARM, -march=armv7-a. The combiner patch produced no code >>> changes. The function.c patch produced regressions (increased register >>> pressure). Both results are as expected. >>> >>> To put it another way: the combiner change is conservatively correct, >>> and necessary if we're going to have extends of hard registers in the >>> RTL. The function.c change is demonstrably incorrect as shown by the ARM >>> codegen regressions. >>> >> >> I checked in my patch into trunk. >> > > I noticed that for x32, all pointers passed in registers are zero-extended > by caller. If we can fix > > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48085 > > by avoiding zero-extension in callee, this issue won't happen for x32. I will > revert the combine change for now and try to implement this approach. > The issues are 1. When passing a 32bit integer parameter in a register, hardware always zero-extends it to 64bit. I can update x32 psABI to document it, which costs nothing to implement in GCC. 2. assign_parm_setup_reg zero-extends 32bit pointer in register to 64bit, which is redundant. It leads 2 problems: 1. Redundant zero-extension at function entry. 2. combine doesn't check zero-extension on hard register and leads to internal compiler error. Is there a way to avoid redundant zero-extension at function entry to solve both problems? Thanks. -- H.J.
Re: [x32] PATCH: PR middle-end/47725: [x32] error: unable to find a register to spill in class DIREG
On Thu, Mar 17, 2011 at 5:30 PM, H.J. Lu wrote: > On Tue, Feb 15, 2011 at 2:55 PM, Bernd Schmidt > wrote: >> On 02/14/2011 08:46 PM, Eric Botcazou wrote: I agree with Jeff that combine would be the correct place to fix this. At least it takes class_likely_spilled_p into account, so it will restrict only those machines where extending the lifetime of hard regs is dangerous. >>> >>> OK, but I don't see how copying to a new pseudo would interfere with that. >> >> For one thing, the set no longer matches the REG_EQUIV note we make >> here, and that does seem to interfere with the optimization. I've tested >> both patches on ARM, -march=armv7-a. The combiner patch produced no code >> changes. The function.c patch produced regressions (increased register >> pressure). Both results are as expected. >> >> To put it another way: the combiner change is conservatively correct, >> and necessary if we're going to have extends of hard registers in the >> RTL. The function.c change is demonstrably incorrect as shown by the ARM >> codegen regressions. >> > > I checked in my patch into trunk. > I noticed that for x32, all pointers passed in registers are zero-extended by caller. If we can fix http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48085 by avoiding zero-extension in callee, this issue won't happen for x32. I will revert the combine change for now and try to implement this approach. Thanks. -- H.J.
Re: [x32] PATCH: PR middle-end/47725: [x32] error: unable to find a register to spill in class DIREG
On Tue, Feb 15, 2011 at 2:55 PM, Bernd Schmidt wrote: > On 02/14/2011 08:46 PM, Eric Botcazou wrote: >>> I agree with Jeff that combine would be the correct place to fix this. >>> At least it takes class_likely_spilled_p into account, so it will >>> restrict only those machines where extending the lifetime of hard regs >>> is dangerous. >> >> OK, but I don't see how copying to a new pseudo would interfere with that. > > For one thing, the set no longer matches the REG_EQUIV note we make > here, and that does seem to interfere with the optimization. I've tested > both patches on ARM, -march=armv7-a. The combiner patch produced no code > changes. The function.c patch produced regressions (increased register > pressure). Both results are as expected. > > To put it another way: the combiner change is conservatively correct, > and necessary if we're going to have extends of hard registers in the > RTL. The function.c change is demonstrably incorrect as shown by the ARM > codegen regressions. > I checked in my patch into trunk. Thanks. -- H.J.
