On Mon, Jun 16, 2025 at 4:28 PM Hongtao Liu <crazy...@gmail.com> wrote: > > On Mon, Jun 16, 2025 at 4:30 PM Hongtao Liu <crazy...@gmail.com> wrote: > > > > >+enum redundant_load_kind > > >+{ > > >+ LOAD_CONST0_VECTOR, > > >+ LOAD_CONSTM1_VECTOR, > > >+ LOAD_VECTOR > > >+}; > > Perhaps rename to x86_cse_kind, X86_CSE_CONST0_VECTOR, > > X86_CSE_CONSTM1_VECTOR, X86_CSE_VEC_DUP? > > LOAD sounds a bit ambiguous. > > Similar to ix86_get_vector_load_mode -> ix86_get_vector_cse_mode? > > > > >+ if (SUBREG_P (op)) > > >+ reg = SUBREG_REG (op); > > Need to make sure subreg is a lowpart of op, it can't be a paradoxical > > subreg. > > > > >+ /* Only single def chain is supported. */ > > >+ df_ref ref = DF_REG_DEF_CHAIN (REGNO (reg)); > > >+ if (!ref || DF_REF_NEXT_REG (ref) != nullptr) > > >+ return nullptr; > > > > Could we just reuse df_find_single_def_src and add extra code to get > > the insn and handle *insn_p = ***. > > Also I notice df_find_single_def_src exclude DF_REF_IS_ARTIFICIAL
I will exclude DF_REF_IS_ARTIFICIAL. Since df_find_single_def_src calls function_invariant_p, it doesn't work on extern __m512i sinkz; extern __m256i sinky; extern char f; void foo(char c, int x) { c += f; sinkz = _mm512_set1_epi8(c); if (x == 2) f += 3; sinky = _mm256_set1_epi8(c); } > > which is not in your original patch, after excluding it, do below > > codes still be needed? > > > I mean this part. > + && (load->bb == bb > + || kind < LOAD_VECTOR > + /* Non all 0s/1s vector load must be in the same > + basic block if it is in a recursive call. */ > + || !recursive_call_p) > > > > > >+ ix86_place_single_vector_set (load->broadcast_reg, > > >+ load->broadcast_source, > > >+ load->bbs, > > >+ (load->kind >= LOAD_VECTOR > > Just load->kind == LOAD_VECTOR? > > > > -- > BR, > Hongtao -- H.J.