Re: RFA: Clean up ADDRESS handling in alias.c
On Sun, Apr 15, 2012 at 5:11 PM, Richard Sandiford
rdsandif...@googlemail.com wrote:
The comment in alias.c says:
The contents of an ADDRESS is not normally used, the mode of the
ADDRESS determines whether the ADDRESS is a function argument or some
other special value. Pointer equality, not rtx_equal_p, determines whether
two ADDRESS expressions refer to the same base address.
The only use of the contents of an ADDRESS is for determining if the
current function performs nonlocal memory memory references for the
purposes of marking the function as a constant function. */
The first paragraph is a bit misleading IMO. AFAICT, rtx_equal_p has
always given ADDRESS the full recursive treatment, rather than saying
that pointer equality determines ADDRESS equality. (This is in contrast
to something like VALUE, where pointer equality is used.) And AFAICT
we've always had:
static int
base_alias_check (rtx x, rtx y, enum machine_mode x_mode,
enum machine_mode y_mode)
{
...
/* If the base addresses are equal nothing is known about aliasing. */
if (rtx_equal_p (x_base, y_base))
return 1;
...
}
So I think the contents of an ADDRESS _are_ used to distinguish
between different bases.
The second paragraph ceased to be true in 2005 when the pure/const
analysis moved to its own IPA pass. Nothing now looks at the contents
beyond rtx_equal_p.
Also, base_alias_check effectively treats all arguments as a single base.
That makes conceptual sense, because this analysis isn't strong enough
to determine whether arguments are base values at all, never mind whether
accesses based on different arguments conflict. But the fact that we have
a single base isn't obvious from the way the code is written, because we
create several separate, non-rtx_equal_p, ADDRESSes to represent arguments.
See:
for (i = 0; i FIRST_PSEUDO_REGISTER; i++)
/* Check whether this register can hold an incoming pointer
argument. FUNCTION_ARG_REGNO_P tests outgoing register
numbers, so translate if necessary due to register windows. */
if (FUNCTION_ARG_REGNO_P (OUTGOING_REGNO (i))
HARD_REGNO_MODE_OK (i, Pmode))
static_reg_base_value[i]
= gen_rtx_ADDRESS (VOIDmode, gen_rtx_REG (Pmode, i));
and:
/* Check for an argument passed in memory. Only record in the
copying-arguments block; it is too hard to track changes
otherwise. */
if (copying_arguments
(XEXP (src, 0) == arg_pointer_rtx
|| (GET_CODE (XEXP (src, 0)) == PLUS
XEXP (XEXP (src, 0), 0) == arg_pointer_rtx)))
return gen_rtx_ADDRESS (VOIDmode, src);
I think it would be cleaner and less wasteful to use a single rtx for
the single base (really potential base).
So if we wanted to, we could now remove the operand from ADDRESS and
simply rely on pointer equality. I'm a bit reluctant to do that though.
It would make debugging harder, and it would mean either adding knowledge
of this alias-specific code to other files (specifically rtl.c:rtx_equal_p),
or adding special ADDRESS shortcuts to alias.c. But I think the code
would be more obvious if we replaced the rtx operand with a unique id,
which is what we already use for the REG_NOALIAS case:
new_reg_base_value[regno] = gen_rtx_ADDRESS (Pmode,
GEN_INT (unique_id++));
And if we do that, we can make the id a direct operand of the ADDRESS,
rather than a CONST_INT subrtx[*]. That should make rtx_equal_p cheaper too.
[*] I'm trying to get rid of CONST_INTs like these that have
no obvious mode.
All of which led to the patch below. I checked that it didn't change
the code generated at -O2 for a recent set of cc1 .ii files. Also
bootstrapped regression-tested on x86_64-linux-gnu. OK to install?
To cover my back: I'm just trying to rewrite the current code according
to its current assumptions. Whether those assumptions are correct or not
is always open to debate...
This all looks reasonable and matches what I discovered by reverse
engineering the last time I ran into ADDRESSes ...
So, ok, given that nobody else has commented yet.
Thanks,
Richard.
Richard
gcc/
* rtl.def (ADDRESS): Turn operand into a HOST_WIDE_INT.
* alias.c (reg_base_value): Expand and update comment.
(arg_base_value): New variable.
(unique_id): Move up file.
(unique_base_value, unique_base_value_p, known_base_value_p): New.
(find_base_value): Use arg_base_value and known_base_value_p.
(record_set): Document REG_NOALIAS handling. Use unique_base_value.
(find_base_term): Use known_base_value_p.
(base_alias_check): Use unique_base_value_p.
(init_alias_target): Initialize arg_base_value. Use unique_base_value.
(init_alias_analysis): Use 1 as the first id for REG_NOALIAS bases.
Index: gcc/rtl.def
Re: RFA: Clean up ADDRESS handling in alias.c
On Sun, Apr 15, 2012 at 8:11 AM, Richard Sandiford
rdsandif...@googlemail.com wrote:
The comment in alias.c says:
The contents of an ADDRESS is not normally used, the mode of the
ADDRESS determines whether the ADDRESS is a function argument or some
other special value. Pointer equality, not rtx_equal_p, determines whether
two ADDRESS expressions refer to the same base address.
The only use of the contents of an ADDRESS is for determining if the
current function performs nonlocal memory memory references for the
purposes of marking the function as a constant function. */
The first paragraph is a bit misleading IMO. AFAICT, rtx_equal_p has
always given ADDRESS the full recursive treatment, rather than saying
that pointer equality determines ADDRESS equality. (This is in contrast
to something like VALUE, where pointer equality is used.) And AFAICT
we've always had:
static int
base_alias_check (rtx x, rtx y, enum machine_mode x_mode,
enum machine_mode y_mode)
{
...
/* If the base addresses are equal nothing is known about aliasing. */
if (rtx_equal_p (x_base, y_base))
return 1;
...
}
So I think the contents of an ADDRESS _are_ used to distinguish
between different bases.
The second paragraph ceased to be true in 2005 when the pure/const
analysis moved to its own IPA pass. Nothing now looks at the contents
beyond rtx_equal_p.
Also, base_alias_check effectively treats all arguments as a single base.
That makes conceptual sense, because this analysis isn't strong enough
to determine whether arguments are base values at all, never mind whether
accesses based on different arguments conflict. But the fact that we have
a single base isn't obvious from the way the code is written, because we
create several separate, non-rtx_equal_p, ADDRESSes to represent arguments.
See:
for (i = 0; i FIRST_PSEUDO_REGISTER; i++)
/* Check whether this register can hold an incoming pointer
argument. FUNCTION_ARG_REGNO_P tests outgoing register
numbers, so translate if necessary due to register windows. */
if (FUNCTION_ARG_REGNO_P (OUTGOING_REGNO (i))
HARD_REGNO_MODE_OK (i, Pmode))
static_reg_base_value[i]
= gen_rtx_ADDRESS (VOIDmode, gen_rtx_REG (Pmode, i));
and:
/* Check for an argument passed in memory. Only record in the
copying-arguments block; it is too hard to track changes
otherwise. */
if (copying_arguments
(XEXP (src, 0) == arg_pointer_rtx
|| (GET_CODE (XEXP (src, 0)) == PLUS
XEXP (XEXP (src, 0), 0) == arg_pointer_rtx)))
return gen_rtx_ADDRESS (VOIDmode, src);
I think it would be cleaner and less wasteful to use a single rtx for
the single base (really potential base).
So if we wanted to, we could now remove the operand from ADDRESS and
simply rely on pointer equality. I'm a bit reluctant to do that though.
It would make debugging harder, and it would mean either adding knowledge
of this alias-specific code to other files (specifically rtl.c:rtx_equal_p),
or adding special ADDRESS shortcuts to alias.c. But I think the code
would be more obvious if we replaced the rtx operand with a unique id,
which is what we already use for the REG_NOALIAS case:
new_reg_base_value[regno] = gen_rtx_ADDRESS (Pmode,
GEN_INT (unique_id++));
And if we do that, we can make the id a direct operand of the ADDRESS,
rather than a CONST_INT subrtx[*]. That should make rtx_equal_p cheaper too.
[*] I'm trying to get rid of CONST_INTs like these that have
no obvious mode.
All of which led to the patch below. I checked that it didn't change
the code generated at -O2 for a recent set of cc1 .ii files. Also
bootstrapped regression-tested on x86_64-linux-gnu. OK to install?
To cover my back: I'm just trying to rewrite the current code according
to its current assumptions. Whether those assumptions are correct or not
is always open to debate...
Richard
gcc/
* rtl.def (ADDRESS): Turn operand into a HOST_WIDE_INT.
* alias.c (reg_base_value): Expand and update comment.
(arg_base_value): New variable.
(unique_id): Move up file.
(unique_base_value, unique_base_value_p, known_base_value_p): New.
(find_base_value): Use arg_base_value and known_base_value_p.
(record_set): Document REG_NOALIAS handling. Use unique_base_value.
(find_base_term): Use known_base_value_p.
(base_alias_check): Use unique_base_value_p.
(init_alias_target): Initialize arg_base_value. Use unique_base_value.
(init_alias_analysis): Use 1 as the first id for REG_NOALIAS bases.
This breaks bootstrap on Linux/x86:
home/regress/tbox/native/build/./gcc/xgcc
-B/home/regress/tbox/native/build/./gcc/
-B/home/regress/tbox/objs/i686-pc-linux-gnu/bin/
Re: RFA: Clean up ADDRESS handling in alias.c
H.J. Lu hjl.to...@gmail.com writes:
On Sun, Apr 15, 2012 at 8:11 AM, Richard Sandiford
rdsandif...@googlemail.com wrote:
The comment in alias.c says:
The contents of an ADDRESS is not normally used, the mode of the
ADDRESS determines whether the ADDRESS is a function argument or some
other special value. Pointer equality, not rtx_equal_p, determines whether
two ADDRESS expressions refer to the same base address.
The only use of the contents of an ADDRESS is for determining if the
current function performs nonlocal memory memory references for the
purposes of marking the function as a constant function. */
The first paragraph is a bit misleading IMO. AFAICT, rtx_equal_p has
always given ADDRESS the full recursive treatment, rather than saying
that pointer equality determines ADDRESS equality. (This is in contrast
to something like VALUE, where pointer equality is used.) And AFAICT
we've always had:
static int
base_alias_check (rtx x, rtx y, enum machine_mode x_mode,
enum machine_mode y_mode)
{
...
/* If the base addresses are equal nothing is known about aliasing. */
if (rtx_equal_p (x_base, y_base))
return 1;
...
}
So I think the contents of an ADDRESS _are_ used to distinguish
between different bases.
The second paragraph ceased to be true in 2005 when the pure/const
analysis moved to its own IPA pass. Nothing now looks at the contents
beyond rtx_equal_p.
Also, base_alias_check effectively treats all arguments as a single base.
That makes conceptual sense, because this analysis isn't strong enough
to determine whether arguments are base values at all, never mind whether
accesses based on different arguments conflict. But the fact that we have
a single base isn't obvious from the way the code is written, because we
create several separate, non-rtx_equal_p, ADDRESSes to represent arguments.
See:
for (i = 0; i FIRST_PSEUDO_REGISTER; i++)
/* Check whether this register can hold an incoming pointer
argument. FUNCTION_ARG_REGNO_P tests outgoing register
numbers, so translate if necessary due to register windows. */
if (FUNCTION_ARG_REGNO_P (OUTGOING_REGNO (i))
HARD_REGNO_MODE_OK (i, Pmode))
static_reg_base_value[i]
= gen_rtx_ADDRESS (VOIDmode, gen_rtx_REG (Pmode, i));
and:
/* Check for an argument passed in memory. Only record in the
copying-arguments block; it is too hard to track changes
otherwise. */
if (copying_arguments
(XEXP (src, 0) == arg_pointer_rtx
|| (GET_CODE (XEXP (src, 0)) == PLUS
XEXP (XEXP (src, 0), 0) == arg_pointer_rtx)))
return gen_rtx_ADDRESS (VOIDmode, src);
I think it would be cleaner and less wasteful to use a single rtx for
the single base (really potential base).
So if we wanted to, we could now remove the operand from ADDRESS and
simply rely on pointer equality. I'm a bit reluctant to do that though.
It would make debugging harder, and it would mean either adding knowledge
of this alias-specific code to other files (specifically rtl.c:rtx_equal_p),
or adding special ADDRESS shortcuts to alias.c. But I think the code
would be more obvious if we replaced the rtx operand with a unique id,
which is what we already use for the REG_NOALIAS case:
new_reg_base_value[regno] = gen_rtx_ADDRESS (Pmode,
GEN_INT (unique_id++));
And if we do that, we can make the id a direct operand of the ADDRESS,
rather than a CONST_INT subrtx[*]. That should make rtx_equal_p cheaper too.
[*] I'm trying to get rid of CONST_INTs like these that have
no obvious mode.
All of which led to the patch below. I checked that it didn't change
the code generated at -O2 for a recent set of cc1 .ii files. Also
bootstrapped regression-tested on x86_64-linux-gnu. OK to install?
To cover my back: I'm just trying to rewrite the current code according
to its current assumptions. Whether those assumptions are correct or not
is always open to debate...
Richard
gcc/
* rtl.def (ADDRESS): Turn operand into a HOST_WIDE_INT.
* alias.c (reg_base_value): Expand and update comment.
(arg_base_value): New variable.
(unique_id): Move up file.
(unique_base_value, unique_base_value_p, known_base_value_p): New.
(find_base_value): Use arg_base_value and known_base_value_p.
(record_set): Document REG_NOALIAS handling. Use unique_base_value.
(find_base_term): Use known_base_value_p.
(base_alias_check): Use unique_base_value_p.
(init_alias_target): Initialize arg_base_value. Use
unique_base_value.
(init_alias_analysis): Use 1 as the first id for REG_NOALIAS bases.
This breaks bootstrap on Linux/x86:
home/regress/tbox/native/build/./gcc/xgcc
-B/home/regress/tbox/native/build/./gcc/
-B/home/regress/tbox/objs/i686-pc-linux-gnu/bin/
RFA: Clean up ADDRESS handling in alias.c
The comment in alias.c says:
The contents of an ADDRESS is not normally used, the mode of the
ADDRESS determines whether the ADDRESS is a function argument or some
other special value. Pointer equality, not rtx_equal_p, determines whether
two ADDRESS expressions refer to the same base address.
The only use of the contents of an ADDRESS is for determining if the
current function performs nonlocal memory memory references for the
purposes of marking the function as a constant function. */
The first paragraph is a bit misleading IMO. AFAICT, rtx_equal_p has
always given ADDRESS the full recursive treatment, rather than saying
that pointer equality determines ADDRESS equality. (This is in contrast
to something like VALUE, where pointer equality is used.) And AFAICT
we've always had:
static int
base_alias_check (rtx x, rtx y, enum machine_mode x_mode,
enum machine_mode y_mode)
{
...
/* If the base addresses are equal nothing is known about aliasing. */
if (rtx_equal_p (x_base, y_base))
return 1;
...
}
So I think the contents of an ADDRESS _are_ used to distinguish
between different bases.
The second paragraph ceased to be true in 2005 when the pure/const
analysis moved to its own IPA pass. Nothing now looks at the contents
beyond rtx_equal_p.
Also, base_alias_check effectively treats all arguments as a single base.
That makes conceptual sense, because this analysis isn't strong enough
to determine whether arguments are base values at all, never mind whether
accesses based on different arguments conflict. But the fact that we have
a single base isn't obvious from the way the code is written, because we
create several separate, non-rtx_equal_p, ADDRESSes to represent arguments.
See:
for (i = 0; i FIRST_PSEUDO_REGISTER; i++)
/* Check whether this register can hold an incoming pointer
argument. FUNCTION_ARG_REGNO_P tests outgoing register
numbers, so translate if necessary due to register windows. */
if (FUNCTION_ARG_REGNO_P (OUTGOING_REGNO (i))
HARD_REGNO_MODE_OK (i, Pmode))
static_reg_base_value[i]
= gen_rtx_ADDRESS (VOIDmode, gen_rtx_REG (Pmode, i));
and:
/* Check for an argument passed in memory. Only record in the
copying-arguments block; it is too hard to track changes
otherwise. */
if (copying_arguments
(XEXP (src, 0) == arg_pointer_rtx
|| (GET_CODE (XEXP (src, 0)) == PLUS
XEXP (XEXP (src, 0), 0) == arg_pointer_rtx)))
return gen_rtx_ADDRESS (VOIDmode, src);
I think it would be cleaner and less wasteful to use a single rtx for
the single base (really potential base).
So if we wanted to, we could now remove the operand from ADDRESS and
simply rely on pointer equality. I'm a bit reluctant to do that though.
It would make debugging harder, and it would mean either adding knowledge
of this alias-specific code to other files (specifically rtl.c:rtx_equal_p),
or adding special ADDRESS shortcuts to alias.c. But I think the code
would be more obvious if we replaced the rtx operand with a unique id,
which is what we already use for the REG_NOALIAS case:
new_reg_base_value[regno] = gen_rtx_ADDRESS (Pmode,
GEN_INT (unique_id++));
And if we do that, we can make the id a direct operand of the ADDRESS,
rather than a CONST_INT subrtx[*]. That should make rtx_equal_p cheaper too.
[*] I'm trying to get rid of CONST_INTs like these that have
no obvious mode.
All of which led to the patch below. I checked that it didn't change
the code generated at -O2 for a recent set of cc1 .ii files. Also
bootstrapped regression-tested on x86_64-linux-gnu. OK to install?
To cover my back: I'm just trying to rewrite the current code according
to its current assumptions. Whether those assumptions are correct or not
is always open to debate...
Richard
gcc/
* rtl.def (ADDRESS): Turn operand into a HOST_WIDE_INT.
* alias.c (reg_base_value): Expand and update comment.
(arg_base_value): New variable.
(unique_id): Move up file.
(unique_base_value, unique_base_value_p, known_base_value_p): New.
(find_base_value): Use arg_base_value and known_base_value_p.
(record_set): Document REG_NOALIAS handling. Use unique_base_value.
(find_base_term): Use known_base_value_p.
(base_alias_check): Use unique_base_value_p.
(init_alias_target): Initialize arg_base_value. Use unique_base_value.
(init_alias_analysis): Use 1 as the first id for REG_NOALIAS bases.
Index: gcc/rtl.def
===
--- gcc/rtl.def 2012-04-15 15:23:52.747632394 +0100
+++ gcc/rtl.def 2012-04-15 15:58:48.234515667 +0100
@@ -109,8 +109,8 @@ DEF_RTL_EXPR(INSN_LIST, insn_list, ue
`emit_insn' takes the SEQUENCE apart and makes separate insns. */
