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

Matt Arsenault via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jan 29 10:32:13 PST 2021


arsenm 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();
----------------
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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95426



More information about the llvm-commits mailing list