[PATCH] D122026: just resize the cache, dont assert equality of sizes

[Benoit Jacob via Phabricator via llvm-commits]

Benoit added a comment.

In D122026#3393235 <https://reviews.llvm.org/D122026#3393235>, @nikic wrote:

> This looks like a consistency assertion that an `<n x %ty>` vector has an `n`-element scatter in the cache -- so it's not obvious to me that just dropping the assertion is fine, I think you might be hiding a legitimate issue here.

Thanks, ok let's dig deeper! Question to start really understanding this code. This `CachePtr` points to a `ScatterMap`. That type is defined here <https://github.com/google/iree-llvm-fork/blob/9879c555f21097aee15e73dd25bd89f652dba8ea/llvm/lib/Transforms/Scalar/Scalarizer.cpp#L82> as:

  using ScatterMap = std::map<Value *, ValueVector>;

So the keys are not Value's --- they are pointers to Value's.

This seems to be assuming that any Value used as a key will outlive this cache. Otherwise, another unrelated Value might get created at the address of a defunct Value that was a key in this cache, and the cache would get confused. What is ensuring that that isn't the case?

> Extracting a test case should be easy with some combination of llvm-reduce and/or reduce_pipeline.py?

It's complicated because I'm only reproducing this in a dependent project that has its own MLIR-based compiler, and that creates the scalarizerPass in C++. I dumped the IR just before running Scalarizer, put it here <https://gist.github.com/bjacob/092ed55d5b336bd3e6df20d01ccdc02e>, but i don't know how to reproduce the issue with `opt`. I tried,

  ./bin/opt -passes='function(scalarizer)' -scalarize-load-store -S /tmp/a.ll

but scalarizer isn't doing anything on this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122026