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

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri marked 2 inline comments as done.
lebedev.ri 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);
----------------
Tyker wrote:
> 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.
Sure, but can you point me at the spot where you believe that would matter?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83177