Op.Pure and granularity of effects

[Paul Sandoz]

Hi,

The term “pure” is a vague and the Op.Pure documentation is not very helpful, even misleading.

It is intended to convey that an operation’s behavior is potentially known ahead of time and if so may be constant folded if some or all of the operands are constant i.e. it can be replaced, which might mean replacing with an exceptional result and throwing.

So in the case of the array access, if the array, its contents, and the index, are determined to constant, then we can determine what the result of that access is. Determining what is a constant may be dependent on the transformation and target domain e.g., we might not be preserving Java programing meaning.

I am not convinced Op.Pure is all that useful, a possible failed experiment, it being vague, opinionated, and easily misused. In my experience it is often insufficient, and instead it may be more usefully represented as a predicate function accepting an operation. See Patterns.matchUnusedPureOps. Maybe there is some inspiration we can gather by looking at what MLIR does?

You are right about that test, it’s too aggressive in its intent if applied more generally. It needs to focus on arithmetic ops.

It's tempting to remove it, otherwise we should update the documentation to clarify its intent (and its possibility of future removal) and comment on its usages.

--

More generally we do need to review the current hierarchy and categorizations of operations esp. for use with pattern matching. Just not ready to do that right now.

Paul. 

> On Jan 27, 2024, at 3:55 AM, Hannes Greule <hannesgreule at outlook.de> wrote:
> 
> Hi,
> 
> I went through the Ops implementing Op.Pure and noticed that some of them aren't pure according to my understanding of "pure".
> 
> Namely, FieldLoadOp and ArrayLoadOp can throw exceptions (and ArithmethicOperation too, I guess).
> 
> Interestingly, the test code in [1] has a similar understanding of pureness; It might eliminate expressions that throw exceptions!
> 
> I came up with two test cases showcasing the problem:
> 
> @Test
> public void testAccessField() {
>    record MyRecord(int a) {}
>    CoreOps.ClosureOp lf = generate((MyRecord myRecord) -> {
>        int i = myRecord.a;
>        return -2;
>    });
>    Assert.assertThrows(NullPointerException.class, () -> Interpreter.invoke(MethodHandles.lookup(), lf, (Object) null));
> }
> 
> @Test
> public void testArrayLoad() {
>    CoreOps.ClosureOp lf = generate((int[] array) -> {
>        int i = array[-1];
>        return -2;
>    });
>    Assert.assertThrows(ArrayIndexOutOfBoundsException.class, () -> Interpreter.invoke(MethodHandles.lookup(), lf, (Object) new int[0]));
> }
> 
> I think to tackle that problem, introducing more interfaces (or a method that returns a set of side effects) would make analyses easier. For example, splitting into "throws exception", "loads from memory", "writes to memory" might easy to understand and work with. Everything that doesn't describe a side effect this way can considered pure then.
> 
> We can go further (which is only possible with interfaces, not with an enum) and let an interface describe which exceptions it can throw. This could be useful when analysing try-catch blocks.
> 
> Please let me know what you think of the problem and the suggestions.
> 
> Hannes
> 
> 
> [1] https://github.com/openjdk/babylon/blob/af40e7d754bc527797dc5c582750dba7c4c9e3af/test/jdk/java/lang/reflect/code/expression/ExpressionElimination.java#L94-L101