[premain] need help merging g1BarrierSetC2.cpp

[ioi.lam at oracle.com]

Hi Andrew,

Thanks for making the fixes. I ran tiers 1-3 in our test pipeline and 
don't see any regression any more. All leyden tests passed.

I've merged your commits into the premain branch.

Thanks

- Ioi

On 10/15/24 9:14 AM, Andrew Dinn wrote:
> Hi Ioi,
>
> I think I know now have bot aarch64 and x86 code working. The tweak I 
> made after the merge to Ashu's code in 
> G1BarrierSetAssembler::generate_post_barrier_fast_path that selects 
> which registers to be pushed and used as scratch registers was 
> incomplete -- it failed to allow for aliasing of a pushed scratch 
> register to new_val and store_addr.
>
> I pushed a follow-up patch to my premain repo which fixes this:
>
>
> https://urldefense.com/v3/__https://github.com/adinn/leyden/commit/88b6b4b2770cc976b82a5f4c35fd87c94b947d28__;!!ACWV5N9M2RV99hQ!PpxY4tG39vD_inF2jUPnQElY-EfCfvqoYLK9eeX6pK8SMDznvK_noH10OdAV9A8doJs7T0_GDfg$ 
>
> All the tests that you flagged as failing are now passing with this 
> extra commit.
>
> Could you merge that change from here and rerun tests:
>
> https://urldefense.com/v3/__https://github.com/adinn/leyden/tree/premain__;!!ACWV5N9M2RV99hQ!PpxY4tG39vD_inF2jUPnQElY-EfCfvqoYLK9eeX6pK8SMDznvK_noH10OdAV9A8doJs7CMpdo3M$ 
>
>
> regards,
>
>
> Andrew Dinn
> -----------
>
> On 14/10/2024 15:49, Andrew Dinn wrote:
>> Hi Ioi,
>>
>> I believe I have worked out what was causing the aarch64 code to 
>> break. A call from archived C2 code to 
>> G1BarrierSetRuntime::write_ref_field_pre_entry was not being 
>> relocated and so we ended up jumping to an address in the old VM 
>> 0xffff76c77578 rather than the correct address in the new VM 
>> 0xfffff4c77578.
>>
>> This was happening because the C2 barrier code was refactored to use 
>> some new helper methods and one of them, 
>> generate_c2_barrier_runtime_call, failed to load its branch target 
>> using a RuntimeAddress wrapper
>>
>>      static void generate_c2_barrier_runtime_call(MacroAssembler* 
>> masm, G1BarrierStubC2* stub, const Register arg, const address 
>> runtime_path) {
>>        SaveLiveRegisters save_registers(masm, stub);
>>        if (c_rarg0 != arg) {
>>          __ mov(c_rarg0, arg);
>>        }
>>        __ mov(c_rarg1, rthread);
>>        __ mov(rscratch1, runtime_path);
>>        __ blr(rscratch1);
>>      }
>>
>>
>> The mov to rscratch1 above needs to be replaced with
>>
>>    __ lea(rscratch1, RuntimeAddress(runtime_path));
>>
>> I have pushed a patch to the branch in my repo and with this patch 
>> the ExcludedClasses, JavacBench, MicronautFirstApp, HelidonQuickStart 
>> and SpringPetclinic tests all passed.
>>
>> I am not sure what is breaking x86 but it is not the same problem -- 
>> the x86 implementation of generate_c2_barrier_runtime_call is defined 
>> correctly as follows:
>>
>>      static void generate_c2_barrier_runtime_call(MacroAssembler* 
>> masm, G1BarrierStubC2* stub, const Register arg, const address 
>> runtime_path) {
>>      #ifdef _LP64
>>        SaveLiveRegisters save_registers(masm, stub);
>>        if (c_rarg0 != arg) {
>>          __ mov(c_rarg0, arg);
>>        }
>>        __ mov(c_rarg1, r15_thread);
>>        // rax is a caller-saved, non-argument-passing register, so it 
>> does not
>>        // interfere with c_rarg0 or c_rarg1. If it contained any live 
>> value before
>>        // entering this stub, it is saved at this point, and restored 
>> after the
>>        // call. If it did not contain any live value, it is free to 
>> be used. In
>>        // either case, it is safe to use it here as a call scratch 
>> register.
>>        __ call(RuntimeAddress(runtime_path), rax);
>>      #else
>>        Unimplemented();
>>      #endif // _LP64
>>      }
>>
>> I will move on now to debugging the problems on x86.
>>
>> regards,
>>
>>
>> Andrew Dinn
>> -----------
>