On 19.06.2023 04:07, Liu, Hongtao wrote: >> -----Original Message----- >> From: Jan Beulich <jbeul...@suse.com> >> Sent: Friday, June 16, 2023 2:22 PM >> >> --- a/gcc/config/i386/sse.md >> +++ b/gcc/config/i386/sse.md >> @@ -12597,11 +12597,11 @@ >> (set_attr "mode" "<sseinsnmode>")]) >> >> (define_insn "*<avx512>_vternlog<mode>_all" >> - [(set (match_operand:V 0 "register_operand" "=v") >> + [(set (match_operand:V 0 "register_operand" "=v,v") >> (unspec:V >> - [(match_operand:V 1 "register_operand" "0") >> - (match_operand:V 2 "register_operand" "v") >> - (match_operand:V 3 "bcst_vector_operand" "vmBr") >> + [(match_operand:V 1 "register_operand" "0,0") >> + (match_operand:V 2 "register_operand" "v,v") >> + (match_operand:V 3 "bcst_vector_operand" "vBr,m") >> (match_operand:SI 4 "const_0_to_255_operand")] >> UNSPEC_VTERNLOG))] >> "TARGET_AVX512F > Change condition to <MODE_SIZE> == 64 || TARGET_AVX512VL || (TARGET_AVX512F > && !TARGET_PREFER_AVX256)
May I ask why you think this is necessary? The condition of the insn already wasn't in sync with the condition used in all three splitters, and I didn't see any reason why now they would need to be brought in sync. First and foremost because of the use of the UNSPEC (equally before and after this patch). Furthermore, isn't it the case that I'm already mostly expressing this with the "enabled" attribute? At the very least I think I should drop that again then if following your request? > Also please add a testcase for case TARGET_AVX512F && !TARGET_PREFER_AVX256. Especially in a case like this one I'm wondering about the usefulness of a contrived testcase: It won't test more than one minor sub-case of the whole set of constructs covered here. But well, here as well as for the other change I'll invent something. Jan
