[PATCH] D112435: [GlobalISel] Add G_BFI (bitfield insertion opcode)

[Konstantin Schwarz via Phabricator via llvm-commits]

kschwarz added inline comments.


================
Comment at: llvm/lib/CodeGen/MachineVerifier.cpp:1616
+    LLT DstTy = MRI->getType(MI->getOperand(0).getReg());
+    if (DstTy.isVector()) {
+      report("Bitfield insertion is not supported on vectors", MI);
----------------
foad wrote:
> arsenm wrote:
> > foad wrote:
> > > arsenm wrote:
> > > > arsenm wrote:
> > > > > paquette wrote:
> > > > > > Maybe add a FIXME or TODO here noting that it may make sense to extend this to vectors in the future? (I can't think of any use-cases, but maybe it makes sense on some target...)
> > > > > Supporting on vectors is good for consistency and also can help with combine ordering problems
> > > > I don't think this should disallow vectors. Having the legalizer scalarize is trivial
> > > > 
> > > > I think it's a better question if the second/third operands can be vectors or not
> > > > I think it's a better question if the second/third operands can be vectors or not
> > > 
> > > What are the rules for basic shifts like G_SHL ? I don't see any special handling for them in MachineVerifier. Do we really allow any combination of scalars and vectors for the first and second inputs?
> > For basic shifts it's all operands vector or all scalar
> OK: D115868
Having the `width` and `pos` operands support vectors too allows different values per lane, which is probably the most generic form.
Are you aware of any ISA that has such instructions?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112435/new/

https://reviews.llvm.org/D112435