Support rodata relocations in crossgen2 for ARM32

[clamp03]

No description provided.

[clamp03]

I fixed a crash in System.String:StartsWith(System.String, int):bool:this which uses JMPTABLE.
And I tested on ARM32.

[jkotas]

The unmanaged crossgen does this for Unix ARM only.

[clamp03]

Thank you. Fixed.

[jkotas]

cc @dotnet/crossgen-contrib

[MichalStrehovsky]

This is a rather dangerous API - it changes what the node depends on within the dependency graph. If the node is already marked, the new dependencies won't be visited. We try to avoid APIs like this.

I would prefer we create a new class for this (e.g. SettableBlobNode) that is pretty much a copy of BlobNode with these differences:

• Instead of the byte[] _data field, it has a ObjectData _data field.
• Data is not passed to the constructor (so we start with _data == null)
• StaticDependenciesAreComputed property is implemented as return _data != null (this is the protection to make sure we don't end up in a situation where first there are no dependencies reported in the graph)
• There is a InitializeData(ObjectData data) method to initialize the _data field.

[clamp03]

Thanks for your comment. I fixed based on your guide.

[MichalStrehovsky]

Let's move the throw new NotImplementException to the place where we're reading locationBlock to have this together and avoid the duplicated condition.

[clamp03]

Thank you. Fixed.

[MichalStrehovsky]

Nit: could use a ref ArrayBuilder<Relocation> sourceBlock; local variable that either points to _codeRelocs or _roDataRelocs` to avoid the duplicated code.

[clamp03]

Thank you. I fixed but I am not sure I did in right way.

[MichalStrehovsky]

Looks good otherwise, thanks!

[MichalStrehovsky]

Could you rename this method to SettableReadOnlyDataBlob? We might want to have the non-settable version back at some point.

[clamp03]

Thanks. Renamed.

[MichalStrehovsky]

Rename this to SettableReadOnlyDataBlobKey as well.

[clamp03]

Thanks. Renamed too.

[MichalStrehovsky]

I didn't realize this was the only use of BlobNode. Can you also delete BlobNode.cs? It feels like something we will need again, but we can just restore it from history it's ever needed again.

[clamp03]

Sure. I deleted BlobNode.cs.

[MichalStrehovsky]

The alignment is actually coming from the ObjectData, so this parameter and field is unused.

[clamp03]

I removed alignment. Thank you!

[MichalStrehovsky]

Looks good otherwise! Thank you!

[MichalStrehovsky]

Can you rename the method to SettableReadOnlyDataBlob?

[clamp03]

Renamed. Thank you!

[MichalStrehovsky]

Sigh, sorry to pester one more time - since the alignment parameter is now gone from here, this struct can be deleted too. We can just use the Utf8String as the key to the NodeCache instead.

[clamp03]

No problem. Thank you so much!

[davidwrighton]

@AntonLapounov It looks like this relocation isn't handled correctly today in CorInfoImpl.GetRelocType . Please fix that there. Also, please adjust the value of this enum to be like the other ARM64 specific relocation types.

[AntonLapounov]

@davidwrighton What code generates this relocation type? Apparently none of my tests hit it.

[davidwrighton]

@AntonLapounov if we tweak the R2R codegen to generate direct calls/jumps at any point, it will do it. It doesn't now, but it might in the future.

The two interesting cases are if we come up with a scheme that allows direct calls without going through a R2R entrypoint indirection cell, or when we implement hot/cold splitting of methods. Of the two, I believe that hot/cold splitting is more likely in the .NET 5.0 timeframe, but neither is currently scheduled.