| Message ID | 4100446e-18fa-6f72-13a2-05905666a787@suse.com |
|---|---|
| State | New |
| Headers | show |
| Series | testsuite: adjust NOP expectations for RISC-V | expand |
On 4/25/23 08:50, Jan Beulich via Gcc-patches wrote: > RISC-V will emit ".option nopic" when -fno-pie is in effect, which > matches the generic pattern. Just like done for Alpha, special-case > RISC-V. > --- > A couple more targets look to be affected as well, simply because their > "no-operation" insn doesn't match the expectation. With the apparently > necessary further special casing I then also question the presence of > "SWYM" in the generic pattern. > > An alternative here might be to use dg-additional-options to add e.g. > -fpie. I don't think I know all possible implications of doing so, > though. I think this is fine. Go ahead and install it. jeff
On Wed, 26 Apr 2023 08:26:26 PDT (-0700), gcc-patches@gcc.gnu.org wrote: > > > On 4/25/23 08:50, Jan Beulich via Gcc-patches wrote: >> RISC-V will emit ".option nopic" when -fno-pie is in effect, which >> matches the generic pattern. Just like done for Alpha, special-case >> RISC-V. >> --- >> A couple more targets look to be affected as well, simply because their >> "no-operation" insn doesn't match the expectation. With the apparently >> necessary further special casing I then also question the presence of >> "SWYM" in the generic pattern. >> >> An alternative here might be to use dg-additional-options to add e.g. >> -fpie. I don't think I know all possible implications of doing so, >> though. Looks like there's already a no-pie for SPARC. Nothing's jumping out as to why, but I'm not super familiar with `-fpatchable-function-entry`. > I think this is fine. Go ahead and install it. We run into this sort of thing somewhat frequently. Maybe we want a DG matcher that avoids matching assembler directives? Or maybe even a "scan-assembler-nop-times" type thing, given that different ports have different names for the instruction? I don't see reason to block fixing the test on something bigger, though, so seems fine for trunk. Presumably we'd want to backport this as well? > > jeff
On 26.04.2023 17:45, Palmer Dabbelt wrote: > On Wed, 26 Apr 2023 08:26:26 PDT (-0700), gcc-patches@gcc.gnu.org wrote: >> >> >> On 4/25/23 08:50, Jan Beulich via Gcc-patches wrote: >>> RISC-V will emit ".option nopic" when -fno-pie is in effect, which >>> matches the generic pattern. Just like done for Alpha, special-case >>> RISC-V. >>> --- >>> A couple more targets look to be affected as well, simply because their >>> "no-operation" insn doesn't match the expectation. With the apparently >>> necessary further special casing I then also question the presence of >>> "SWYM" in the generic pattern. >>> >>> An alternative here might be to use dg-additional-options to add e.g. >>> -fpie. I don't think I know all possible implications of doing so, >>> though. > > Looks like there's already a no-pie for SPARC. Nothing's jumping out as > to why, but I'm not super familiar with `-fpatchable-function-entry`. > >> I think this is fine. Go ahead and install it. > > We run into this sort of thing somewhat frequently. Maybe we want a DG > matcher that avoids matching assembler directives? Or maybe even a > "scan-assembler-nop-times" type thing, given that different ports have > different names for the instruction? > > I don't see reason to block fixing the test on something bigger, though, > so seems fine for trunk. Presumably we'd want to backport this as well? Perhaps, but in order to do so I'd need to be given the respective okay. Jan
On 4/27/23 01:39, Jan Beulich via Gcc-patches wrote: > On 26.04.2023 17:45, Palmer Dabbelt wrote: >> On Wed, 26 Apr 2023 08:26:26 PDT (-0700), gcc-patches@gcc.gnu.org wrote: >>> >>> >>> On 4/25/23 08:50, Jan Beulich via Gcc-patches wrote: >>>> RISC-V will emit ".option nopic" when -fno-pie is in effect, which >>>> matches the generic pattern. Just like done for Alpha, special-case >>>> RISC-V. >>>> --- >>>> A couple more targets look to be affected as well, simply because their >>>> "no-operation" insn doesn't match the expectation. With the apparently >>>> necessary further special casing I then also question the presence of >>>> "SWYM" in the generic pattern. >>>> >>>> An alternative here might be to use dg-additional-options to add e.g. >>>> -fpie. I don't think I know all possible implications of doing so, >>>> though. >> >> Looks like there's already a no-pie for SPARC. Nothing's jumping out as >> to why, but I'm not super familiar with `-fpatchable-function-entry`. >> >>> I think this is fine. Go ahead and install it. >> >> We run into this sort of thing somewhat frequently. Maybe we want a DG >> matcher that avoids matching assembler directives? Or maybe even a >> "scan-assembler-nop-times" type thing, given that different ports have >> different names for the instruction? >> >> I don't see reason to block fixing the test on something bigger, though, >> so seems fine for trunk. Presumably we'd want to backport this as well? > > Perhaps, but in order to do so I'd need to be given the respective okay. Given how often we're trying to avoid matching directives, particularly directives which refer to filenames this sounds like a good idea to me. jeff
On Fri, 28 Apr 2023 08:20:24 PDT (-0700), jeffreyalaw@gmail.com wrote: > > > On 4/27/23 01:39, Jan Beulich via Gcc-patches wrote: >> On 26.04.2023 17:45, Palmer Dabbelt wrote: >>> On Wed, 26 Apr 2023 08:26:26 PDT (-0700), gcc-patches@gcc.gnu.org wrote: >>>> >>>> >>>> On 4/25/23 08:50, Jan Beulich via Gcc-patches wrote: >>>>> RISC-V will emit ".option nopic" when -fno-pie is in effect, which >>>>> matches the generic pattern. Just like done for Alpha, special-case >>>>> RISC-V. >>>>> --- >>>>> A couple more targets look to be affected as well, simply because their >>>>> "no-operation" insn doesn't match the expectation. With the apparently >>>>> necessary further special casing I then also question the presence of >>>>> "SWYM" in the generic pattern. >>>>> >>>>> An alternative here might be to use dg-additional-options to add e.g. >>>>> -fpie. I don't think I know all possible implications of doing so, >>>>> though. >>> >>> Looks like there's already a no-pie for SPARC. Nothing's jumping out as >>> to why, but I'm not super familiar with `-fpatchable-function-entry`. >>> >>>> I think this is fine. Go ahead and install it. >>> >>> We run into this sort of thing somewhat frequently. Maybe we want a DG >>> matcher that avoids matching assembler directives? Or maybe even a >>> "scan-assembler-nop-times" type thing, given that different ports have >>> different names for the instruction? >>> >>> I don't see reason to block fixing the test on something bigger, though, >>> so seems fine for trunk. Presumably we'd want to backport this as well? >> >> Perhaps, but in order to do so I'd need to be given the respective okay. > Given how often we're trying to avoid matching directives, particularly > directives which refer to filenames this sounds like a good idea to me. I think the ask there was for an OK to backport this fix to 13? So I guess more concretely: OK for trunk. OK to backport for 13?
On 4/28/23 10:43, Palmer Dabbelt wrote: > On Fri, 28 Apr 2023 08:20:24 PDT (-0700), jeffreyalaw@gmail.com wrote: >> >> >> On 4/27/23 01:39, Jan Beulich via Gcc-patches wrote: >>> On 26.04.2023 17:45, Palmer Dabbelt wrote: >>>> On Wed, 26 Apr 2023 08:26:26 PDT (-0700), gcc-patches@gcc.gnu.org >>>> wrote: >>>>> >>>>> >>>>> On 4/25/23 08:50, Jan Beulich via Gcc-patches wrote: >>>>>> RISC-V will emit ".option nopic" when -fno-pie is in effect, which >>>>>> matches the generic pattern. Just like done for Alpha, special-case >>>>>> RISC-V. >>>>>> --- >>>>>> A couple more targets look to be affected as well, simply because >>>>>> their >>>>>> "no-operation" insn doesn't match the expectation. With the >>>>>> apparently >>>>>> necessary further special casing I then also question the presence of >>>>>> "SWYM" in the generic pattern. >>>>>> >>>>>> An alternative here might be to use dg-additional-options to add e.g. >>>>>> -fpie. I don't think I know all possible implications of doing so, >>>>>> though. >>>> >>>> Looks like there's already a no-pie for SPARC. Nothing's jumping >>>> out as >>>> to why, but I'm not super familiar with `-fpatchable-function-entry`. >>>> >>>>> I think this is fine. Go ahead and install it. >>>> >>>> We run into this sort of thing somewhat frequently. Maybe we want a DG >>>> matcher that avoids matching assembler directives? Or maybe even a >>>> "scan-assembler-nop-times" type thing, given that different ports have >>>> different names for the instruction? >>>> >>>> I don't see reason to block fixing the test on something bigger, >>>> though, >>>> so seems fine for trunk. Presumably we'd want to backport this as >>>> well? >>> >>> Perhaps, but in order to do so I'd need to be given the respective okay. >> Given how often we're trying to avoid matching directives, particularly >> directives which refer to filenames this sounds like a good idea to me. > > I think the ask there was for an OK to backport this fix to 13? So I > guess more concretely: > > OK for trunk. OK to backport for 13? Sure, OK for backporting as well. jeff
--- a/gcc/testsuite/c-c++-common/patchable_function_entry-decl.c +++ b/gcc/testsuite/c-c++-common/patchable_function_entry-decl.c @@ -1,8 +1,9 @@ /* { dg-do compile { target { ! { nvptx*-*-* visium-*-* } } } } */ /* { dg-options "-O2 -fpatchable-function-entry=3,1" } */ /* { dg-additional-options "-fno-pie" { target sparc*-*-* } } */ -/* { dg-final { scan-assembler-times "nop|NOP|SWYM" 2 { target { ! { alpha*-*-* } } } } } */ +/* { dg-final { scan-assembler-times "nop|NOP|SWYM" 2 { target { ! { alpha*-*-* riscv*-*-* } } } } } */ /* { dg-final { scan-assembler-times "bis" 2 { target alpha*-*-* } } } */ +/* { dg-final { scan-assembler-times "nop\n" 2 { target riscv*-*-* } } } */ extern int a; --- a/gcc/testsuite/c-c++-common/patchable_function_entry-default.c +++ b/gcc/testsuite/c-c++-common/patchable_function_entry-default.c @@ -4,8 +4,9 @@ /* See PR99888, one single preceding nop isn't allowed on powerpc_elfv2, so overriding with two preceding nops to make it pass there. */ /* { dg-additional-options "-fpatchable-function-entry=3,2" { target powerpc_elfv2 } } */ -/* { dg-final { scan-assembler-times "nop|NOP|SWYM" 3 { target { ! { alpha*-*-* } } } } } */ +/* { dg-final { scan-assembler-times "nop|NOP|SWYM" 3 { target { ! { alpha*-*-* riscv*-*-* } } } } } */ /* { dg-final { scan-assembler-times "bis" 3 { target alpha*-*-* } } } */ +/* { dg-final { scan-assembler-times "nop\n" 3 { target riscv*-*-* } } } */ extern int a; --- a/gcc/testsuite/c-c++-common/patchable_function_entry-definition.c +++ b/gcc/testsuite/c-c++-common/patchable_function_entry-definition.c @@ -1,8 +1,9 @@ /* { dg-do compile { target { ! { nvptx*-*-* visium-*-* } } } } */ /* { dg-options "-O2 -fpatchable-function-entry=3,1" } */ /* { dg-additional-options "-fno-pie" { target sparc*-*-* } } */ -/* { dg-final { scan-assembler-times "nop|NOP|SWYM" 1 { target { ! { alpha*-*-* } } } } } */ +/* { dg-final { scan-assembler-times "nop|NOP|SWYM" 1 { target { ! { alpha*-*-* riscv*-*-* } } } } } */ /* { dg-final { scan-assembler-times "bis" 1 { target alpha*-*-* } } } */ +/* { dg-final { scan-assembler-times "nop\n" 1 { target riscv*-*-* } } } */ extern int a;