This is an archive of the discontinued LLVM Phabricator instance.

[InstCombine] Eliminate vector reverse if all inputs/outputs to an instruction are reverses
ClosedPublic

Authored by mnadeem on Sep 14 2021, 10:15 PM.

Diff Detail

Event Timeline

mnadeem created this revision.Sep 14 2021, 10:15 PM
mnadeem requested review of this revision.Sep 14 2021, 10:15 PM
Herald added a project: Restricted Project. · View Herald TranscriptSep 14 2021, 10:15 PM

@mnadeem Nice patch.
I believe you need to test the other way around for binary operations, no?
I left a comment, I hope it makes sense.

llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
2053

Why are you not simplifying
rev(binop Splat, rev(X)) --> binop, Splat, X.
too?
When I run this test:

define <vscale x 4 x i32> @binop_reverse_splat(<vscale x 4 x i32> %a, i32 %b) {
  %reva = tail call <vscale x 4 x i32> @llvm.experimental.vector.reverse.nxv4i32(<vscale x 4 x i32> %a)
  %splat_insert = insertelement <vscale x 4 x i32> poison, i32 %b, i32 0
  %splat = shufflevector <vscale x 4 x i32> %splat_insert, <vscale x 4 x i32> poison, <vscale x 4 x i32> zeroinitializer
  %add = add <vscale x 4 x i32> %splat, %reva
  %revadd = tail call <vscale x 4 x i32> @llvm.experimental.vector.reverse.nxv4i32(<vscale x 4 x i32> %add)
  ret <vscale x 4 x i32> %revadd
}

I do not see any simplification.

mnadeem added inline comments.Sep 16 2021, 1:46 PM
llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
2053

Good catch, I'll update the code.

I only tested some C code and it seemed like there was some kind of canonicalization going on where the RHS is made to be the splat. It seems like this is only the case if the value is a constant.

mnadeem updated this revision to Diff 373359.Sep 17 2021, 4:03 PM

Address comments, handle cases where either side of the binary op is a splat and the other is a reverse.

CarolineConcatto accepted this revision.Sep 20 2021, 5:38 AM

Thank you @mnadeem.
For me, this patch looks good to land on main.

This revision is now accepted and ready to land.Sep 20 2021, 5:38 AM