[PATCH] D105053: [INSTCOMBINE] Transform reduction(shuffle V, poison, unique_mask) to reduction(V).
Sanjay Patel via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Jun 29 07:00:08 PDT 2021
spatel accepted this revision.
spatel added a comment.
This revision is now accepted and ready to land.
LGTM - see inline for a few minor points.
================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp:2010
+ ArrayRef<int> Mask;
+ if (auto *FVTy = dyn_cast<FixedVectorType>(Arg->getType()))
+ if (CanBeReassociated &&
----------------
Nit: we're not using FVTy, so it would be easier to read (less indented) if we just bail out on scalable vectors:
if (!isa<FixedVectorType>(Arg->getType())) break;
You could also combine the next set of conditions with this to early exit and remove another level of indent.
================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp:2015
+ int Sz = Mask.size();
+ SmallBitVector UsedIndeces(Sz);
+ for (int Idx : Mask) {
----------------
Unusual spelling: Indices or Indexes?
================
Comment at: llvm/test/Transforms/InstCombine/reduction-shufflevector.ll:90
%shuf = shufflevector <4 x float> %x, <4 x float> poison, <4 x i32> <i32 2, i32 0, i32 3, i32 1>
%res = call float @llvm.vector.reduce.fmax.v4f32(<4 x float> %shuf)
ret float %res
----------------
This is correct (no FMF required) based on LangRef, but it would be good to add some extra flags on at least one FP test, so we verify that those are all propagated through the transform.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D105053/new/
https://reviews.llvm.org/D105053
More information about the llvm-commits
mailing list