[PATCH] D14797: [BranchFolding] Merge MMOs during tail merge
Chad Rosier via llvm-commits
llvm-commits at lists.llvm.org
Wed Dec 2 08:00:48 PST 2015
mcrosier added a comment.
Another suggestion.
================
Comment at: lib/CodeGen/BranchFolding.cpp:756
@@ +755,3 @@
+ // Mostly, MI1's MMO count is 1 or zero. So we don't have to use
+ // SmallSet.
+ for (; I1 != E1; ++I1) {
----------------
mcrosier wrote:
> gberry wrote:
> > flyingforyou wrote:
> > > When I wrote the code with N^2 search, there is a reason that most of cases, MI only has a one or zero MMO.
> > > So, This routine is faster than using SmallSet.
> > >
> > > In order to support that most of MI has one or zero MMO, I tested some benchmarks app and got a data.
> > >
> > > When I investigated 12354 MI which is used by mergeMMOs function,
> > > MI(MMO is 0) == 1595, MI(MMO is 1) == 10514. Sum of two cases is 12109 and it's 98%.
> > >
> > > So, I don't think Smallset is useful in this case.
> > > But there is no explain why I use N^2 search, so I added the comment.
> > >
> > > I want you agree with this, please.
> > I believe a SmallSet will have almost exactly the same performance as the code you have written here when N is small, since it just uses a SmallVector and does a linear search. If N ever gets large though it will switch to using a std::set which should have log(N) lookup. I'd like to hear from another reviewer on whether this is warranted in this case.
> I believe the number of MMO's on a MI is target-dependent. For AArch64 it tends to be 1, but for x86 it can be more, IIRC. Therefore, I tend to agree with Geoff; we should consider using a SmallSet. @gberry, do you have a suggestion on how the compare function could be written?
As an alternative you might consider adding logic to handle the common cases and then use std::unique for the other cases.
Something like:
static void mergeMMOs(MachineInstr *MI1, MachineInstr *MI2) {
// Nothing to merge.
if (MI2->getNumMemOperands() == 0)
return;
if (MI1->getNumMemOperands() == 0) {
// Copy MMOs from MI2 to MI1.
}
if (MI1->getNumMemOperands() == 1 && MI1->getNumMemOperands() == 1) {
if (MMO1 == MMO2)
return;
// Copy MMO2 to MI1.
return;
}
// Copy all MMOs from MI2 to MI1.
// std::unique MI1's MMOs.
}
http://reviews.llvm.org/D14797
More information about the llvm-commits
mailing list