| Message ID | 20240730230325.273019-1-patrick@rivosinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | RISC-V: Add deprecation warning to LP64E abi | expand |
On Tue, Jul 30, 2024 at 4:04 PM Patrick O'Neill <patrick@rivosinc.com> wrote: > > gcc/ChangeLog: > > PR 116152 > * config/riscv/riscv.cc (riscv_option_override): Add deprecation > warning. > > gcc/testsuite/ChangeLog: > > * gcc.target/riscv/predef-9.c: Add check for warning. > > Signed-off-by: Patrick O'Neill <patrick@rivosinc.com> > --- > Tested prior to adding link. Relying on precommit for full testing. > --- > gcc/config/riscv/riscv.cc | 3 +++ > gcc/testsuite/gcc.target/riscv/predef-9.c | 1 + > 2 files changed, 4 insertions(+) > > diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc > index 8ece7859945..2d32427bef1 100644 > --- a/gcc/config/riscv/riscv.cc > +++ b/gcc/config/riscv/riscv.cc > @@ -9818,6 +9818,9 @@ riscv_option_override (void) > error ("rv64e requires lp64e ABI"); > } > > + if (riscv_abi == ABI_LP64E) > + warning (OPT_Wdeprecated, "LP64E ABI is marked for deprecation in GCC. If you would like LP64E to remain supported in future versions of GCC Please notify upstream at https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116152"); This is way past the 72/80 character normal limit. Please split the warning message into different lines. Or better yet do: ``` if (riscv_abi == ABI_LP64E) { if (warning (OPT_Wdeprecated, "LP64E ABI is marked for deprecation in GCC")) note ("If you need LP64E to please notify GCC project via https://gcc.gnu.org/PR116152"); } ``` Or something to that effect, that is use the note function. The if around the warning function is to have the note only emit if the warning is emitted. Thanks, Andrew Pinski > + > /* Zfinx require abi ilp32, ilp32e, lp64 or lp64e. */ > if (TARGET_ZFINX > && riscv_abi != ABI_ILP32 && riscv_abi != ABI_LP64 > diff --git a/gcc/testsuite/gcc.target/riscv/predef-9.c b/gcc/testsuite/gcc.target/riscv/predef-9.c > index cc3abc9a741..bf764947181 100644 > --- a/gcc/testsuite/gcc.target/riscv/predef-9.c > +++ b/gcc/testsuite/gcc.target/riscv/predef-9.c > @@ -1,5 +1,6 @@ > /* { dg-do compile } */ > /* { dg-options "-march=rv64em -mabi=lp64e -mno-div -mcmodel=medlow" } */ > +/* { dg-warning "LP64E ABI is marked for deprecation in GCC. If you would like LP64E to remain supported in future versions of GCC Please notify upstream at https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116152" "" { target *-*-* } 0 } */ > > int main () { > #if !defined(__riscv) > -- > 2.34.1 >
On 7/30/24 16:08, Andrew Pinski wrote: > On Tue, Jul 30, 2024 at 4:04 PM Patrick O'Neill <patrick@rivosinc.com> wrote: >> gcc/ChangeLog: >> >> PR 116152 >> * config/riscv/riscv.cc (riscv_option_override): Add deprecation >> warning. >> >> gcc/testsuite/ChangeLog: >> >> * gcc.target/riscv/predef-9.c: Add check for warning. >> >> Signed-off-by: Patrick O'Neill <patrick@rivosinc.com> >> --- >> Tested prior to adding link. Relying on precommit for full testing. >> --- >> gcc/config/riscv/riscv.cc | 3 +++ >> gcc/testsuite/gcc.target/riscv/predef-9.c | 1 + >> 2 files changed, 4 insertions(+) >> >> diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc >> index 8ece7859945..2d32427bef1 100644 >> --- a/gcc/config/riscv/riscv.cc >> +++ b/gcc/config/riscv/riscv.cc >> @@ -9818,6 +9818,9 @@ riscv_option_override (void) >> error ("rv64e requires lp64e ABI"); >> } >> >> + if (riscv_abi == ABI_LP64E) >> + warning (OPT_Wdeprecated, "LP64E ABI is marked for deprecation in GCC. If you would like LP64E to remain supported in future versions of GCC Please notify upstream at https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116152"); > > This is way past the 72/80 character normal limit. Please split the > warning message into different lines. > Or better yet do: > ``` > if (riscv_abi == ABI_LP64E) > { > if (warning (OPT_Wdeprecated, "LP64E ABI is marked for > deprecation in GCC")) > note ("If you need LP64E to please notify GCC project via > https://gcc.gnu.org/PR116152"); > } > ``` > > Or something to that effect, that is use the note function. The if > around the warning function is to have the note only emit if the > warning is emitted. Thanks for the review, that looks much better. I'll send a v2 using inform () and the much shorter link. Patrick
diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc index 8ece7859945..2d32427bef1 100644 --- a/gcc/config/riscv/riscv.cc +++ b/gcc/config/riscv/riscv.cc @@ -9818,6 +9818,9 @@ riscv_option_override (void) error ("rv64e requires lp64e ABI"); } + if (riscv_abi == ABI_LP64E) + warning (OPT_Wdeprecated, "LP64E ABI is marked for deprecation in GCC. If you would like LP64E to remain supported in future versions of GCC Please notify upstream at https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116152"); + /* Zfinx require abi ilp32, ilp32e, lp64 or lp64e. */ if (TARGET_ZFINX && riscv_abi != ABI_ILP32 && riscv_abi != ABI_LP64 diff --git a/gcc/testsuite/gcc.target/riscv/predef-9.c b/gcc/testsuite/gcc.target/riscv/predef-9.c index cc3abc9a741..bf764947181 100644 --- a/gcc/testsuite/gcc.target/riscv/predef-9.c +++ b/gcc/testsuite/gcc.target/riscv/predef-9.c @@ -1,5 +1,6 @@ /* { dg-do compile } */ /* { dg-options "-march=rv64em -mabi=lp64e -mno-div -mcmodel=medlow" } */ +/* { dg-warning "LP64E ABI is marked for deprecation in GCC. If you would like LP64E to remain supported in future versions of GCC Please notify upstream at https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116152" "" { target *-*-* } 0 } */ int main () { #if !defined(__riscv)
gcc/ChangeLog: PR 116152 * config/riscv/riscv.cc (riscv_option_override): Add deprecation warning. gcc/testsuite/ChangeLog: * gcc.target/riscv/predef-9.c: Add check for warning. Signed-off-by: Patrick O'Neill <patrick@rivosinc.com> --- Tested prior to adding link. Relying on precommit for full testing. --- gcc/config/riscv/riscv.cc | 3 +++ gcc/testsuite/gcc.target/riscv/predef-9.c | 1 + 2 files changed, 4 insertions(+) -- 2.34.1