[PATCH] D155218: [InstCombine] Optimize addition/subtraction operations of splats of vscale multiplied by a constant
Paul Walker via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Aug 9 09:37:57 PDT 2023
paulwalker-arm accepted this revision.
paulwalker-arm added a comment.
This revision is now accepted and ready to land.
A couple of minor requests but otherwise looks good.
================
Comment at: llvm/lib/Transforms/InstCombine/InstructionCombining.cpp:1614
+ // 2) A multiplication of a constant and VScale
+ // 3) A shift left of VScale on a constant value
+ auto m_ConstMultipliedVScale =
----------------
by
================
Comment at: llvm/lib/Transforms/InstCombine/InstructionCombining.cpp:1656-1657
+
+ B = getSplatValue(SplatB);
+ C = getSplatValue(SplatC);
+
----------------
Better to move the initialisation here (i.e. `Value *B = `) since there's no other uses.
================
Comment at: llvm/lib/Transforms/InstCombine/InstructionCombining.cpp:1599-1602
+ if (match(&Inst,
+ m_c_Add(m_c_Add(m_Value(A), m_SplatVscale), m_SplatVscale)) &&
+ match(&Inst, m_c_Add(m_c_Add(m_Specific(A), m_Instruction(SplatB)),
+ m_Instruction(SplatC)))) {
----------------
igor.kirillov wrote:
> paulwalker-arm wrote:
> > This seems wasteful given the number of times you're matching `m_SplatVscale`. Perhaps drop the first `match` from all the `if` blocks and then once you've figured out the add/sub combo you can have a single block that does:
> > ```
> > if (!match(SplatB, m_SplatVscale) || !match(SplatC, m_SplatVscale))
> > return nullptr;
> > ```
> >
> > Perhaps you don't even need `m_SplatVscale` because `getSplatValue` returns null on failure so the check could be:
> > ```
> > if (!B || !match(B, m_ConstMultipliedVscale) ||
> > !C || !match(C, m_ConstMultipliedVscale))
> > return nullptr;
> > ```
> What do you think about this version? My first version was using `getSplatValue`, but then I had to decide the exact order of A, B and C. Using the current approach, we can first match A, SpatB, SplatC and then match the exact add/sub operations without calling splat matcher several times.
Seems like a reasonable compromise to me.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D155218/new/
https://reviews.llvm.org/D155218
More information about the llvm-commits
mailing list