This is an archive of the discontinued LLVM Phabricator instance.

[X86][AVX] Attempt to fold a scaled index into a gather/scatter scale immediate (PR13310)
ClosedPublic

Authored by RKSimon on Aug 23 2021, 4:02 AM.

Details

Summary

If the index operand for a gather/scatter intrinsic is being scaled (self-addition or a shl-by-immediate) then we may be able to fold that scaling into the intrinsic scale immediate value instead.

Fixes PR13310.

Diff Detail

Event Timeline

RKSimon created this revision.Aug 23 2021, 4:02 AM
RKSimon requested review of this revision.Aug 23 2021, 4:02 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 23 2021, 4:02 AM
RKSimon updated this revision to Diff 369896.Sep 1 2021, 3:49 AM

Fix clang-format warnings

pengfei added inline comments.Sep 1 2021, 6:30 AM
llvm/test/CodeGen/X86/masked_gather_scatter.ll
813

Why do we need an extra xor now?

RKSimon planned changes to this revision.Sep 10 2021, 9:13 AM

I'm going to see whether this would be better moving to matchVectorAddressRecursively now that D111595 has landed.

llvm/test/CodeGen/X86/masked_gather_scatter.ll
813

D112505 - for dependency breaking I guess :)

pengfei added inline comments.Oct 26 2021, 1:44 AM
llvm/test/CodeGen/X86/masked_gather_scatter.ll
813

Interesting :)

craig.topper added inline comments.Oct 26 2021, 8:12 AM
llvm/test/CodeGen/X86/masked_gather_scatter.ll
813

I guess this is because we were calling getAVX2Gather which adds dependency breaking?

RKSimon updated this revision to Diff 382670.Oct 27 2021, 7:37 AM
RKSimon edited the summary of this revision. (Show Details)

rebase

pengfei accepted this revision.Oct 28 2021, 5:55 AM

LGTM.

This revision is now accepted and ready to land.Oct 28 2021, 5:55 AM
craig.topper added inline comments.Oct 28 2021, 8:09 AM
llvm/lib/Target/X86/X86ISelLowering.cpp
50239

Does the Index element size need to be pointer width for this to be legal? The implicit sign extend is applied before the scale.

RKSimon added inline comments.Oct 28 2021, 11:23 AM
llvm/lib/Target/X86/X86ISelLowering.cpp
50239

Nice catch! I'll add a check/test.

craig.topper added inline comments.Oct 28 2021, 12:16 PM
llvm/lib/Target/X86/X86ISelLowering.cpp
50252

Is there a common helper function we can split off of getAVX2GatherNode and getGatherNode to use here. Most of the code in getAVX2GatherNode isn't needed for this path and the Mask handling in getAVX2GatherNode happens to work for AVX512 gathers, but wasn't intentional.

The helper could probably be shared with the getMemIntrinsicNode call in LowerMGATHER as well.

Similar for getScatterNode and the getMemIntrinsicNode call in LowerMSCATTER.