Re: [PATCH] Use MEM_EXPR only if MEM_P is true

[Stefan Schulze Frielinghaus]

On Wed, Jun 04, 2025 at 09:12:58AM +0100, Richard Sandiford wrote:
> Richard Biener <richard.guent...@gmail.com> writes:
> > On Wed, Jun 4, 2025 at 7:28 AM H.J. Lu <hjl.to...@gmail.com> wrote:
> >>
> >> On s390x, for input:
> >>
> >> (call_insn/u 7 6 11 2 (parallel [
> >>             (set (reg:SI 2 %r2)
> >>                 (call (subreg:QI (symbol_ref:SI ("__tls_get_offset")
> >> [flags 0x1]) 3)
> >>                     (const_int 0 [0])))
> >>             (clobber (reg:SI 14 %r14))
> >>             (use (unspec:SI [
> >>                         (const_int 0 [0])
> >>                     ] UNSPEC_TLSLDM))
> >>         ]) "/tmp/foo.c":12:26 2602 {*brasl_tls}
> >>      (expr_list:REG_EH_REGION (const_int -2147483648 [0xffffffff80000000])
> >>         (nil))
> >>     (expr_list (use (reg:SI 2 %r2))
> >>         (expr_list (use (reg:SI 12 %r12))
> >>             (nil))))
> >>
> >> after r16-1041-g2da641d0170090, get_call_rtx_from returns:
> >>
> >> (call (subreg:QI (symbol_ref:SI ("__tls_get_offset") [flags 0x1]) 3)
> >>     (const_int 0 [0]))
> >
> > That's a strange call!
> 
> Yeah.  Are we sure it's really correct?  Taken literally, it says that
> we're interpreting the symbol __tls_get_offset as a sequence of instructions.
> 
> Also, the docs say:
> 
>   @item (call @var{function} @var{nargs})
>   Represents a function call.  @var{function} is a @code{mem} expression
>   whose address is the address of the function to be called.
> 
> So I don't object to the patch, but it seems like it might be papering
> over a target bug.  cc:ing s390 maintainers.

Thanks for the ping!  The subreg inside the call

(call (subreg:QI (symbol_ref:SI ("__tls_get_offset") [flags 0x1]) 3)
    (const_int 0 [0]))

stems from s390_delegitimize_address() where we have at the very end of
the function

  if (GET_MODE (orig_x) != Pmode)
    {
      if (GET_MODE (orig_x) == BLKmode)
        return orig_x;
      y = lowpart_subreg (GET_MODE (orig_x), y, Pmode);
      if (y == NULL_RTX)
        return orig_x;
    }

Building a subreg in case of

  else if (GET_CODE (x) == CONST)
    {
      /* Extract the symbol ref from:
         (mem:QI (const:DI (unspec:DI [(symbol_ref:DI ("foo"))]
                                       UNSPEC_PLT/GOTENT)))  */

      y = XEXP (x, 0);
      if (GET_CODE (y) == UNSPEC
          && (XINT (y, 1) == UNSPEC_GOTENT
              || XINT (y, 1) == UNSPEC_PLT31))
        y = XVECEXP (y, 0, 0);
      else
        return orig_x;
    }

is questionable to me.  Shouldn't we in this case if also
GET_MODE (orig_x) != Pmode holds, return the SYMBOL_REF wrapped in a MEM
with mode of orig_x instead?

Cheers,
Stefan