@rriddle as an external consumer of MLIR, it is quite annoying to have to parse both foo<"..."> as well as foo.bar<balanced-parens>.

Rebase patch to latest MLIR

Rebase on latest build, fix tests

Out of curiosity: may I please have some context for what this base class is being created for? Ie, what consumers other than the existing PatternRewriter will soon exist? :)

I was curious on the progress of this patch: May I pick it up if work has stalled, @uenoku?

Thanks for the answer! That clears up a lot.

I have a question in the context of optimising sequential programs -- I'm interested in optimising across a trampoline call, since I have a haskell-like lazy language, which lowers CPS into LLVM-IR by constructing a trampoline. Example IR here Some salient features are:

Looks good, I haven't looked too much into how Any works, which I will.

Looks good, I haven't looked too much into how Any works, which I will.

So, how do we coordinate this? :) Or do you push the patches at the same time?

Thanks! However, I am a little confused: when do I merge this, precisely? It breaks on current LLVM HEAD.

Abandoned in favour of D49024.

@Meinersbur in polly, we use sizes per dimension, so the sizes data structure actually stores the size of a dimension. In the "stride" version, we rather store the "block size" of each dimension, which is akin to the Fortran representation, and the representation (if I understand the chapel representation correctly).

Extract out codegen as well.

• Backport code to detect chapel/fortran style indexing. TODO: Codegen.

As mentioned, I don't have the bandwidth :( If you could fix the issues that were raised during the review, it would be awesome.

Sorry, I meant to accept this last week. I accept this as an acceptable "software engineering" compromise, while I still hold the philosophical reservations I have mentioned.

@Meinersbur If you are opposed to merging this, please request changes.

@Meinersbur even assuming that such a concept exists (since one could argue such a concept does exist for a range-based for loop to work), I still don't think it allows the compiler to optimise away iterators based on the interpretation of the spec I outlined (== is an equivalence relation whose domain is mutable).

Also, please run the file through instnamer. That way, we will have "named" instructions and not "numbered" instructions. This is useful to modify the testcase in the future (it is less brittle): "numbered" instructions must be sequential, while named instructions need to be.

I'm going to request changes so this does not show up on the feed. I added two more comments regarding constructors :)

Thanks for the patch! I've added some comments about general LLVM style, and some personal choices. Feel free to ignore the personal choices :)

Thanks for the patch! I've added some comments about general LLVM style, and some personal choices. Feel free to ignore the personal choices :)

This might be too much to ask for, but is it possible to report_fatal_error() at the earliest when accessing out of bounds using the iterator? If ISL performs inbounds checks, then disregard this.

This might be too much to ask for, but is it possible to report_fatal_error() at the earliest when accessing out of bounds using the iterator? If ISL performs inbounds checks, then disregard this.

I'm sorry. I don't have the bandwidth for this right now. Please do take it up, it'd be great to see this upstreamed! If we can get the base version upstreamed, I don't mind quickly iterating on smaller features / tests / things like that.

@Meinersbur I agree. However, I don't think that this is the right place for the code either. It is more a DependenceInfo thing, and maybe it makes sense to teach DependenceInfo to generate this information for us?

Thanks a lot for the patch! I took a shot at a first round of review :) I can be nitpicky, so we can hash out the details as we go along with the review.

Regarding StripDebugInfo, I did not know it existed. I should refactor the code to use it, I assume? I'll check this out.

No, unfortunately, I'm not sure. I didn't want to guess, so I didn't write a particular clang revision.

Removed expletives from patch that arose due to frustration with debug metadata. Run clang-format on file.

Sorry for the delay, I resubmitted the patch as D40778. I'm closing this in favor of continuing the discussion there. Ping, @vsk, and thanks for all the help so far!

Other than request for documentation, LGTM. I did not test it, but I reduced the test case with bugpoint, so it should be the right fix. If not, I can complain again ;)

@vsk In that case, I think we agree. Bugpoint is useful, but a debugger is really helpful in certain contexts. It's easier for me (and I suspect others) to debug on the IR level when they would like to do so. Does that provide enough of a motivation to go ahead with the review of the patch? You mentioned that there are technical issues, I'd like to sort them out :)

@vsk Bugpoint does has four weaknesses that I have noticed not work (as far as I know, please do let me know if bugpoint can do this)

@vsk I do use the same technique for testing my frontend. However, this is useful when stuff _breaks_ in the frontend.g I'm not as good at reading asm as I'd like to be, so being able to debug at the IR level is a boon.

• remove unrelated files.

• update code and test case

• [NFC wrt patch] remove debug code

• add changes in PPCGCodeGen that were missed due to incorrect rebase.

@efriedma Thanks, I totally screwed up the rebase :)