Accidental duplicate.

Review.

Rebase

Nit: could you please update the commit message to specify the translation _from LLVM IR_. Otherwise, it may trick a casual reader into thinking that we haven't supported translating simple instructions _to LLVM IR_ until this point.

I am not very convinced by the motivation of LLVM backend selecting the appropriate instructions. On other platforms, we have repeatedly seen the selection being fragile and sensitive to factors beyond our control, so I would rather have the possibility to target these directly even if it is not being used by some compilations pipelnes.

Thanks!

In D125543#3512054, @bzcheeseman wrote:

Actually it'd probably be easier to just add an if (std::is_same<T, ::mlir::Operation *>::value) return true; condition to isPossible in the non-const casts, everything else should "just work".

This will not happen for a random class that does not have CastInfo specialized though. For Operation *, the compiler picks the CastInfo<T, ::mlir::Operation *> specialization over the more generic CastInfo<T, T>, I don't think there is an easy way around this.

The minimal example is

Nice, thank you!

Could you please reference the commit that changed the function name in the description of this commit?

Since reshape ops don't have offsets, sizes, or strides, this

patch simply sets the allocated and aligned pointers of the MemRef
descriptor.

I'm incline to rewrite the algorithm used in GEPOp::findKnownStructIndices (and recordStructIndices) to adopt the better solution I mentioned previously.

Thanks, something might be off with the check, but I'm not sure I agree with your current assessment. The verifier does recurse into structs - https://github.com/llvm/llvm-project/blob/ff8d0b338f48ab26919ac84aef2c79e1e1a20ef2/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp#L406-L429. My another suspicion is that build methods are not matching your expectation, it would be helpful to have a MLIR-only test that demonstrates the problem by parsing IR, rather than building it (use the generic form to bypass parser checks). If this proves impossible, the issue is in the build method.

This is indeed sad, but seemed like least worst solution compared to enable_if.

Please add tests for user-visible error messages.

The complexity of the test is scary. Is it just a matter of having a sufficient amount of instructions and/or constants in a vector?

Yes, it will be required. b84f95fe5348ac97d33ffd2f6845b7f1b17b389c should silence the warning.

Any reason to restrict num_threads to be 32-bit specifically? The spec says it must evaluate to a positive integer value, nothing about its size. I would rather keep it as AnyInteger, and maybe even allowed Index.

Nice catch, thanks!

I edited the description a bit. Please also note that phabricator-style markup (e.g., using // for italic) looks weird in git commit messages (e.g., C-style comment).

Nice, thank you!

We should add a facility (a pass presumably) that hoists allocas up to some user-specified level. Maybe the allocation scope that is not nested in another allocation scope, but is nested in the closest isolated-from-above operation. I believe @wsmoses has a prototype for this.

In D124181#3465833, @Mogball wrote:

Does allowing the reordering of transform operations really make sense if they do not declare all their pattern inputs and outputs? E.g.

alloc/write(%1) = transform.foo read(%0) 
alloc/write(%2) = transform.bar read(%0)

What if foo creates other ops for which bar optionally checks and which, if present, change what bar does? Reordering these two operations becomes invalid. Any transform op that corresponds to a pattern, for example, that is not purely functional (i.e. that does not declare all of its input ops and output ops) will need to be annotated with a generic MemoryRead and MemoryWrite.

Address review.

Thanks for the cleanup!

Try fixing Windows.

Fix misrebase.