[PATCH] D83177: [llvm-reduce] Reducing call operand bundles

Tyker via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jul 6 14:09:17 PDT 2020


Tyker added inline comments.


================
Comment at: llvm/tools/llvm-reduce/deltas/ReduceOperandBundles.cpp:138
+
+  for_each(R.CallsToRefine, [](const auto &P) {
+    return maybeRewriteCallWithDifferentBundles(P.first, P.second);
----------------
lebedev.ri wrote:
> Tyker wrote:
> > lebedev.ri wrote:
> > > Tyker wrote:
> > > > Maybe we should make CallsToRefine a MapVector since the association from a index to a Bundle depends on its order in the map.
> > > > and the key depends on a pointer value that will change when the Module gets cloned.
> > > No, the current logic is correct, that map doesn't live that long.
> > here is the situation i think fails.
> > 
> > extractOperandBundesFromModule is called a first time and generate a reduction.
> > the reduction isn't considered interesting.
> > extractOperandBundesFromModule is called a second time with different chunks to keep.
> > but the association from an index to a Bundle is different from the first time because the module isn't the same
> > the association from a index to a Bundle isn't the same,
> > so extractOperandBundesFromModule can remove the same operand bundle a second time and not have tried every operand bundles at the end of the passe.
> > 
> > 
> In `reduceOperandBundesDeltaPass()`, we tell `runDeltaPass()` how many features (here: bundles)
> we have in this module `M0`. It then comes with different chunks to keep and tells us
> to mutate the module `Mc` (which is a perfect clone of `M0`).
> 
> It is up to us to actually enumerate the features (here: bundles).
> As long as the mapping is stable, i.e. we get the same result
> when calling `extractOperandBundesFromModule()` on the same input,
> we're good.
> when calling extractOperandBundesFromModule() on the same input, we're good.

i agree but runDeltaPass makes clones of modules before giving them to `extractOperandBundesFromModule` to modify.
so the pointer values of Modules will not be the same across runs of `extractOperandBundesFromModule` even if the Modules are identical.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D83177/new/

https://reviews.llvm.org/D83177





More information about the llvm-commits mailing list