Merged
Conversation
…into new-pipeline
…e propagating persistent refinements out
… it did more harm than good
… into bounds-checking
… to simplify Acond and Awhile
…e GVal on each variable The old approach wouldn't work for terms without free variables
…orm if and awhile if condition is known, and other small changes to bounds analysis
… front-end, generate assertions from beginning
…them in Language.hs and Prelude.hs Desugaring was already rather difficult, and this way we don't need to complicate it further
…esugaring (aka lowering)
By other passes, the first subexpression may be (re)moved, making the analysis unsound
The old syntax-oriented uniqueness check did not work after adding assertions to permute. The fusion & in-place updates pass also could not remove the copy, as the array was constructed in an alloc and not written to by any operation. The copy is now eliminated by the operation simplifier, which checks wheter an operation is a copy operation for all of its outputs, and the corresponding inputs are undef
This makes it more clear that the assertion is ran after the given term. This commit also adds documentation to explain these semantics.
…nd vec (un)pack in Exp simplifier
These are equivalent to Lt and GtEq with their arguments swapped. By removing these two constructors, we simplify the compiler. We also can better detect that two expressions are the same (via matchOpenExp).
Since they just restructure two tuples, I removed these constructors. We can better track and analyse the program by just using the tuple constructors (Pair and Nil). The Exp simplifier already tried to remove uses of IndexSlice and IndexFull. Now we just do that always, directly from the beginning.
It can be replaced by a regular Const. This simplifies any pass that operates on expressions, and we can still recognize the primitive constants if we ever need to do that
TODO: Handle all OperationAcc constructors, and more testing
…est to inspectCompiler, add type synonym for set of transformation type classes of backends.
Owner
Author
|
@VLNohai I implemented a simplified bounds analysis based on your implementation and experiments. The new implementation is more conservative, but should not be sound with overflows and dead code elimination. It is also shorter (as we should also consider the maintainability of Accelerate). For now I put the bounds checks behind a flag. I would like to enable them by default, but I first want to see what the performance impact of the bounds checks are. The simplified analysis can already eliminate the bounds checks from |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Adds bounds checks, and a simplified analysis to statically prove some (hopefully most) of them statically.
Motivation and context
Indexing outside the bounds of an array is a common cause of many problems. Furthermore, other operations like fold1 require that the input is non-empty, which we didn't check yet in the new pipeline
How has this been tested?
Describe how your changes have been tested. Include details of your testing environment.
Types of changes
What types of changes does your code introduce? Put an
xin all the boxes that apply:Checklist
Go over all the following points, and put an
xin all the boxes that apply. If you're unsure about any of these, don't hesitate to ask. We're here to help!