Any reason why you have not converted all slots to this scheme?

Non-virtual slots and non-vtable slots are plain pointers in non-NGEN mode and relative pointers in NGEN mode, so, for IsVtableSlot() slot is always a relative pointer (for ARM with FEATURE_FNV_MEM_OPTIMIZATIONS), otherwise it's not.

Why can't you use PlaceInternedStructureForAddress anymore? PlaceInternedStructureForAddress was used here as binary size optimization.

I've missed the check for MethodTable::VTableIndir2_t::isRelative there: it should be PlaceStructureForAddress if this condition is met, otherwise, as before. I'll fix this.

When vtable chunks consist of relative pointers, they can't be reused, as these chunks can be the same for different method tables, but base addresses must be different.

The base address is always the address of the slot itself. I do not see what prevents reusing them.

What I meant was that without this optimization if there are two method tables MT1 with chunk1 and MT2 with chunk2, these chunks differ, as they consist of different absolute addresses. If addresses are relative, memory blocks of these chunks might become equal, when all methods in both chunks have the same relative offsets. Then, base addresses of these chunks have to be different in order to store different addresses in them.

If addresses are relative, memory blocks of these chunks might become equal

PlaceInternedStructureForAddress does not look at bytes inside the blocks. It just looks at the addresses. I do not think that the addresses can ever become accidentally equal.

I agree, this place could be left unchanged, as m_reusedStructures in PlaceInternedStructureForAddress will not contain vtable chunks, and this change is unneeded. Thank you.

Should we have just one flag for it that turns it on/off for the whole thing?

Two separate flags could be useful for enabling more or less aggressive memory optimizations for FNV. Now there are next possible combinations:
isRelative==false and isRelativeAfterIndirection==false
isRelative==true and isRelativeAfterIndirection==false
isRelative==true and isRelativeAfterIndirection==true

I understand that it is more flexibility, but I doubt that we will ever take advantage of it and it increases the number of options one has to worry about when working on the code.

We target different devices and this option might be useful, as memory reduction of this optimization comes with performance reduction on benchmarks.
Maybe this and similar places could be refactored, so that it will be easier to maintain them. What do you think about this?

Historically, fine grained controls like these did not survive for long. I think we just need two options: fully position independent data structures on/off. Similar to how position-independent-code works in C/C++. You either turn it on or off. There are no fine-grained controls.

Fixed

The FEATURE_FNV_MEM_OPTIMIZATION ifdefs should be just on the VM side. They are not necessary on the JIT side since the JIT gets instructions on what to do from the VM side, and the cost of the extra checks is negligible.

Fixed

I think the name of the ifdef should be something like FEATURE_NGEN_RELOCS_OPTIMIZATIONS to better describe what it does.

And switch the existing places that use #if defined(PLATFORM_UNIX) && defined(_TARGET_ARM_) to use it as well.

Could we add build with FEATURE_NGEN_RELOCS_OPTIMIZATIONS for Linux ARM to default set of tests on pull requests?

That's certainly possible.

This is changing JIT/EE interface definition. The JIT/EE interface definition will need new GUID.

It would be better to add IAT_RELPVALUE at the end of the enum rather than in the middle.
It less of a breaking change that way.

Fixed

There's a fundamental problem here, or maybe two: (1) you are temporarily trashing R11, which is the frame pointer. This will break stack walking, and local variable access (e.g., debugging), and if an exception occurs after FP has been trashed, FP wouldn't get restored, and the stack would also be off, and (2) we never change the stack pointer outside of the prolog or epilog. I think that's required, also for stack walking.

Presumably if you ran GCStress in a fully-interruptible method with this type of call, you would hit a problem (since GC would be attempted after the push). Actually, I'm guessing GC would fail dramatically because we don't track stack pointer moves outside the prolog/epilog, so the GC info would be incorrect.

The correct way to do this would be to make information known to the register allocator that we need a temporary register here, and then grab that temporary. This might be a case where we've never done that, so I don't know if there's an established mechanism for communicating this.

I notice also that StubLinkerCPU::ThumbEmitCallManagedMethod has apparently the same call sequence, which uses R11 in the same way. I'm not sure what the requirements are on stubs, and whether this is ok in that context or not.

@BruceForstall, what if we use some other register for this instead of R11? For example, R1, to fix problem (1), which you have mentioned.

@BruceForstall For problem (2), we can reserve additional space on stack in prolog, and store helper register there instead of push. This will allow to not change stack pointer. What do you think about this?

Those two things would probably make this work. However, it wouldn't be the "right" way to do it. The right way is to get the register allocator to allocate a temporary register that would be available at this point. It done, then it would automatically save/restore any conflicting register (if needed), and allocate stack space (if needed).

@BruceForstall I've pushed new commits related to these. Could you, please, take a look?

We do not use FEATURE ifdefs for cases like these when the VM asked the JIT to do something. We just leave the handling in unconditionally, even though it creates a bit of unreachable code in the JIT. It simplifies the matrix of different build combinations we need to worry about. It is nice to have one JIT library that can handle all of them (per host/target combination).