| Message ID | 1d76627b-6690-6da1-99b1-29aab0dbf860@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | [rs6000,PR,target/87474] fix strncmp expansion with -mno-power8-vector | expand |
On Mon, Oct 01, 2018 at 11:09:44PM -0500, Aaron Sawdey wrote: > PR/87474 happens because I didn't check that both vector and VSX instructions > were enabled, so insns that are disabled get generated with -mno-power8-vector. > PR target/87474 > * config/rs6000/rs6000-string.c (expand_strn_compare): Check that both > vector and VSX are enabled. You mean "P8 vector" or similar, I think? > --- gcc/config/rs6000/rs6000-string.c (revision 264760) > +++ gcc/config/rs6000/rs6000-string.c (working copy) > @@ -2205,6 +2205,7 @@ > } > else > { > + /* Implies TARGET_P8_VECTOR here. */ That isn't true as far as I see. Okay for trunk with improved changelog and that stray line removed. Thanks! Segher
On 10/2/18 3:38 AM, Segher Boessenkool wrote: > On Mon, Oct 01, 2018 at 11:09:44PM -0500, Aaron Sawdey wrote: >> PR/87474 happens because I didn't check that both vector and VSX instructions >> were enabled, so insns that are disabled get generated with -mno-power8-vector. > >> PR target/87474 >> * config/rs6000/rs6000-string.c (expand_strn_compare): Check that both >> vector and VSX are enabled. > > You mean "P8 vector" or similar, I think? True, it should say TARGET_P[89]_VECTOR. > > >> --- gcc/config/rs6000/rs6000-string.c (revision 264760) >> +++ gcc/config/rs6000/rs6000-string.c (working copy) >> @@ -2205,6 +2205,7 @@ >> } >> else >> { >> + /* Implies TARGET_P8_VECTOR here. */ > > That isn't true as far as I see. We can only enter emit_final_str_compare_vec() if TARGET_P8_VECTOR is set. That's the additional check this patch adds. So in this function you can have both P8 and P9 vector, or just p8 vector. rs6000_option_override_internal() enforces that P8 vector must be set if P9 vector is set. So in the else here we know that only p8 vector is set. > > > Okay for trunk with improved changelog and that stray line removed. > Thanks! > > > Segher >
On Tue, Oct 02, 2018 at 08:24:31AM -0500, Aaron Sawdey wrote: > >> @@ -2205,6 +2205,7 @@ > >> } > >> else > >> { > >> + /* Implies TARGET_P8_VECTOR here. */ > > > > That isn't true as far as I see. > > We can only enter emit_final_str_compare_vec() if TARGET_P8_VECTOR is set. > That's the additional check this patch adds. So in this function you can have > both P8 and P9 vector, or just p8 vector. rs6000_option_override_internal() > enforces that P8 vector must be set if P9 vector is set. So in the else here > we know that only p8 vector is set. It's not obvious. Add an assert, maybe? Instead of the comment :-) Segher
Index: gcc/config/rs6000/rs6000-string.c =================================================================== --- gcc/config/rs6000/rs6000-string.c (revision 264760) +++ gcc/config/rs6000/rs6000-string.c (working copy) @@ -2205,6 +2205,7 @@ } else { + /* Implies TARGET_P8_VECTOR here. */ rtx diffix = gen_reg_rtx (DImode); rtx result_gbbd = gen_reg_rtx (V16QImode); /* Since each byte of the input is either 00 or FF, the bytes in @@ -2313,9 +2314,12 @@ /* Is it OK to use vec/vsx for this. TARGET_VSX means we have at least POWER7 but we use TARGET_EFFICIENT_UNALIGNED_VSX which is at least POWER8. That way we can rely on overlapping compares to - do the final comparison of less than 16 bytes. Also I do not want - to deal with making this work for 32 bits. */ - int use_vec = (bytes >= 16 && !TARGET_32BIT && TARGET_EFFICIENT_UNALIGNED_VSX); + do the final comparison of less than 16 bytes. Also I do not + want to deal with making this work for 32 bits. In addition, we + have to make sure that we have at least P8_VECTOR (we don't allow + P9_VECTOR without P8_VECTOR). */ + int use_vec = (bytes >= 16 && !TARGET_32BIT + && TARGET_EFFICIENT_UNALIGNED_VSX && TARGET_P8_VECTOR); if (use_vec) required_align = 16;