RFR: 8359965: Enable paired pushp and popp instruction usage for APX enabled CPUs

Volodymyr Paprotski vpaprotski at openjdk.org
Wed Jul 2 18:35:40 UTC 2025


On Wed, 2 Jul 2025 17:44:34 GMT, Jatin Bhateja <jbhateja at openjdk.org> wrote:

> @vamsi-parasa, It's better to make this as a subclass of MacroAssembler in src/hotspot/cpu/x86/macroAssembler_x86.hpp and pass Tracker as an argument to push / pop for a cleaner interface.

I don't think its possible? Unless I am missing something.. 
- Subclass has an instance of the base class (i.e. the memory allocation of `PushPopTracker` would have the `MacroAssembler` base class with extra fields appended); and `MacroAssembler` has already been allocated (i.e. you can't tack on more fields onto the end of the underlying memory of existing `MacroAssembler`..)
- If its a subclass, there is no reason to pass it as a parameter, because it already would have the parent's instance? Also, the extra parameter to push/pop (flag) was what I had originally objected to? (i.e. would like for push/pop to still just take one register as a parameter..)
- This class is sort of a stripped-down implementation of reference counting; we want the stack-allocated variable (i.e. explicit constructor call) and the implicit destructor calls (i.e. inserted by g++ on all function exits). That is, we must have a stack allocated variable for it to be deallocated (and destructor called for assert check)

Here is an attempt to make it a subclass? And sample usage...

class PushPopTracker : public MacroAssembler {
   private:
      int _counter;
      const int REGS = 32; // Increase as needed
      int regs[REGS];
   public:
   // MacroAssembler(CodeBuffer* code) is the only constructor?
      PushPopTracker() : _counter(0), MacroAssembler(???) {} //FIXME???
      ~PushPopTracker() {
         assert(_counter == 0, "Push/pop pair mismatch");
      }

      void push(Register reg) {
         assert(_counter<REGS, "Push/pop overflow");
         regs[_counter++] = reg.encoding();
         if (VM_Version::supports_apx_f()) {
            Assembler::pushp(reg);
         } else {
            Assembler::push(reg);
         }
      }
/*...*/
}

address StubGenerator::generate_intpoly_montgomeryMult_P256() {
  __ align(CodeEntryAlignment);
  /*...*/
  address start = __ pc();
  __ enter();
  PushPopTracker s(???); //FIXME?
  s.push(r12, /* Extra parm? */); //1

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

PR Review Comment: https://git.openjdk.org/jdk/pull/25889#discussion_r2180738209


More information about the hotspot-compiler-dev mailing list