RFR: 8274130: C2: MulNode::Ideal chained transformations may act on wrong nodes

[Andrew Dinn]

On Wed, 22 Sep 2021 10:12:16 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

> I was puzzled by it when fixing JDK-8274060. It looks that new optimizations added by [JDK-8273454](https://bugs.openjdk.java.net/browse/JDK-8273454) and [JDK-8263006](https://bugs.openjdk.java.net/browse/JDK-8263006) rewire `in(1)` and `in(2)` in `MulNode::Ideal`, which means the chained transformations should see them? Yet, both inputs and their `Type`-s are cached locally and not refreshed. I have not seen failures due to this yet, but it looks that the current code is subtly incorrect because of this. 
> 
> I thought about doing `return this` instead of `progress = true`, so that we leave `MulNode::Ideal` once we hit any transform and hope to return back, but I wondered if that would expose us to different graph shapes in-between successive `MulNode::Ideal` calls, which might have other unintended consequences. Therefore, I opted to a more conservative patch.
> 
> Additional testing:
>  - [x] `compiler/` tests
>  - [x] `tier1` tests
>  - [ ] Fuzzer tests

This is actually cleaner but I'm not sure the change is strictly needed. In these specific transforms I think the types of the operands and the operation ought never to differ.

e.g for the multuply rule we transform (MulF (MinusF f2) (MinusF f2)) ==> (MulF f1 f2. The types of the MinusF terms input to the MulF both have to be float. So, do the types of the inputs f1 and f2. We should never get an input graph that has, say, a float for one arg and a double for another.

-------------

PR: https://git.openjdk.java.net/jdk/pull/5631