This is an archive of the discontinued LLVM Phabricator instance.

[LoopVectorize] Use DataLayout::getIndexType instead of i32 for non-constant GEP indices.
ClosedPublic

Authored by sdesmalen on Feb 3 2023, 7:02 AM.

Details

Summary

This is specifically relevant for loops that vectorize using a scalable VF,
where the code results in:

%vscale = call i32 llvm.vscale.i32()
%vf.part1 = mul i32 %vscale, 4
%gep = getelementptr  ..., i32 %vf.part1

Which InstCombine then changes into:

%vscale = call i32 llvm.vscale.i32()
%vf.part1 = mul i32 %vscale, 4
%vf.part1.zext = sext i32 %vf.part1 to i64
%gep = getelementptr  ..., i32 %vf.part1.zext

D143016 tried to remove these extends, but that only works when
the call to llvm.vscale.i32() has a single use. After doing any
kind of CSE on these calls the combine no longer kicks in.

It seems more sensible to ask DataLayout what type to use, rather
than relying on InstCombine to insert the extend and hoping it can
fold it away.

I've only changed this for indices that are not constant, because
I vaguely remember there was a reason for sticking with i32. It
would also mean patching up loads more tests.

Diff Detail

Event Timeline

sdesmalen created this revision.Feb 3 2023, 7:02 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 3 2023, 7:02 AM
sdesmalen requested review of this revision.Feb 3 2023, 7:02 AM
paulwalker-arm accepted this revision.Feb 5 2023, 6:58 AM

Patch looks good to me albeit a little clunky to not use a consistent type regardless of constness. We should just emit the IR instcombine considers canonical (I did something similar recently when it came to splat indices) but I take your point about the significant test update burden.

This revision is now accepted and ready to land.Feb 5 2023, 6:58 AM