"H.J. Lu" <hjl.to...@gmail.com> writes:
> On Wed, Jun 4, 2025 at 4:13 PM Richard Sandiford
> <richard.sandif...@arm.com> 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.
>>
>> Richard
>
> After r16-1041-g2da641d0170090, get_call_rtx_from returns
>
> (call (unspec:SI [
> (mem:SI (reg/f:SI 98) [0 MEM[(void (*<T2f6>) (struct
> test_st))209715
> 2B] S4 A32])
> ] UNSPEC_NONSECURE_MEM)
> (const_int 0 [0]))
>
> on
>
> (call_insn 14 13 0 (parallel [
> (call (unspec:SI [
> (mem:SI (reg/f:SI 98) [0 MEM[(void (*<T2f6>) (struct
> tes
> t_st))2097152B] S4 A32])
> ] UNSPEC_NONSECURE_MEM)
> (const_int 0 [0]))
> (use (const_int 0 [0]))
> (clobber (reg:SI 14 lr))
> ])
> "/export/gnu/import/git/sources/gcc/gcc/testsuite/gcc.target/arm/cmse
> /mainline/8_1m/../../bitfield-4.x":38:3 -1
> (nil)
> (nil))
>
> Update to check MEM_P before using MEM_EXPR in emit_call_1 and
> rest_of_insert_endbr_and_patchable_area.
>
> PR other/120493
> * calls.cc (emit_call_1): Use MEM_EXPR only if MEM_P is true.
> * config/i386/i386-features.cc
> (rest_of_insert_endbr_and_patchable_area): Likewise.
Not sure, but: did you mean to attach a patch?
But the call above also doesn't follow the documentation, in that
the operand of the call is an unspec rather than a mem. So if we
want to bless this kind of call, we should at least update the
documentation to say what is now allowed.
FWIW, if the intent is to use the unspec to attach on-the-side
information to the call, I think it'd better to do that in parallel
with the call, rather than as an operand to it. I suppose that's
what the:
(use (const_int 0 [0]))
is already doing for other information (and is also what AArch64 uses
to record the PCS of the target function).
Thanks,
Richard
