[llvm] [RISCV][GlobalISel] Legalize G_ADD, G_SUB, G_AND, G_OR, G_XOR on RISC-V Vector Extension (PR #71400)

[Jiahan Xie via llvm-commits]

jiahanxie353 wrote:

> > > Given different conditions
> > > > nxv1s64, nxv2s64, nxv4s64, nxv8s64 require hasVInstructionsI64 instead of hasVInstructions
> > > 
> > > 
> > > > nxv1s8, nxv1s16, nxv1s32 require Subtarget.getELen() == 64
> > > 
> > > 
> > > I have this literal implementation locally:
> > > ```c++
> > > getActionDefinitionsBuilder({G_ADD, G_SUB, G_AND, G_OR, G_XOR})
> > >         .legalFor({s32, sXLen})
> > >         .legalFor(ST.hasVInstructions()
> > >                       ? std::initializer_list<LLT>{nxv2s8, nxv4s8, nxv8s8,
> > >                                                    nxv16s8, nxv32s8, nxv64s8,
> > >                                                    nxv2s16, nxv4s16, nxv8s16,
> > >                                                    nxv16s16, nxv32s16, nxv2s32,
> > >                                                    nxv4s32, nxv8s32, nxv16s32}
> > >                       : std::initializer_list<LLT>())
> > >         .legalFor(
> > >             ST.hasVInstructionsI64()
> > >                 ? std::initializer_list<LLT>{nxv1s64, nxv2s64, nxv4s64, nxv8s64}
> > >                 : std::initializer_list<LLT>())
> > >         .legalFor(ST.getELen() == 64
> > >                       ? std::initializer_list<LLT>{nxv1s8, nxv1s16, nxv1s32}
> > >                       : std::initializer_list<LLT>())
> > >         .widenScalarToNextPow2(0)
> > >         .clampScalar(0, s32, sXLen);
> > > ```
> > > 
> > > 
> > >     
> > >       
> > >     
> > > 
> > >       
> > >     
> > > 
> > >     
> > >   
> > > Please point out any issue this code snippet has.
> > 
> > 
> > This LGTM.
> > > In addition, it has to have fragments of `AllVecTys`, so I was wondering, as per @michaelmaitland 's review:
> > > > In any case, I'd like to have AllVecTys exist so it can be reused by future patches.
> > > 
> > > 
> > > Shall we still be declaring `AllVecTys`? It's good to declare it because it's likely to be reused, what if future patches also need the aforementioned condition checking so it won't be reused as a whole, is it still worth it?
> > 
> > 
> > Since we are not using AllVecTys, I think we should not include this variable. The compiler would emit a warning for an unused variable, and we don't want that. I think you should make this change and bring this PR out of draft, marking it ready for review!
> 
> Something like this could work.
> 
> ```
> .legalFor([=, &ST](const LegalityQuery &Query) {
>   return ST.hasVInstruction() && typeInSet(0, AllVecTys)(Query) &&
>              (Query.Types[0].getScalarSize() != 64 || ST.hasVInstruction64()) &&
>              (Query.Types[0].getElementCount().getKnownMinValue() != 1 || ST.getELEN() == 64));
> }
> ```

Do you happen to know how does `Query` check type equality? Unfortunately, `typeInSet(0, AllVecTys)(Query)` returns false for all vector types even though they are declared and present inside `AllVecTys`.

https://github.com/llvm/llvm-project/pull/71400