[xls][mlir] Add missing MLIR dialect ops

[schilkp]

@jpienaar

Adds the following XLS-supported ops to the MLIR dialect:

• nand, nor
• and_reduce, or_reduce, xor_reduce
• umulp, smulp
• gate

Note that for the partial product operations, operands of different bit widths and operands of different bit width than the result are explicitly allowed.

Please double check the following in the tablegen ODS for partial product ops:

TypesMatchWith<"result types must match", "result_lhs", "result_rhs", "$_self">

Is this a sane way of constraining the two result values to have the same type? Is there a different constraint that matches better? Seems like a bit of a hack to use this with the "identity transform expression" "$_self" :)

[jmolloy]

Am I correct in saying that the element type of $result should always be i1? If so, can this please be encoded as a constraint?

[schilkp]

You are correct.

Here I was mirroring the behavior of all other ops that produce single-bit results (eq, ne, sge, ...) with the exception of them providing a builder with an I1 return type default.

I guess for the sake of your scalarize-setup I can't just have a return type of I1.

Do you prefer a simple verifier for this, or a new Xls_Bit type (that is either I1 or a tensor thereof) which then could also be used for better typing of the comparison ops down the road?

[jpienaar]

Looks good with James's comments addressed.

[schilkp]

Regarding the AllTypesMatch constraint:

Ok this sent me on a bit of a journey :)

I had actually tried AllTypesMatch originally, but this did not link because
inferReturnTypes was not defined for the partial product ops. Since we cannot
infer return types here, I assumed this constraint was not the correct one and used
TypesMatchWith.

With your review I dug into this a bit more. Adding the constraint
AllTypesMatch<["result_lhs", "result_rhs"]> to Xls_PartialProdOp, resulted
in the following:

• For SmulpOp, inferReturnTypes was both declared and implemented.
• For umulpOp, inferReturnTypes was only declared.

This makes little sense, because no result types should be inferable. Also,
there is no reason for the two ops to behave differently: Their ODS is
perfectly identical except for name and description.

I tracked this down to a bug that triggers some UB inside the MLIR TableGen
backend. This explains the non-deterministic behaviour.

See upstream PR here:
llvm/llvm-project#122717

In short: AllTypesMatch fell apart with multiple result constraints, because
of incorrect indexing (result index vs combined arg&result index).

Do you want to keep this as-is, or wait for upstream to propagate down and use AllTypesMatch?

@jmolloy @jpienaar

[schilkp]

LLVM fix merged upstream. This should not be merged until an LLVM version with this is included is pinned: llvm/llvm-project@7aebacb

[schilkp]

Rebased. Required upstream commit is now available.

[schilkp]

I saw this got the "Reviewing Internally" label. Please hold-off merging this for a bit - I will address the final review above once #1856 is merged.