RFR: 8330851: C2: More efficient TypeFunc creation

[Amit Kumar]

On Wed, 30 Oct 2024 06:43:23 GMT, Amit Kumar <amitkumar at openjdk.org> wrote:

> Lazy computation of TypeFunc.
> 
> Testing: Tier1 on Fastdebug & Release VMs (`s390x architecture`)

@dean-long can you take a look at these changes

"Pre-submit test" for zero are not related. 

[build.sh][INFO] Downloading https://archive.apache.org/dist/ant/binaries/apache-ant-1.10.8-bin.zip to /home/runner/work/jdk/jdk/jtreg/src/make/../build/deps/apache-ant-1.10.8-bin.zip
Error: sh][ERROR] wget exited with exit code 4
Error: Process completed with exit code 1.

Sorry for delay, I was out for long weekend.

>For example, rename LockNode::lock_type() to LockNode::lock_type_init(), and have it save the result in a static const field. Then have LockNode::lock_type() simply return the field.

But as you mentioned "the data field is `static const`". So we can't do assignment operation in the class itself. To do that we have to go outside the scope of class and do the definition part there. Or do you have another way in mind ?

with that change I am getting this error: 

=== Output from failing command(s) repeated here ===
* For target hotspot_variant-server_libjvm_objs_BUILD_LIBJVM_run_ld:
Undefined symbols for architecture arm64:
  "LockNode::_lock_type_tf", referenced from:
      GraphKit::shared_lock(Node*) in graphKit.o
      LockNode::lock_type_init() in type.o
ld: symbol(s) not found for architecture arm64
clang++: error: linker command failed with exit code 1 (use -v to see invocation)


Here is shorter version: 

class Temp {
public:
  static const int* ptr;

public:
  static void set_ptr() {
    const int *abs = new int(20);
    ptr = abs;
  }
};

// Initialize static member;
const int* Temp::ptr = nullptr;

int main() {
  Temp::set_ptr();
  cout << *Temp::ptr << endl;
  return 0;
}


If I comment out `const int* Temp::ptr = nullptr;` then I am getting the similar error as I pasted above which I got from the build failure. Here we might need to give the definition out of scope of the class. 


Another solution is making the data-field inline: 

class Temp {
public:
  static inline const int* ptr = nullptr;

public:
  static void set_ptr() {
    const int *abs = new int(20);
    ptr = abs;
  }
};

// Initialize static member;
//const int* Temp::ptr = nullptr;

int main() {
  Temp::set_ptr();
  cout << *Temp::ptr << endl;
  return 0;
}


Here If we mark `ptr` as inline variable that is also acceptable, though C++17 started accepting it, but hotspot code is throwing warning over there as well.

I don't see any way through which we can shrink the code here; 

Though methods with `*_Type` can be derived from macro because all of them are doing same task i.e. checking for assert & returning the field. But not sure that's a good choice. Because it will sprinkle the macro everywhere. Overall I think code will became less intuitive and more error prone. 


const TypeFunc *OptoRuntime::athrow_Type() {
  assert(_athrow_tf != nullptr, "should be initialized");
  return _athrow_tf;
}


But if you want this or have another idea, I am happy to give it try.

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

PR Comment: https://git.openjdk.org/jdk/pull/21782#issuecomment-2446193033
PR Comment: https://git.openjdk.org/jdk/pull/21782#issuecomment-2446204918
PR Comment: https://git.openjdk.org/jdk/pull/21782#issuecomment-2453901207
PR Comment: https://git.openjdk.org/jdk/pull/21782#issuecomment-2456246788
PR Comment: https://git.openjdk.org/jdk/pull/21782#issuecomment-2458710642