| Message ID | 1f5922e9-ee1c-5409-b4e5-543ee7a374fb@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | rs6000: ICE in unrecognizable insn when using -mpower10 | expand |
On Thu, Jul 23, 2020 at 08:15:42PM -0500, Peter Bergner wrote: > We get an ICE when using -mpower10 and a -mcpu= value that is older > than power10. The -mpower10 option requires -mcpu=power10 or later. > The following patch enforces that. Hi! > + /* If the user explicitly uses -mpower10, ensure our ISA flags are > + compatible with it. */ > + if (TARGET_POWER10 > + && (rs6000_isa_flags_explicit & OPTION_MASK_POWER10) != 0 > + && (processor_target_table[cpu_index].target_enable > + & OPTION_MASK_POWER10) == 0) > + error ("%qs requires %qs", "-mpower10", "-mcpu=power10"); This still allows -mpower10 without corresponding -mcpu=. We should just remove this command like flag (but keep the internal flag); for power10 we can do that without any issues, it is new (some testcases will need fixing, but it is that: fixing). Segher
On 7/24/20 6:32 AM, Segher Boessenkool wrote: > On Thu, Jul 23, 2020 at 08:15:42PM -0500, Peter Bergner wrote: >> + /* If the user explicitly uses -mpower10, ensure our ISA flags are >> + compatible with it. */ >> + if (TARGET_POWER10 >> + && (rs6000_isa_flags_explicit & OPTION_MASK_POWER10) != 0 >> + && (processor_target_table[cpu_index].target_enable >> + & OPTION_MASK_POWER10) == 0) >> + error ("%qs requires %qs", "-mpower10", "-mcpu=power10"); > > This still allows -mpower10 without corresponding -mcpu=. We should > just remove this command like flag (but keep the internal flag); for > power10 we can do that without any issues, it is new (some testcases > will need fixing, but it is that: fixing). I think our gcc driver will always pass a -mcpu= option to the compiler. That said, I too would prefer not even having the option. However, I don't know how to remove the -mpower10 option but keep the flag. You had mentioned -mdirect-move as one that had bee removed, but you actually only get a warning if you try and use that option, not an error, so it hasn't actually been removed. Peter
On Fri, Jul 24, 2020 at 11:10:29AM -0500, Peter Bergner wrote: > On 7/24/20 6:32 AM, Segher Boessenkool wrote: > > On Thu, Jul 23, 2020 at 08:15:42PM -0500, Peter Bergner wrote: > >> + /* If the user explicitly uses -mpower10, ensure our ISA flags are > >> + compatible with it. */ > >> + if (TARGET_POWER10 > >> + && (rs6000_isa_flags_explicit & OPTION_MASK_POWER10) != 0 > >> + && (processor_target_table[cpu_index].target_enable > >> + & OPTION_MASK_POWER10) == 0) > >> + error ("%qs requires %qs", "-mpower10", "-mcpu=power10"); > > > > This still allows -mpower10 without corresponding -mcpu=. We should > > just remove this command like flag (but keep the internal flag); for > > power10 we can do that without any issues, it is new (some testcases > > will need fixing, but it is that: fixing). > > I think our gcc driver will always pass a -mcpu= option to the compiler. > That said, I too would prefer not even having the option. However, I > don't know how to remove the -mpower10 option but keep the flag. > You had mentioned -mdirect-move as one that had bee removed, but > you actually only get a warning if you try and use that option, not > an error, so it hasn't actually been removed. It *was* removed, until a change in the option infrastructure removed that feature :-( It used "Ignore", so that the command line option didn't do anything, but we still had the flag. It was mdirect-move Target Undocumented Mask(DIRECT_MOVE) Var(rs6000_isa_flags) Ignore Warn(%qs is deprecated) but after various changes that destroyed this functionality it was changed to mdirect-move Target Undocumented Mask(DIRECT_MOVE) Var(rs6000_isa_flags) WarnRemoved which seems to actually work, but is completely ununderstandable syntax. Eventually we want to not use command-line flags to be able to select this at all, but to do that we will need to change all our current logic that uses target_flags. Which isn't so bad, but it will take time. Not made easier by that we already *have* split off an rs6000_isa_flags variable. It's a mess :-( Segher
diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c index 6bea544d26a..96241a9d0a3 100644 --- a/gcc/config/rs6000/rs6000.c +++ b/gcc/config/rs6000/rs6000.c @@ -4094,6 +4094,14 @@ rs6000_option_override_internal (bool global_init_p) rs6000_isa_flags &= ~OPTION_MASK_FLOAT128_HW; } + /* If the user explicitly uses -mpower10, ensure our ISA flags are + compatible with it. */ + if (TARGET_POWER10 + && (rs6000_isa_flags_explicit & OPTION_MASK_POWER10) != 0 + && (processor_target_table[cpu_index].target_enable + & OPTION_MASK_POWER10) == 0) + error ("%qs requires %qs", "-mpower10", "-mcpu=power10"); + /* Enable -mprefixed by default on power10 systems. */ if (TARGET_POWER10 && (rs6000_isa_flags_explicit & OPTION_MASK_PREFIXED) == 0) rs6000_isa_flags |= OPTION_MASK_PREFIXED; diff --git a/gcc/testsuite/g++.target/powerpc/pr95907.C b/gcc/testsuite/g++.target/powerpc/pr95907.C new file mode 100644 index 00000000000..45d276f25a7 --- /dev/null +++ b/gcc/testsuite/g++.target/powerpc/pr95907.C @@ -0,0 +1,7 @@ +// PR target/95907 +// { dg-do compile } +// { dg-require-effective-target power10_ok } +// { dg-options "-mpower10" } + +int foobool_argc; +bool foobool() { return foobool_argc; }