"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/ > -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)),