| Message ID | c3e17c19-0cd4-9d07-bcfc-0312487f029a@suse.com |
|---|---|
| State | New |
| Headers | show |
| Series | x86: vec_extract_* adjustments | expand |
On Wed, Jul 5, 2023 at 4:01 PM Jan Beulich via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > > V2TImode values cannot appear in the upper 16 YMM registers without > AVX512VL being enabled. Therefore forcing 512-bit mode (also not > reflected in the "mode" attribute) is pointless. Please set isa attribute for alternative 1 to avx512vl. > > gcc/ > > * config/i386/sse.md (*vec_extractv2ti): Drop g modifiers. > > --- a/gcc/config/i386/sse.md > +++ b/gcc/config/i386/sse.md > @@ -20115,7 +20115,7 @@ > "TARGET_AVX" > "@ > vextract%~128\t{%2, %1, %0|%0, %1, %2} > - vextracti32x4\t{%2, %g1, %0|%0, %g1, %2}" > + vextracti32x4\t{%2, %1, %0|%0, %1, %2}" > [(set_attr "type" "sselog") > (set_attr "prefix_extra" "1") > (set_attr "length_immediate" "1") >
On 05.07.2023 10:47, Hongtao Liu wrote: > On Wed, Jul 5, 2023 at 4:01 PM Jan Beulich via Gcc-patches > <gcc-patches@gcc.gnu.org> wrote: >> >> V2TImode values cannot appear in the upper 16 YMM registers without >> AVX512VL being enabled. Therefore forcing 512-bit mode (also not >> reflected in the "mode" attribute) is pointless. > Please set isa attribute for alternative 1 to avx512vl. Since that looks redundant to me (as per the description), would you mind explaining why that's necessary / wanted? It also feels orthogonal to the change I'm making, as there was no "isa" attribute so far (which would have wanted to be "avx512f" as per what you ask for, prior to the change I'm making). Again me asking back is primarily to properly describe the changes I'm making, of course along with me still needing to properly understand when what attribute needs specifying explicitly. Jan
On Wed, Jul 5, 2023 at 5:03 PM Jan Beulich <jbeulich@suse.com> wrote: > > On 05.07.2023 10:47, Hongtao Liu wrote: > > On Wed, Jul 5, 2023 at 4:01 PM Jan Beulich via Gcc-patches > > <gcc-patches@gcc.gnu.org> wrote: > >> > >> V2TImode values cannot appear in the upper 16 YMM registers without > >> AVX512VL being enabled. Therefore forcing 512-bit mode (also not > >> reflected in the "mode" attribute) is pointless. > > Please set isa attribute for alternative 1 to avx512vl. > > Since that looks redundant to me (as per the description), would you > mind explaining why that's necessary / wanted? It also feels orthogonal > to the change I'm making, as there was no "isa" attribute so far (which > would have wanted to be "avx512f" as per what you ask for, prior to the > change I'm making). Again me asking back is primarily to properly > describe the changes I'm making, of course along with me still needing > to properly understand when what attribute needs specifying explicitly. I checked ix86_hard_regno_ok, TImode/V2TImode will be allocated with evex sse register only under TARGET_AVX512VL. otherwise alternative 0 is matched. So yes, no need to set isa attribute here, patch LGTM. > > Jan -- BR, Hongtao
On Wed, Jul 5, 2023 at 6:22 PM Hongtao Liu <crazylht@gmail.com> wrote: > > On Wed, Jul 5, 2023 at 5:03 PM Jan Beulich <jbeulich@suse.com> wrote: > > > > On 05.07.2023 10:47, Hongtao Liu wrote: > > > On Wed, Jul 5, 2023 at 4:01 PM Jan Beulich via Gcc-patches > > > <gcc-patches@gcc.gnu.org> wrote: > > >> > > >> V2TImode values cannot appear in the upper 16 YMM registers without > > >> AVX512VL being enabled. Therefore forcing 512-bit mode (also not > > >> reflected in the "mode" attribute) is pointless. > > > Please set isa attribute for alternative 1 to avx512vl. > > > > Since that looks redundant to me (as per the description), would you > > mind explaining why that's necessary / wanted? It also feels orthogonal > > to the change I'm making, as there was no "isa" attribute so far (which > > would have wanted to be "avx512f" as per what you ask for, prior to the > > change I'm making). Again me asking back is primarily to properly > > describe the changes I'm making, of course along with me still needing > > to properly understand when what attribute needs specifying explicitly. It's decided by many factors: instruction isa requirement, possible register allocation for the alternative, also how recog_memoized(constrain_operands) decide which_alternative. For *vec_extractv2ti the alternative is implicitly guarded by ix86_hard_regno_ok and no need for explicit isa attribute. > I checked ix86_hard_regno_ok, TImode/V2TImode will be allocated > with evex sse register only under TARGET_AVX512VL. otherwise > alternative 0 is matched. > So yes, no need to set isa attribute here, patch LGTM. > > > > Jan > > > > > -- > BR, > Hongtao
--- a/gcc/config/i386/sse.md +++ b/gcc/config/i386/sse.md @@ -20115,7 +20115,7 @@ "TARGET_AVX" "@ vextract%~128\t{%2, %1, %0|%0, %1, %2} - vextracti32x4\t{%2, %g1, %0|%0, %g1, %2}" + vextracti32x4\t{%2, %1, %0|%0, %1, %2}" [(set_attr "type" "sselog") (set_attr "prefix_extra" "1") (set_attr "length_immediate" "1")