On Fri, Jan 31, 2025 at 2:54 PM Uros Bizjak <ubiz...@gmail.com> wrote:
>
> On Fri, Jan 31, 2025 at 2:36 PM H.J. Lu <hjl.to...@gmail.com> wrote:
> >
> > -fno-plt forces external call to indirect call via GOT memory. But
> > -mindirect-branch-register requires indirect call and jump via register.
> > For -mindirect-branch-register, expanding indirect call via register and
> > update call patterns and peepholes to disable indirect call via memory.
> >
> > gcc/
> >
> > PR target/118713
> > * config/i386/i386-expand.cc (ix86_expand_call): Force indirect
> > call via register for -mindirect-branch-register.
> > * config/i386/i386.md (*call): Disable indirect call via memory
> > for -mindirect-branch-register.
> > (*call_got_x32): Likewise.
> > (*sibcall_GOT_32): Likewise.
> > (*sibcall): Likewise.
> > (*sibcall_memory): Likewise.
> > (*call_pop): Likewise.
> > (*sibcall_pop): Likewise.
> > (*sibcall_pop_memory): Likewise.
> > (*call_value): Likewise.
> > (*call_value_got_x32): Likewise.
> > (*sibcall_value_GOT_32): Likewise.
> > (*sibcall_value): Likewise.
> > (*sibcall_value_memory): Likewise.
> > (*call_value_pop): Likewise.
> > (*sibcall_value_pop): Likewise.
> > (*sibcall_value_pop_memory): Likewise.
> >
> > gcc/testsuite/
> >
> > PR target/118713
> > * gcc.target/i386/pr118713-1-x32.c: New test.
> > * gcc.target/i386/pr118713-1.c: Likewise.
> > * gcc.target/i386/pr118713-2-x32.c: Likewise.
> > * gcc.target/i386/pr118713-2.c: Likewise.
> > * gcc.target/i386/pr118713-3-x32.c: Likewise.
> > * gcc.target/i386/pr118713-3.c: Likewise.
> > * gcc.target/i386/pr118713-4-x32.c: Likewise.
> > * gcc.target/i386/pr118713-4.c: Likewise.
> > * gcc.target/i386/pr118713-5-x32.c: Likewise.
> > * gcc.target/i386/pr118713-5.c: Likewise.
> > * gcc.target/i386/pr118713-6-x32.c: Likewise.
> > * gcc.target/i386/pr118713-6.c: Likewise.
> > * gcc.target/i386/pr118713-7-x32.c: Likewise.
> > * gcc.target/i386/pr118713-7.c: Likewise.
> > * gcc.target/i386/pr118713-8-x32.c: Likewise.
> > * gcc.target/i386/pr118713-8.c: Likewise.
> > * gcc.target/i386/pr118713-9-x32.c: Likewise.
> > * gcc.target/i386/pr118713-9.c: Likewise.
> > * gcc.target/i386/pr118713-10-x32.c: Likewise.
> > * gcc.target/i386/pr118713-10.c: Likewise.
> > * gcc.target/i386/pr118713-11-x32.c: Likewise.
> > * gcc.target/i386/pr118713-11.c: Likewise.
> > * gcc.target/i386/pr118713-12-x32.c: Likewise.
> > * gcc.target/i386/pr118713-12.c: Likewise.
> >
> > Co-Authored-By: Uros Bizjak <ubiz...@gmail.com>
> > Signed-off-by: H.J. Lu <hjl.to...@gmail.com>
> > ---
> > gcc/config/i386/i386-expand.cc | 20 ++--
> > gcc/config/i386/i386.md | 98 +++++++++++++------
> > .../gcc.target/i386/pr118713-1-x32.c | 8 ++
> > gcc/testsuite/gcc.target/i386/pr118713-1.c | 14 +++
> > .../gcc.target/i386/pr118713-10-x32.c | 8 ++
> > gcc/testsuite/gcc.target/i386/pr118713-10.c | 15 +++
> > .../gcc.target/i386/pr118713-11-x32.c | 8 ++
> > gcc/testsuite/gcc.target/i386/pr118713-11.c | 14 +++
> > .../gcc.target/i386/pr118713-12-x32.c | 8 ++
> > gcc/testsuite/gcc.target/i386/pr118713-12.c | 14 +++
> > .../gcc.target/i386/pr118713-2-x32.c | 8 ++
> > gcc/testsuite/gcc.target/i386/pr118713-2.c | 15 +++
> > .../gcc.target/i386/pr118713-3-x32.c | 8 ++
> > gcc/testsuite/gcc.target/i386/pr118713-3.c | 14 +++
> > .../gcc.target/i386/pr118713-4-x32.c | 8 ++
> > gcc/testsuite/gcc.target/i386/pr118713-4.c | 14 +++
> > .../gcc.target/i386/pr118713-5-x32.c | 8 ++
> > gcc/testsuite/gcc.target/i386/pr118713-5.c | 13 +++
> > .../gcc.target/i386/pr118713-6-x32.c | 15 +++
> > gcc/testsuite/gcc.target/i386/pr118713-6.c | 14 +++
> > .../gcc.target/i386/pr118713-7-x32.c | 8 ++
> > gcc/testsuite/gcc.target/i386/pr118713-7.c | 13 +++
> > .../gcc.target/i386/pr118713-8-x32.c | 8 ++
> > gcc/testsuite/gcc.target/i386/pr118713-8.c | 13 +++
> > .../gcc.target/i386/pr118713-9-x32.c | 8 ++
> > gcc/testsuite/gcc.target/i386/pr118713-9.c | 14 +++
> > 26 files changed, 353 insertions(+), 35 deletions(-)
> > create mode 100644 gcc/testsuite/gcc.target/i386/pr118713-1-x32.c
> > create mode 100644 gcc/testsuite/gcc.target/i386/pr118713-1.c
> > create mode 100644 gcc/testsuite/gcc.target/i386/pr118713-10-x32.c
> > create mode 100644 gcc/testsuite/gcc.target/i386/pr118713-10.c
> > create mode 100644 gcc/testsuite/gcc.target/i386/pr118713-11-x32.c
> > create mode 100644 gcc/testsuite/gcc.target/i386/pr118713-11.c
> > create mode 100644 gcc/testsuite/gcc.target/i386/pr118713-12-x32.c
> > create mode 100644 gcc/testsuite/gcc.target/i386/pr118713-12.c
> > create mode 100644 gcc/testsuite/gcc.target/i386/pr118713-2-x32.c
> > create mode 100644 gcc/testsuite/gcc.target/i386/pr118713-2.c
> > create mode 100644 gcc/testsuite/gcc.target/i386/pr118713-3-x32.c
> > create mode 100644 gcc/testsuite/gcc.target/i386/pr118713-3.c
> > create mode 100644 gcc/testsuite/gcc.target/i386/pr118713-4-x32.c
> > create mode 100644 gcc/testsuite/gcc.target/i386/pr118713-4.c
> > create mode 100644 gcc/testsuite/gcc.target/i386/pr118713-5-x32.c
> > create mode 100644 gcc/testsuite/gcc.target/i386/pr118713-5.c
> > create mode 100644 gcc/testsuite/gcc.target/i386/pr118713-6-x32.c
> > create mode 100644 gcc/testsuite/gcc.target/i386/pr118713-6.c
> > create mode 100644 gcc/testsuite/gcc.target/i386/pr118713-7-x32.c
> > create mode 100644 gcc/testsuite/gcc.target/i386/pr118713-7.c
> > create mode 100644 gcc/testsuite/gcc.target/i386/pr118713-8-x32.c
> > create mode 100644 gcc/testsuite/gcc.target/i386/pr118713-8.c
> > create mode 100644 gcc/testsuite/gcc.target/i386/pr118713-9-x32.c
> > create mode 100644 gcc/testsuite/gcc.target/i386/pr118713-9.c
> >
> > diff --git a/gcc/config/i386/i386-expand.cc b/gcc/config/i386/i386-expand.cc
> > index 117f6f6f7eb..1d1614d2ccc 100644
> > --- a/gcc/config/i386/i386-expand.cc
> > +++ b/gcc/config/i386/i386-expand.cc
> > @@ -10223,15 +10223,21 @@ ix86_expand_call (rtx retval, rtx fnaddr, rtx
> > callarg1,
> > && GET_CODE (XEXP (fnaddr, 0)) == SYMBOL_REF
> > && !local_symbolic_operand (XEXP (fnaddr, 0), VOIDmode))
> > fnaddr = gen_rtx_MEM (QImode, construct_plt_address (XEXP (fnaddr,
> > 0)));
> > + else if (TARGET_INDIRECT_BRANCH_REGISTER && MEM_P (fnaddr))
> > + {
> > + fnaddr = convert_to_mode (word_mode, XEXP (fnaddr, 0), 1);
> > + fnaddr = gen_rtx_MEM (QImode, copy_to_mode_reg (word_mode, fnaddr));
> > + }
> > /* Since x32 GOT slot is 64 bit with zero upper 32 bits, indirect
> > branch via x32 GOT slot is OK. */
> > - else if (!(TARGET_X32
> > - && MEM_P (fnaddr)
> > - && GET_CODE (XEXP (fnaddr, 0)) == ZERO_EXTEND
> > - && GOT_memory_operand (XEXP (XEXP (fnaddr, 0), 0), Pmode))
> > - && (sibcall
> > - ? !sibcall_insn_operand (XEXP (fnaddr, 0), word_mode)
> > - : !call_insn_operand (XEXP (fnaddr, 0), word_mode)))
> > + if (TARGET_X32
> > + && MEM_P (fnaddr)
> > + && GET_CODE (XEXP (fnaddr, 0)) == ZERO_EXTEND
> > + && GOT_memory_operand (XEXP (XEXP (fnaddr, 0), 0), Pmode))
> > + ;
> > + else if (sibcall
> > + ? !sibcall_insn_operand (XEXP (fnaddr, 0), word_mode)
> > + : !call_insn_operand (XEXP (fnaddr, 0), word_mode))
> > {
> > fnaddr = convert_to_mode (word_mode, XEXP (fnaddr, 0), 1);
> > fnaddr = gen_rtx_MEM (QImode, copy_to_mode_reg (word_mode, fnaddr));
> > diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
> > index 52c02b6351a..0c9721bdc7a 100644
> > --- a/gcc/config/i386/i386.md
> > +++ b/gcc/config/i386/i386.md
> > @@ -20122,18 +20122,23 @@ (define_expand "sibcall"
> > })
> >
> > (define_insn "*call"
> > - [(call (mem:QI (match_operand:W 0 "call_insn_operand" "<c>BwBz"))
> > + [(call (mem:QI (match_operand:W 0 "call_insn_operand" "<c>,BwBz"))
> > (match_operand 1))]
> > "!SIBLING_CALL_P (insn)"
> > "* return ix86_output_call_insn (insn, operands[0]);"
> > - [(set_attr "type" "call")])
> > + [(set_attr "type" "call")
> > + (set (attr "enabled")
> > + (if_then_else
> > + (eq_attr "alternative" "1")
> > + (symbol_ref "!TARGET_INDIRECT_BRANCH_REGISTER")
> > + (const_string "*")))])
>
> Did you read my previous message? You don't need to add "enabled"
> attribute (and alternatives), because Bw and Bs already include:
>
> (define_constraint "Bs"
> "@internal Sibcall memory operand."
> (ior (and (not (match_test "TARGET_INDIRECT_BRANCH_REGISTER"))
> (not (match_test "TARGET_X32"))
> (match_operand 0 "sibcall_memory_operand"))
> (and (match_test "TARGET_X32")
> (match_test "Pmode == DImode")
> (match_operand 0 "GOT_memory_operand"))))
>
> (define_constraint "Bw"
> "@internal Call memory operand."
> (ior (and (not (match_test "TARGET_INDIRECT_BRANCH_REGISTER"))
> (not (match_test "TARGET_X32"))
> (match_operand 0 "memory_operand"))
> (and (match_test "TARGET_X32")
> (match_test "Pmode == DImode")
> (match_operand 0 "GOT_memory_operand"))))
>
> "Bz" constraint does not mention TARGET_INDIRECT_BRANCH_REGISTER.
>
> So, please investigate the condition and eventually fix Bs, Bw and Bz
> constraints. There is no need to introduce and disable alternatives
> when corresponding constraints are disabled.
OTOH, you can use and disable alternatives, but then you don't need to
check TARGET_INDIRECT_BRANCH_REGISTER usage in "Bs" and "Bw"
constraint. The choice depends on the question if T_I_B_R fully
disables "Bs" and "Bw". If "Bz" is also invalid with
TARGET_INDIRECT_BRANCH_REGISTER, only then you should use alternatives
to disable "BwBz" resp "BsBz" alternatives in call patterns.
AFAICS, when T_I_B_R is active, it should fully disable "Bs" as well
as "Bw" constraint. As it is written now, it doesn't disable X32 GOT
memory operand, which is probably wrong.
Also, TLS patterns use the "Bz" constraint, so it should not be
disabled with T_I_B_R.
Uros.
