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: RFA: Clean up ADDRESS handling in alias.c


"H.J. Lu" <hjl.tools@gmail.com> writes:
> On Sun, Apr 15, 2012 at 8:11 AM, Richard Sandiford
> <rdsandiford@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/
> -B/home/regress/tbox/objs/i686-pc-linux-gnu/lib/ -isystem
> /home/regress/tbox/objs/i686-pc-linux-gnu/include -isystem
> /home/regress/tbox/objs/i686-pc-linux-gnu/sys-include    -g -O2 -O2
> -g -O2 -DIN_GCC   -W -Wall -Wwrite-strings -Wcast-qual
> -Wstrict-prototypes -Wmissing-prototypes -Wold-style-definition
> -isystem ./include   -fpic -g -DIN_LIBGCC2 -fbuilding-libgcc
> -fno-stack-protector   -fpic -I. -I. -I../.././gcc
> -I/home/regress/tbox/svn-gcc/libgcc
> -I/home/regress/tbox/svn-gcc/libgcc/.
> -I/home/regress/tbox/svn-gcc/libgcc/../gcc
> -I/home/regress/tbox/svn-gcc/libgcc/../include
> -I/home/regress/tbox/svn-gcc/libgcc/config/libbid
> -DENABLE_DECIMAL_BID_FORMAT -DHAVE_CC_TLS  -DUSE_TLS -o __main_s.o -MT
> __main_s.o -MD -MP -MF __main_s.dep -DSHARED -DL__main -c
> /home/regress/tbox/svn-gcc/libgcc/libgcc2.c
> In file included from
> /home/regress/tbox/svn-gcc/libgcc/unwind-dw2-fde-dip.c:79:0:
> /home/regress/tbox/svn-gcc/libgcc/unwind-dw2-fde.c: In function 'search_object':
> /home/regress/tbox/svn-gcc/libgcc/unwind-dw2-fde.c:997:1: internal
> compiler error: Segmentation fault
>  }
>  ^
> Please submit a full bug report,
> with preprocessed source if appropriate.
> See <http://gcc.gnu.org/bugs.html> for instructions.
> make[3]: *** [unwind-dw2-fde-dip.o] Error 1
> make[3]: *** Waiting for unfinished jobs....
> make[3]: Leaving directory
> `/home/regress/tbox/native/build/i686-pc-linux-gnu/libgcc'
> make[2]: *** [all-stage1-target-libgcc] Error 2
> make[2]: Leaving directory `/home/regress/tbox/native/build'
> make[1]: *** [stage1-bubble] Error 2
> make[1]: Leaving directory `/home/regress/tbox/native/build'
> make: *** [bootstrap] Error 2

Gah, sorry, the perils of testing on a 64-bit host.  This patch seems
to fix it.  Applied after getting past the stage 1 failure.

Richard


gcc/
	PR bootstrap/53021
	* rtl.c (rtx_code_size): Handle ADDRESS.

Index: gcc/rtl.c
===================================================================
--- gcc/rtl.c	2012-04-17 21:07:55.000000000 +0100
+++ gcc/rtl.c	2012-04-17 21:07:55.524619837 +0100
@@ -110,7 +110,8 @@ #define DEF_RTL_EXPR(ENUM, NAME, FORMAT,
 
 const unsigned char rtx_code_size[NUM_RTX_CODE] = {
 #define DEF_RTL_EXPR(ENUM, NAME, FORMAT, CLASS)				\
-  ((ENUM) == CONST_INT || (ENUM) == CONST_DOUBLE || (ENUM) == CONST_FIXED\
+  (((ENUM) == CONST_INT || (ENUM) == CONST_DOUBLE			\
+    || (ENUM) == CONST_FIXED || (ENUM) == ADDRESS)			\
    ? RTX_HDR_SIZE + (sizeof FORMAT - 1) * sizeof (HOST_WIDE_INT)	\
    : RTX_HDR_SIZE + (sizeof FORMAT - 1) * sizeof (rtunion)),
 


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