Addressed review of Matt Arsenault (put the Connex back end in the experimental list, LLVM_EXPERIMENTAL_TARGETS_TO_BUILD, in CMakeLists.txt; added the source file llvm/unittests/TargetParser/TripleTest.cpp).

Addressed reviews of Matt Arsenault.
Formatted all C++ source files with clang-format.

Indented better the code, partly as suggested by the clang-format tool.

Added relevant tests in test/CodeGen/Connex.

Addressed most reviews of efriedma, arsenm, luismarques (and of asb).

Added all required files for the back end (a few were missing).

In D60052#1471560, @asb wrote:

Some initial feedback:

• This patch is pretty huge which makes it pretty hard to meaningfully review
• There seem to be effectively no tests. I'd expect test/CodeGen/Connex and test/MC/Connex to be reasonably well populated
• Plenty of code commented out or with date-based comments that don't match our style, e.g. // 2018_*
• Files have the old license header rather than the new one

Made a first round of corrections following Alex Bradbury's review.

Added myself as maintainer of the Connex backend in CODE_OWNERS.TXT.

Added all source files for the Connex back end.
Followed coding standards from https://llvm.org/docs/CodingStandards.html .

More refactoring on the .td TableGen files. Added also more source files.

A few corrections to ConnexInstrInfoVec.td .

Added 2 more files. I still have to add another about 50 files.

Addressed small review issue of greened (David Greene) on comment in method: void visitAddRecExpr(const SCEVAddRecExpr *Numerator) .

Did corrections, following Dave Green's comments.

Made a better patch following comments from Michael Kruse.
Most importantly I added comments that I consider SCEVDivision::divide() to be signed division.

Addressed Michael Kruse's comment on the patch - changed the test .ll file.

Adding, as Roman suggested, in the header of the test_sext.ll file a line describing the RUN command and a CHECK line.

Adding as Roman said the patch for ScalarEvolution.cpp and also the associated test test/Analysis/Delinearization/test_sext.ll .

Note that in this patch we only address the SExt, as suggested by Michael (Meinersbur).

Adding very simple unit tests on which normally Polly fails to detect the right SCoP due to the sext (and trunc, as well) in both dividend and divisor used in the third step of the delinearization algorithm (it seems the SCEV expressions sext(N * N), sext(N), etc are created by the 1st step of the delinearization algorithm extracting the terms from the sum of products from the array index i * N * N + j * N + k) presented in Section 4.1 of the paper Grosser et al, "On recovering Multi-dimensional Arrays in Polly", IMPACT 2015 - http://impact.gforge.inria.fr/impact2015/papers/impact2015-grosser.pdf ). Because of this Polly complains of non-affine expression for array indexing.
To make Polly work, we need to apply the patch I already committed (I will put another patch ASAP).
Please note my patch is not incremental w.r.t. patch from https://reviews.llvm.org/D35478 - you need to apply ONLY my patch.

In D38953#900086, @Meinersbur wrote:

Thanks for improving our documentation!

Hello.

I applied the patch that was provided in this review to my LLVM build.
I would like to draw attention to a bug of the deliniarization procedure that is related to type casts (from i32 to i64), and is probably related to the problems that were supposed to be addressed by the patch in this review.
I attach a C file that reproduces this bug - the delinearization procedure fails due to sext from i32 to i64, and Polly finds Non affine access functions, which he shouldn't. Note that in the same C file I also provide on line 3 a commented signature of the Test function which can be uncommented, for which delinearization works OK and all access functions are affine.
I will try to look deeper in the problem with delinearization. Please let me know if you can help with this problem.

Best regards,

Alex

(nothing serious, part 2)

(nothing serious)

In D35478#814250, @ManuelSelva wrote:

Hi Tobias,

Thank you for looking at that. To first answer your questions "out of interest", this IR is generated by JavaScriptCore, a JavaScript virtual machine performing JIT compilation. The virtual machine is using NaN-boxing, i.e., uses the unused values (all representing NaN) of 64 bits floating point representation to represent JavaScript objects. In this representation, 32 bits integers are represented by 64 bits values all starting with the FFFF 0000 32 bits pattern. As a consequence, to get the integer value from a 64 bits value, the virtual machine generates LLVM code that truncates the value to a 32 bits integer.

Then, when performing arithmetic operations on these 32 bits integers, we need to ensure that they do not overflow, because if it's the case, we can't represent them anymore with the NaN-boxing trick and so we need to do *something* to represent them properly. This "something" is not present in the test case I provided in order to make it as minimal as possible.

If you are curious on this NaN-boxing thing and more generally on JavaScript implementations, you can have a look at the following links:

• https://manuelselva.github.io/docs/presentations/2017-06-21-Manuel-Selva-js-polyhedral-journees-compil.pdf (see in the backup slides, slides I made for a recent presentation)
• https://wingolog.org/archives/2011/05/18/value-representation-in-javascript-implementations)

I'll now look into the details of this patch, to ensure its correctness as you said.

Best,
Manu