- User Since
- Jun 17 2015, 7:07 AM (239 w, 2 d)
Fri, Jan 10
Thu, Jan 9
It would be nicer if you would commit (and upload for review) the three changes separately. You might just commit the first two changes since I think they are ok, but I am not sure about the ensureValid change, which needs some expertise from someone knowing sys::DynamicLibrary and llvm::ManagedStatic.
Wed, Jan 8
Tue, Jan 7
Thank you for the patch!
I changed CMAKE_CURRENT_BINARY_DIR to LLVM_BINARY_DIR in the commit description. Thank you for the patch.
Please add another comment for if you need someone else to do the commit.
Fri, Jan 3
With or without my suggestion, the change look fine.
I don't think CMAKE_CURRENT_BINARY_DIR will work. It is different for each subdirectory, such that multiple plugins create separate Extension.def.tmp instead of appending to a central one. LLVM_BINARY_DIR might be the better choice.
Fri, Dec 20
[serious] Overriding CMAKE_CXX_FLAGS applies the change globally to everything in the test-suite. The file itself shows how it is done correctly right before the line you added.
Dec 18 2019
- Adapt for refactored ASTRecordReader/Writer
- Rework TransformedTree a bit
This is a quite deep introspection that I'd assume would be in the domain of some analysis, such as value numbering. It could ensure that equivalent conditional branches will take the same llvm::Value.
LGTM. Please consider the [style] remarks as well before committing.
Dec 17 2019
Btw, this patch did not apply cleanly on trunk. I instead test the version from your repository.
I tested the configurations mentioned at https://groups.google.com/d/topic/polly-dev/vxumPMhrSEs/discussion again and did not find any issues that were not there before. Therefore IMHO this is good to go.
Dec 16 2019
Could you upload the nest diff with context (git diff -U999999)?
Dec 13 2019
I am ok with this patch. @fhahn what do you think?
[suggestion] The scope of the lcssa PHI node and the definition of its value are in different loops, hence calling getSCEVAtScope at the lcssa's scope may simplify the expression. This might even be mandatory since SCEV should be canonical.
Dec 12 2019
- Add release notes
- Remove handling of OpenMP simd and LoopHints by CGTransform
- Remove handling of LoopHints (#pragma clang loop) and OpenMP (#pragma omp simd) to reduce the size of the patches. Compatibility with OpenMP is still a goal (to implement the upcoming OpenMP loop transformations such as #pragma omp tile), but for now they are handled by separate code paths.
- Simplify transformation classes
Dec 11 2019
I'd only store the list of perfectly nested loops that are have the analysis loop as outermost loop in the analysis results. This should be the most expensive analysis that might be worth caching/preserving between passes.
Dec 10 2019
Dec 7 2019
I slightly tend towards @bmahjour's suggestion, for convenience of the user.
Is there a mistake in the example? Every execution will end in the infinite loop header4. Why did you mean to move preheader3 instead of latch1?
Dec 5 2019
I do not understand why the ordinals are necessary. For deterministic behavior, the deterministic iteration order over blocks and instructions should already ensure that (unless you are iterating over DenseMaps, which does not seem the case and is easily fixed). If for correctness, do you have an illustrative example when this would be critical?
[suggestion] Add a test case to check for "not rotated" to be rejected.
Dec 2 2019
LGTM assuming this fixes the leak (and the unused value warning).
For reference: landed as aaf7f05a96e6c21b7a6d1ad9e73fb7ab5eee7d83
[suggestion] Could you add a test case with a reduction (not necessarily in this patch), such as
double sum = 0; for (int i = 0; i < n; i+=1) sum+=A[i]; return sum;
Nov 25 2019
I still have to try out the patch.
[serious] We did not come to a conclusion during the discussion about the semantics ivdep being implementation-dependent.
Nov 22 2019
- Remove some leftovers
Nov 21 2019
Nov 20 2019
[suggestion] Add a methods that returns/fills a vector with all the Loop*s that are part of the perfect loop nest.
Nov 19 2019
- Address @ABataev's review
Nov 18 2019
- Address @bhomerdin's review:
- Add README
- Respect TEST_SUITE_BENCHMARKING_ONLY
Adding @grosser as reviewer who wrote the code in question and already looked into the issue.
Nov 14 2019
Could you elaborate why this is better than __builtin_assume_aligned?
s/NDEBUG/LLVM_NDEBUG/ will not apply to the use of NDEBUG in assert.h. Most uses of NDEBUG are hidden in the assert macro.
Nov 12 2019
Sorry for the delay, for some reason I did not see a notification for the update.
Nov 11 2019
Thanks for looking into the bug. The suggestion change is obvious (IMHO).