Omr: Investigate why scalarizing of arraycopies is disabled by default

Created on 7 May 2020  路  3Comments  路  Source: eclipse/omr

While working on a miscellaneous task to improve our tracing in the compiler I stumbled upon the scalarizaion of arraycopies optimization [1]. This optimization is disabled by default because the option TR_ScalarizeSSOps is also disabled by default. This is effectively dead code at the moment.

The only place where these two TransformUtil functions are used is in VP [2] when we process delayed transformations. This seems like a useful transformation as stores are relatively far cheaper than arraycopy evaluators. Some questions:

  1. Do we know why this optimization was disabled by default?
  2. Should we invest into re-enabling it?
  3. The scalarization seems to be invoked in VP, for no apparent reason. Should we consider migrating this to the simplifier, since it really seems like a simplification rather than value propagation?

[1] https://github.com/eclipse/omr/blob/5b48c2d72447b37ba4564511e9f0318cf20626cb/compiler/optimizer/OMRTransformUtil.cpp#L41-L213
[2] https://github.com/eclipse/omr/blob/5b48c2d72447b37ba4564511e9f0318cf20626cb/compiler/optimizer/OMRValuePropagation.cpp#L7110-L7123

backlog compiler help wanted

Most helpful comment

So the TR_ScalarizeSSOps option controlled optimizations related to a tree type eliminated as part of the clean-up for contribution of the code to this project - the o-type. We would not really want to do this in the optimizer because it complicates the trees. It is much easier to grasp the semantics of what is going on when we have an arraycopy. How the arraycopy should be implemented varies by platform (there are platform checks in that transformer that hint at the platform specific nature of what is going on). As a result, I think this is of minimal value. The real trick here is to do the right thing with the arraycopy node in the codegen and or lower the arraycopy to trees when it makes doing the evaluation in the codegen appropriate. That should only happen after optimization. I suspect this code has only ever had specialist use and so whether it is functionally correct in general is in question.

Overall, I don't think there is a lot of benefit to messing with this and it can be cleaned up or moved to the codegen phase where it may make evaluation simpler if short/simple arraycopies are scalarized to save doing it manually in the assembly code...

All 3 comments

@vijaysun-omr @andrewcraik FYI hopefully you can shed some light on this.

So the TR_ScalarizeSSOps option controlled optimizations related to a tree type eliminated as part of the clean-up for contribution of the code to this project - the o-type. We would not really want to do this in the optimizer because it complicates the trees. It is much easier to grasp the semantics of what is going on when we have an arraycopy. How the arraycopy should be implemented varies by platform (there are platform checks in that transformer that hint at the platform specific nature of what is going on). As a result, I think this is of minimal value. The real trick here is to do the right thing with the arraycopy node in the codegen and or lower the arraycopy to trees when it makes doing the evaluation in the codegen appropriate. That should only happen after optimization. I suspect this code has only ever had specialist use and so whether it is functionally correct in general is in question.

Overall, I don't think there is a lot of benefit to messing with this and it can be cleaned up or moved to the codegen phase where it may make evaluation simpler if short/simple arraycopies are scalarized to save doing it manually in the assembly code...

Thanks Andrew. I'll put this on the backlog to move it to a generic, shared and configurable codegen phase and investigate whether it is worth performing. If we can find value in enabling the optimization we'll keep it. If not we will nuke it.

Was this page helpful?
0 / 5 - 0 ratings