eclipse / omr

Eclipse OMR™ Cross platform components for building reliable, high performance language runtimes
http://www.eclipse.org/omr
Other
943 stars 396 forks source link

Investigate possibility of consolidating all sign/zero extension optimizations #4107

Open fjeremic opened 5 years ago

fjeremic commented 5 years ago

As part of #4103 we fixed a bug due to incorrect handling of sign/zero extension on Z. As part of this work and the previous work done in #2205 I think we need to do some further cleanup/consolidation in the area of sign/zero extension.

The Load Extensions [1] codegen optimization is cross platform, but only Z is taking advantage of it at the moment. There also seem to be other sign extension optimizations on Z which I cannot find in other platforms, for example the use of the resetAlreadySignExtended and setAlreadySignExtended APIs on global registers to denote that a sign extension has been performed [2] on a global register at the source.

More globally we have notions of sign extension optimizations in GRA which sets the needsSignExtension flag on a node [3]. I do not fully understand what this GRA optimization does that Load Extensions does not already do, since we look for global opportunities as part of Load Extensions as well. In addition the GRA sing extension optimization seems to be an afterthought that is only enabled for Z [4].

I do not think we should pollute the GRA optimization with such sign extension decisions. This should be the job of Load Extensions which is a self-contained single purpose optimization. Consolidating all sign/zero extension optimizations to within the Load Extensions optimization further reduces code duplication and potential overlap as seen in #4103.

There are several things we need to investigate and work on here:

  1. Understand the GRA sign extension optimization and what use cases it is trying to cover (#2187 may be a useful reference here)
  2. Understand how the GRA optimization different from what we do globally within Load Extensions
  3. Investigate whether it is possible to handle what is done via alreadySignExtended APIs with the plumbing we already have in Load Extensions and thereby making this optimization available on other platforms
  4. Investigate whether all the sign/zero extension optimizations can be consolidated in Load Extensions

[1] https://github.com/eclipse/omr/blob/1fb4081278d1b322120cb2b1bf6fe6ea66fb9c8b/compiler/optimizer/LoadExtensions.hpp#L37-L103 [2] https://github.com/eclipse/omr/blob/1fb4081278d1b322120cb2b1bf6fe6ea66fb9c8b/compiler/z/codegen/OMRTreeEvaluator.cpp#L12072 [3] https://github.com/eclipse/omr/blob/1fb4081278d1b322120cb2b1bf6fe6ea66fb9c8b/compiler/optimizer/GlobalRegisterAllocator.cpp#L943-L962 [4] https://github.com/eclipse/omr/blob/1fb4081278d1b322120cb2b1bf6fe6ea66fb9c8b/compiler/optimizer/GlobalRegisterAllocator.cpp#L3058-L3069

fjeremic commented 5 years ago

@andrewcraik @cathyzhyi @gita-omr @0xdaryl FYI Z is the only codegen currently taking advantage of Load Extensions as it is an optional optimization. To enable it on other platforms we would need some codegen work to ensure the various flags that are set are respected.

github-actions[bot] commented 4 years ago

This issue is stale because it has been open 180 days with no activity. Remove stale label or comment on the issue or it will be closed in 60 days.

fjeremic commented 4 years ago

I still want to get to this eventually to reduce the complexity we have throughout the compiler in this area.

gita-omr commented 4 years ago

@fjeremic I agree this something we should look into.