[PATCH] D124530: [X86] Fix gather/scatter with large scales (PR55021)

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:31567
+    EVT IndexVT = Index.getValueType();
+    Index = DAG.getNode(ISD::SHL, dl, IndexVT, Index,
+                        DAG.getConstant(Log2_32(ScaleVal), dl, IndexVT));
----------------
nikic wrote:
> gpei wrote:
> > ```
> > DATA_ADDR←BASE_ADDR + (SignExtend(VINDEX[i+31:i])*SCALE + DISP;
> > ```
> > 
> > Gather will signextend(index) to 64-bit first, then multiple the scale. When Index is 32-bit, it may occur overflow if we just multiply scale to index.
> > 
> > BTW, can we just bail out in getUniformBase when the scale is not supported by the target.
> > BTW, can we just bail out in getUniformBase when the scale is not supported by the target.
> 
> This should be possible with a new TLI hook (unless something for this already exists?)
> 
> Alternatively, we can always enforce element type == scale in SDAG builder, and let target DAG combines combine mgather/mscatter + shift into scale. X86 already does this already.
I've put up https://reviews.llvm.org/D124605 as a variant that adjusts SDAGBuilder instead. Let me know which general approach is preferred.


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

https://reviews.llvm.org/D124530