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


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

-- 
H.J.


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