* Claudiu Zissulescu <claz...@gmail.com> [2019-06-06 10:32:11 +0300]:
> Hi Andrew,
>
> This is a proposed fix for bugzilla PR89838 issue. It also needs to be
> backported to gcc9 and, eventually, gcc8 branches.
>
> Ok to apply?
> Claudiu
I had a look through the patch and found just one small nit detailed
below. Otherwise this looks fine.
Thanks,
Andrew
>
> gcc/
> xxxx-xx-xx Claudiu Zissulescu <claz...@synopsys.com>
>
> * config/arc/arc.c (arc_symbol_binds_local_p): New function.
> (arc_legitimize_pic_address): Simplify and cleanup the function.
> (SYMBOLIC_CONST): Remove.
> (prepare_pic_move): Likewise.
> (prepare_move_operands): Handle complex mov cases here.
> (arc_legitimize_address_0): Remove call to
> arc_legitimize_pic_address.
> (arc_legitimize_address): Remove call to
> arc_legitimize_tls_address.
> * config/arc/arc.md (movqi_insn): Allow Cm3 match.
> (movhi_insn): Likewise.
>
> /gcc/testsuite
> xxxx-xx-xx Claudiu Zissulescu <claz...@synopsys.com>
>
> * gcc.target/arc/pr89838.c: New file.
> ---
> gcc/config/arc/arc.c | 215 +++++++------------------
> gcc/config/arc/arc.md | 8 +-
> gcc/testsuite/gcc.target/arc/pr89838.c | 16 ++
> 3 files changed, 77 insertions(+), 162 deletions(-)
> create mode 100644 gcc/testsuite/gcc.target/arc/pr89838.c
>
> diff --git a/gcc/config/arc/arc.c b/gcc/config/arc/arc.c
> index 20dfae66370..a5ee5c49a8f 100644
> --- a/gcc/config/arc/arc.c
> +++ b/gcc/config/arc/arc.c
> @@ -5986,130 +5986,47 @@ arc_legitimize_tls_address (rtx addr, enum tls_model
> model)
> }
> }
>
> -/* Legitimize a pic address reference in ORIG.
> - The return value is the legitimated address.
> - If OLDX is non-zero, it is the target to assign the address to first. */
> +/* Return true if SYMBOL_REF X binds locally. */
>
> -static rtx
> -arc_legitimize_pic_address (rtx orig, rtx oldx)
> +static bool
> +arc_symbol_binds_local_p (const_rtx x)
> {
> - rtx addr = orig;
> - rtx pat = orig;
> - rtx base;
> + return (SYMBOL_REF_DECL (x)
> + ? targetm.binds_local_p (SYMBOL_REF_DECL (x))
> + : SYMBOL_REF_LOCAL_P (x));
> +}
> +
> +/* Legitimize a pic address reference in ORIG. The return value is
> + the legitimated address. */
This comment needs updating I think, it references ORIG, but there is
no ORIG in arc_legitimize_pic.
>
> - if (oldx == orig)
> - oldx = NULL;
> +static rtx
> +arc_legitimize_pic_address (rtx addr)
> +{
> + if (!flag_pic)
> + return addr;
>
> - if (GET_CODE (addr) == LABEL_REF)
> - ; /* Do nothing. */
> - else if (GET_CODE (addr) == SYMBOL_REF)
> + switch (GET_CODE (addr))
> {
> - enum tls_model model = SYMBOL_REF_TLS_MODEL (addr);
> - if (model != 0)
> - return arc_legitimize_tls_address (addr, model);
> - else if (!flag_pic)
> - return orig;
> - else if (CONSTANT_POOL_ADDRESS_P (addr) || SYMBOL_REF_LOCAL_P (addr))
> - return arc_unspec_offset (addr, ARC_UNSPEC_GOTOFFPC);
> + case SYMBOL_REF:
> + /* TLS symbols are handled in different place. */
> + if (SYMBOL_REF_TLS_MODEL (addr))
> + return addr;
>
> /* This symbol must be referenced via a load from the Global
> Offset Table (@GOTPC). */
> - pat = arc_unspec_offset (addr, ARC_UNSPEC_GOT);
> - pat = gen_const_mem (Pmode, pat);
> -
> - if (oldx == NULL)
> - oldx = gen_reg_rtx (Pmode);
> -
> - emit_move_insn (oldx, pat);
> - pat = oldx;
> - }
> - else
> - {
> - if (GET_CODE (addr) == CONST)
> - {
> - addr = XEXP (addr, 0);
> - if (GET_CODE (addr) == UNSPEC)
> - {
> - /* Check that the unspec is one of the ones we generate? */
> - return orig;
> - }
> - /* fwprop is placing in the REG_EQUIV notes constant pic
> - unspecs expressions. Then, loop may use these notes for
> - optimizations resulting in complex patterns that are not
> - supported by the current implementation. The following
> - two if-cases are simplifying the complex patters to
> - simpler ones. */
> - else if (GET_CODE (addr) == MINUS)
> - {
> - rtx op0 = XEXP (addr, 0);
> - rtx op1 = XEXP (addr, 1);
> - gcc_assert (oldx);
> - gcc_assert (GET_CODE (op1) == UNSPEC);
> -
> - emit_move_insn (oldx,
> - gen_rtx_CONST (SImode,
> - arc_legitimize_pic_address (op1,
> -
> NULL_RTX)));
> - emit_insn (gen_rtx_SET (oldx, gen_rtx_MINUS (SImode, op0, oldx)));
> - return oldx;
> -
> - }
> - else if (GET_CODE (addr) != PLUS)
> - {
> - rtx tmp = XEXP (addr, 0);
> - enum rtx_code code = GET_CODE (addr);
> -
> - /* It only works for UNARY operations. */
> - gcc_assert (UNARY_P (addr));
> - gcc_assert (GET_CODE (tmp) == UNSPEC);
> - gcc_assert (oldx);
> -
> - emit_move_insn
> - (oldx,
> - gen_rtx_CONST (SImode,
> - arc_legitimize_pic_address (tmp,
> - NULL_RTX)));
> -
> - emit_insn (gen_rtx_SET (oldx,
> - gen_rtx_fmt_ee (code, SImode,
> - oldx, const0_rtx)));
> -
> - return oldx;
> - }
> - else
> - {
> - gcc_assert (GET_CODE (addr) == PLUS);
> - if (GET_CODE (XEXP (addr, 0)) == UNSPEC)
> - return orig;
> - }
> - }
> -
> - if (GET_CODE (addr) == PLUS)
> - {
> - rtx op0 = XEXP (addr, 0), op1 = XEXP (addr, 1);
> -
> - base = arc_legitimize_pic_address (op0, oldx);
> - pat = arc_legitimize_pic_address (op1,
> - base == oldx ? NULL_RTX : oldx);
> + if (!arc_symbol_binds_local_p (addr))
> + return gen_const_mem (Pmode, arc_unspec_offset (addr, ARC_UNSPEC_GOT));
>
> - if (base == op0 && pat == op1)
> - return orig;
> + /* Local symb: use @pcl to access it. */
> + /* Fall through. */
> + case LABEL_REF:
> + return arc_unspec_offset (addr, ARC_UNSPEC_GOTOFFPC);
>
> - if (GET_CODE (pat) == CONST_INT)
> - pat = plus_constant (Pmode, base, INTVAL (pat));
> - else
> - {
> - if (GET_CODE (pat) == PLUS && CONSTANT_P (XEXP (pat, 1)))
> - {
> - base = gen_rtx_PLUS (Pmode, base, XEXP (pat, 0));
> - pat = XEXP (pat, 1);
> - }
> - pat = gen_rtx_PLUS (Pmode, base, pat);
> - }
> - }
> + default:
> + break;
> }
>
> - return pat;
> + return addr;
> }
>
> /* Output address constant X to FILE, taking PIC into account. */
> @@ -6271,28 +6188,6 @@ arc_output_pic_addr_const (FILE * file, rtx x, int
> code)
> }
> }
>
> -#define SYMBOLIC_CONST(X) \
> -(GET_CODE (X) == SYMBOL_REF \
> - || GET_CODE (X) == LABEL_REF
> \
> - || (GET_CODE (X) == CONST && symbolic_reference_mentioned_p (X)))
> -
> -/* Emit insns to move operands[1] into operands[0]. */
> -
> -static void
> -prepare_pic_move (rtx *operands, machine_mode)
> -{
> - if (GET_CODE (operands[0]) == MEM && SYMBOLIC_CONST (operands[1])
> - && flag_pic)
> - operands[1] = force_reg (Pmode, operands[1]);
> - else
> - {
> - rtx temp = (reload_in_progress ? operands[0]
> - : flag_pic? gen_reg_rtx (Pmode) : NULL_RTX);
> - operands[1] = arc_legitimize_pic_address (operands[1], temp);
> - }
> -}
> -
> -
> /* The function returning the number of words, at the beginning of an
> argument, must be put in registers. The returned value must be
> zero for arguments that are passed entirely in registers or that
> @@ -9072,30 +8967,38 @@ prepare_move_operands (rtx *operands, machine_mode
> mode)
> }
> }
>
> - if (mode == SImode && SYMBOLIC_CONST (operands[1]))
> + if (GET_CODE (operands[1]) == SYMBOL_REF)
> {
> - prepare_pic_move (operands, SImode);
> -
> - /* Disable any REG_EQUALs associated with the symref
> - otherwise the optimization pass undoes the work done
> - here and references the variable directly. */
> + enum tls_model model = SYMBOL_REF_TLS_MODEL (operands[1]);
> + if (MEM_P (operands[0]) && flag_pic)
> + operands[1] = force_reg (mode, operands[1]);
> + else if (model)
> + operands[1] = arc_legitimize_tls_address (operands[1], model);
> }
>
> + operands[1] = arc_legitimize_pic_address (operands[1]);
> +
> + /* Store instructions are limited, they only accept as address an
> + immediate, a register or a register plus a small immediate. */
> if (MEM_P (operands[0])
> - && !(reload_in_progress || reload_completed))
> + && !move_dest_operand (operands[0], mode))
> {
> - operands[1] = force_reg (mode, operands[1]);
> - if (!move_dest_operand (operands[0], mode))
> - {
> - rtx addr = copy_to_mode_reg (Pmode, XEXP (operands[0], 0));
> - /* This is like change_address_1 (operands[0], mode, 0, 1) ,
> - except that we can't use that function because it is static. */
> - rtx pat = change_address (operands[0], mode, addr);
> - MEM_COPY_ATTRIBUTES (pat, operands[0]);
> - operands[0] = pat;
> - }
> + rtx tmp0 = copy_to_mode_reg (Pmode, XEXP (operands[0], 0));
> + rtx tmp1 = change_address (operands[0], mode, tmp0);
> + MEM_COPY_ATTRIBUTES (tmp1, operands[0]);
> + operands[0] = tmp1;
> }
>
> + /* Check if it is constant but it is not legitimized. */
> + if (CONSTANT_P (operands[1])
> + && !arc_legitimate_constant_p (mode, operands[1]))
> + operands[1] = force_reg (mode, XEXP (operands[1], 0));
> + else if (MEM_P (operands[0])
> + && ((CONSTANT_P (operands[1])
> + && !satisfies_constraint_Cm3 (operands[1]))
> + || MEM_P (operands[1])))
> + operands[1] = force_reg (mode, operands[1]);
> +
> return false;
> }
>
> @@ -9566,11 +9469,10 @@ arc_legitimize_address_0 (rtx x, rtx oldx
> ATTRIBUTE_UNUSED,
> {
> rtx addr, inner;
>
> - if (flag_pic && SYMBOLIC_CONST (x))
> - (x) = arc_legitimize_pic_address (x, 0);
> addr = x;
> if (GET_CODE (addr) == CONST)
> addr = XEXP (addr, 0);
> +
> if (GET_CODE (addr) == PLUS
> && CONST_INT_P (XEXP (addr, 1))
> && ((GET_CODE (XEXP (addr, 0)) == SYMBOL_REF
> @@ -9601,13 +9503,6 @@ arc_legitimize_address_0 (rtx x, rtx oldx
> ATTRIBUTE_UNUSED,
> static rtx
> arc_legitimize_address (rtx orig_x, rtx oldx, machine_mode mode)
> {
> - if (GET_CODE (orig_x) == SYMBOL_REF)
> - {
> - enum tls_model model = SYMBOL_REF_TLS_MODEL (orig_x);
> - if (model != 0)
> - return arc_legitimize_tls_address (orig_x, model);
> - }
> -
> rtx new_x = arc_legitimize_address_0 (orig_x, oldx, mode);
>
> if (new_x)
> diff --git a/gcc/config/arc/arc.md b/gcc/config/arc/arc.md
> index 082b5bba6ec..ed29aa3d06e 100644
> --- a/gcc/config/arc/arc.md
> +++ b/gcc/config/arc/arc.md
> @@ -672,7 +672,9 @@ core_3, archs4x, archs4xd, archs4xd_slow"
> [(set (match_operand:QI 0 "move_dest_operand" "=Rcq,Rcq#q, w,Rcq#q,
> h, w, w,???w,h, w,Rcq, S,!*x, r,r, Ucm,m,???m, m,Usc")
> (match_operand:QI 1 "move_src_operand" " cL, cP,Rcq#q,
> P,hCm1,cL, I,?Rac,i,?i, T,Rcq,Usd,Ucm,m,?Rac,c,?Rac,Cm3,i"))]
> "register_operand (operands[0], QImode)
> - || register_operand (operands[1], QImode)"
> + || register_operand (operands[1], QImode)
> + || (satisfies_constraint_Cm3 (operands[1])
> + && memory_operand (operands[0], QImode))"
> "@
> mov%? %0,%1%&
> mov%? %0,%1%&
> @@ -714,7 +716,9 @@ core_3, archs4x, archs4xd, archs4xd_slow"
> /* Don't use a LIMM that we could load with a single insn - we loose
> delay-slot filling opportunities. */
> && !satisfies_constraint_I (operands[1])
> - && satisfies_constraint_Usc (operands[0]))"
> + && satisfies_constraint_Usc (operands[0]))
> + || (satisfies_constraint_Cm3 (operands[1])
> + && memory_operand (operands[0], HImode))"
> "@
> mov%? %0,%1%&
> mov%? %0,%1%&
> diff --git a/gcc/testsuite/gcc.target/arc/pr89838.c
> b/gcc/testsuite/gcc.target/arc/pr89838.c
> new file mode 100644
> index 00000000000..559434ac87e
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/arc/pr89838.c
> @@ -0,0 +1,16 @@
> +/* { dg-do compile } */
> +/* { dg-require-effective-target tls } */
> +/* { dg-options "-O2" } */
> +
> +extern void foo (void);
> +extern void bar (void *);
> +
> +struct {
> + int __attribute__(()) a;
> + int __attribute__(()) b;
> +} __thread c __attribute__((tls_model("initial-exec")));
> +
> +void foo (void)
> +{
> + bar (&c.b);
> +}
> --
> 2.21.0
>
