Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Detect overlapping memory in assignments #746

Open
ukoethe opened this issue Mar 29, 2018 · 4 comments
Open

Detect overlapping memory in assignments #746

ukoethe opened this issue Mar 29, 2018 · 4 comments

Comments

@ukoethe
Copy link
Contributor

ukoethe commented Mar 29, 2018

At present, assignments create a temporary copy of the RHS, unless noalias() explicitly prevents it. In many cases, the possibility of aliasing can be ruled out by simply ensuring that the LHS memory has no overlap with the RHS memory. I think that the test will succeed quite often in real code (although I've never actually measured how often), freeing the programmer from the need to infer manually if noalias() is permitted at any particular assignment. This is not only a simplification, but also takes care of many situations were the programmer just doesn't possess the required information to decide about noalias() (e.g. within a generic function that has no control over its inputs).

I propose to implement such a check and use it in suitable assignment operations. To get an idea of how this works, you might want to have a look at xvigra's class overlapping_memory_checker and its use in assign_impl.

@JohanMabille
Copy link
Member

JohanMabille commented Mar 30, 2018

Hi UlIrich,

Thanks for coming back on this. I changed my mind about this, I agree with you that in generic functions taking any kind of expression as its input, the programmer has no simple mean to decide whether to use noalias or not. So even if we cannot cover 100% of the case, covering 80% of the cases where we can avoid temporaries and being conservative to avoid bugs in the remaining 20% sounds like a good approach for me.

I think that we can even go further and try to detect the need for temporaries at compile time. Note that this is not in contradiction with what you propose since there are cases where static check is not possible and in that cases we will rely on the dynamic one. This static check can be implemented after the dynamic one, though.

About the implementation, I think it would be better to add a method in each expression type instead of centralizing it in a single class for the following reasons:

  • when we add a new expression type, we don't need to change other files / classes, the new expression is self-contained
  • we don't bring dependencies on everthing in the semantic classes.

@SylvainCorlay
Copy link
Member

Note: as per discussions with Johan, I would go for the compile time cases first, but still be very coutious because it is easy to get it wrong...

We would prefer to iron out the library on stability, cleanup, documentation before we take this on...

@ukoethe
Copy link
Contributor Author

ukoethe commented Mar 30, 2018

covering 80% of the cases sounds like a good approach

Exactly.

it would be better to add a method in each expression type

For now, I wanted a simple design that was non-intrusive on xtensor, but a method in each expression certainly has advantages. The downside is that the method must be implemented in every expression, whereas a non-intrusive design can just implement the cases one cares about and return true otherwise. But this is up to you to decide...

detect the need for temporaries at compile time

I'm not sure to what extend this can succeed. I think it is very difficult to figure out the overlap of views at compile time. For example, two views referring to different z-slices of a 3D volume wouldn't overlap, but when the indices given to xt::view() are variables (as is usually the case, because these calls often happen within loops), the compiler cannot be certain.

As a general remark: I've become more conservative about compile-time computations over the years. They have their downsides (more complex code, hard to explain to students, longer compilation times, crazy compiler errors), and the performance gains are often not big enough to justify these complications.

@ukoethe
Copy link
Contributor Author

ukoethe commented Mar 30, 2018

before we take this on

You can also start with a non-intrusive approach that covers the important cases, and improve it later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants