On Sat, Mar 13, 2021 at 8:39 PM H.J. Lu <hjl.to...@gmail.com> wrote:
>
> On Fri, Mar 12, 2021 at 8:37 AM Uros Bizjak <ubiz...@gmail.com> wrote:
> >
> > On Fri, Mar 12, 2021 at 2:20 PM H.J. Lu <hjl.to...@gmail.com> wrote:
> > >
> > > On Thu, Mar 11, 2021 at 11:21 PM Uros Bizjak <ubiz...@gmail.com> wrote:
> > > >
> > > > On Thu, Mar 11, 2021 at 11:22 PM H.J. Lu <hjl.to...@gmail.com> wrote:
> > > > >
> > > > > Update 'P' operand modifier for -fno-plt to support inline assembly
> > > > > statements. In 64-bit, we can always load function address with
> > > > > @GOTPCREL. In 32-bit, we load function address with @GOT only for
> > > > > non-PIC since PIC register may not be available at call site.
> > > > >
> > > > > gcc/
> > > > >
> > > > > PR target/99504
> > > > > * config/i386/i386.c (ix86_print_operand): Update 'P' handling
> > > > > for -fno-plt.
> > > > >
> > > > > gcc/testsuite/
> > > > >
> > > > > PR target/99504
> > > > > * gcc.target/i386/pr99530-1.c: New test.
> > > > > * gcc.target/i386/pr99530-2.c: Likewise.
> > > > > * gcc.target/i386/pr99530-3.c: Likewise.
> > > > > * gcc.target/i386/pr99530-4.c: Likewise.
> > > > > * gcc.target/i386/pr99530-5.c: Likewise.
> > > > > * gcc.target/i386/pr99530-6.c: Likewise.
> > > > > ---
> > > > > gcc/config/i386/i386.c | 33
> > > > > +++++++++++++++++++++--
> > > > > gcc/testsuite/gcc.target/i386/pr99530-1.c | 11 ++++++++
> > > > > gcc/testsuite/gcc.target/i386/pr99530-2.c | 11 ++++++++
> > > > > gcc/testsuite/gcc.target/i386/pr99530-3.c | 11 ++++++++
> > > > > gcc/testsuite/gcc.target/i386/pr99530-4.c | 11 ++++++++
> > > > > gcc/testsuite/gcc.target/i386/pr99530-5.c | 11 ++++++++
> > > > > gcc/testsuite/gcc.target/i386/pr99530-6.c | 11 ++++++++
> > > > > 7 files changed, 97 insertions(+), 2 deletions(-)
> > > > > create mode 100644 gcc/testsuite/gcc.target/i386/pr99530-1.c
> > > > > create mode 100644 gcc/testsuite/gcc.target/i386/pr99530-2.c
> > > > > create mode 100644 gcc/testsuite/gcc.target/i386/pr99530-3.c
> > > > > create mode 100644 gcc/testsuite/gcc.target/i386/pr99530-4.c
> > > > > create mode 100644 gcc/testsuite/gcc.target/i386/pr99530-5.c
> > > > > create mode 100644 gcc/testsuite/gcc.target/i386/pr99530-6.c
> > > > >
> > > > > diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
> > > > > index 2603333f87b..8733fcecf65 100644
> > > > > --- a/gcc/config/i386/i386.c
> > > > > +++ b/gcc/config/i386/i386.c
> > > > > @@ -12701,7 +12701,8 @@ print_reg (rtx x, int code, FILE *file)
> > > > > y -- print "st(0)" instead of "st" as a register.
> > > > > d -- print duplicated register operand for AVX instruction.
> > > > > D -- print condition for SSE cmp instruction.
> > > > > - P -- if PIC, print an @PLT suffix.
> > > > > + P -- if PIC, print an @PLT suffix. For -fno-plt, load function
> > > > > + address from GOT.
> > > > > p -- print raw symbol name.
> > > > > X -- don't print any sort of PIC '@' suffix for a symbol.
> > > > > & -- print some in-use local-dynamic symbol name.
> > > > > @@ -13445,7 +13446,35 @@ ix86_print_operand (FILE *file, rtx x, int
> > > > > code)
> > > > > x = const0_rtx;
> > > > > }
> > > > >
> > > > > - if (code != 'P' && code != 'p')
> > > > > + if (code == 'P')
> > > > > + {
> > > > > + if (current_output_insn == NULL_RTX
> > > > > + && (TARGET_64BIT || (!flag_pic && HAVE_AS_IX86_GOT32X))
> > > > > + && !TARGET_PECOFF
> > > > > + && !TARGET_MACHO
> > > > > + && ix86_cmodel != CM_LARGE
> > > > > + && ix86_cmodel != CM_LARGE_PIC
> > > > > + && GET_CODE (x) == SYMBOL_REF
> > > > > + && SYMBOL_REF_FUNCTION_P (x)
> > > > > + && (!flag_plt
> > > > > + || (SYMBOL_REF_DECL (x)
> > > > > + && lookup_attribute ("noplt",
> > > > > + DECL_ATTRIBUTES
> > > > > (SYMBOL_REF_DECL (x)))))
> > > > > + && !SYMBOL_REF_LOCAL_P (x))
> > > > > + {
> > > > > + /* For inline assembly statement, load function address
> > > > > + from GOT with 'P' operand modifier to avoid PLT.
> > > > > + NB: This works only with call or jmp. */
> > > > > + const char *xasm;
> > > > > + if (TARGET_64BIT)
> > > > > + xasm = "{*%p0@GOTPCREL(%%rip)|[QWORD PTR
> > > > > %p0@GOTPCREL[rip]]}";
> > > > > + else
> > > > > + xasm = "{*%p0@GOT|[DWORD PTR %p0@GOT]}";
> > > > > + output_asm_insn (xasm, &x);
> > > > > + return;
> > > >
> > > > This should be handled in output_pic_addr_const.
> > > >
> > >
> > > call/jmp are special and are handled by ix86_output_call_insn,
> > > not output_pic_addr_const.
> >
> > I see, the call_insn is output using output_asm_insn, which I think is
> > not appropriate in ix86_print_operand. Probably you should introduce a
> > new helper function and output a GOTPCREL reloc there. Something like
> > x86_print_operand with 'A' code, calling output_addr_const and
> > appending @GOTPCREL. Perhaps some parts of ix86_print_opreands can be
> > used instead.
>
> Done. Here is the updated patch. Tested on Linux/x86-64. OK for master?
I don't understand the purpose of the current_output_insn check and I
don't know if the usage of current_output_insn is correct. The
comments are not helpful either, and no other target uses this
variable in the way you propose. Can you please elaborate the reason
and the purpose of the check a bit more?
Uros.
