[PATCH] D95426: [GlobalISel] Extract a narrowScalarAddSub method. NFC

[Cassie Jones via Phabricator via llvm-commits]

porglezomp added inline comments.


================
Comment at: llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp:4448
+
+  uint64_t SizeOp0 = MRI.getType(MI.getOperand(0).getReg()).getSizeInBits();
+  uint64_t NarrowSize = NarrowTy.getSizeInBits();
----------------
arsenm wrote:
> porglezomp wrote:
> > arsenm wrote:
> > > This is ignoring vectors
> > This is what the existing code did. Re-reading it more closely, I'm beginning to think that the existing code didn't work for vectors at all—it ends up scalarizing the adds, and then doing carries between the lanes?
> > 
> > Running the whole LLVM test suite (with at least AMDGPU, AArch64, and X86 enabled), nothing seems to ever use the vector code path.
> > 
> > I think the right course here might be to remove support for vectors here, and someone can add them back properly later if they actually plan to use them.
> I didn't realize this was just moving it. Should probably just move the code into the separate function, and separately address these other concerns
It's already more than just moving it because it has to change stuff to merge the add/sub case which were both written a bit differently despite doing the exact same thing, so I'm not sure where the line is exactly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95426