Thanks! Let me go over this in more detail tonight.

@Mogball Hi, I don't have commit access. Could you commit this patch for me? Thanks!

Explicitly spell out WorkItem instead of auto.

I don't have commit access. Could you submit this patch for me? Thanks!

Pass odsState.regions to inferReturnTypes() and update relevant test (op-result.td).

Initialize ConstantValue::dialect with nullptr.

Update the definition of ConstantValue into { Optional<Attribute> constant; Dialect *dialect; }.

In D133260#3768847, @Mogball wrote:

In this case, the actual values differ (1 vs 2) so they won't fold

In D133260#3768806, @Mogball wrote:

If two ConstantValues have the same constant but different dialects, then they are still unequal because they would materialize into different ops.

This isn't true. Most dialects upstream (and many downstream) use arith.constant.

This patch will unnecessarily impede hybrid dialect IRs from behaving nicely in SCCP

In D132800#3768799, @Mogball wrote:

It doesn't make sense to me that the fold result of a math.absi should differ from that of an arith.addi if their attributes are the same but their dialects are different (both dialects use arith.constant).

Created https://reviews.llvm.org/D133260 for letting dialect participate in equality comparison. Will rebase this patch on top of that one.

In D132800#3768503, @Mogball wrote:

Ugh that's true. Integer range inference (the test pass) is broken then. At least I wouldn't have the Impl struct and just have Optional<Attribute> with a Dialect * hanging around as metadata

Reformat.

Remove setValue() - join() should be enough for a monotonic framework.

Reformat.

Rebase.

In D132086#3732322, @Mogball wrote:

Can you take a look at the failures?

ConstantPropagationAnalysis.cpp:42:5: error: use of undeclared identifier 'markAllPessimisticFixpoint'
    markAllPessimisticFixpoint(results);

Rename NonConstant to UnknownConstant.

In D131660#3716480, @aartbik wrote:

I am added as reviewer due to the trigger word "sparse" ;-), but this changes looks acceptable (perhap make it a bit more clear that is does change semantics beyond just removing the function in the title)

Also remove isAtFixpoint() from dense analysis.

I think update() is better than manually calling propagateIfChanged() because it reduces the chance of misuse without requiring more code.

Sorry for the delay, I'll leave some more comments tomorrow.