Page MenuHomePhabricator

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

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

Diff Detail

Event Timeline

mnadeem created this revision.Tue, Sep 14, 10:15 PM
mnadeem requested review of this revision.Tue, Sep 14, 10:15 PM
Herald added a project: Restricted Project. · View Herald TranscriptTue, Sep 14, 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
2066

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.Thu, Sep 16, 1:46 PM
llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
2066

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.Fri, Sep 17, 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.Mon, Sep 20, 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.Mon, Sep 20, 5:38 AM