Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Conversation

CarolEidt
Copy link

@dotnet/jit-contrib PTAL
This reflects my initial thinking for restructuring fgMorphArgs() for improved struct passing.

--------------------------
This is a preliminary design, and is likely to change as the implementation proceeds:

First, the `fgArgInfo` is extended to contain all the information needed to determine
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would also, in theory, make it easier to support new calling conventions (like __vectorcall on Windows), correct?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right! I was just thinking about that yesterday during a discussion on the topic of HW intrinsics.

@sdmaclea
Copy link

Carol this looks like a reasonable first draft.

This will also make fixing the ARM64 Short Vector and HVA ABI for intrinsics easier as you mention.

I have also wondered why the gathering of info from VM was so different between platforms. It is possible reworking the JIT VM interface should also be considerd.

For the altjit case it would be nice if the VM always populated ABI info for all platforms.

You may also want to comment/design handling the argument shifting when invoking indirectly?

@CarolEidt
Copy link
Author

It is possible reworking the JIT VM interface should also be considerd.

Good point; I'll add it to a "futures" section, along with the Vector ABIs.

For the altjit case it would be nice if the VM always populated ABI info for all platforms.

Another excellent idea; at least when there's an altjit present. Given that we've already done this for the HFA and Unix struct register arguments, this shouldn't add a great deal of complexity, I wouldn't think.
Filed #18131.

You may also want to comment/design handling the argument shifting when invoking indirectly?

Can you elaborate? I'm not sure what you mean.

- Sets up the actual argument for any non-standard args.

- Transform struct arg nodes from `GT_LCL_VAR`, `GT_OBJ` or `GT_MKREFANY` into:
- `GT_FIELD_LIST` (i.e. a list of fields) if the lclVar is promoted and
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't it be possible to avoid the need for this field list concept by "decomposing" the call argument into multiple arguments of primitive types? That is, if you have a struct containing 3 int fields and the calling convention requires it to be passed on the stack can't that single struct argument be split into 3 separate int arguments (each initialized from the 3 lclvars that were supposed to be in the field list) ?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that would add more complexity than simplification. I don't think we'd want to do it up-front (i.e. at the time of the proposed fgInitArgInfo(), as it would unnecessarily break up the codegen for the case where it's loaded from the stack. Decomposing later would mean that we'd either have to change the fgArgInfo (making it not match the actual signature), or have some other mechanism to map back from these "partial" arguments to their associated fgArgInfo. There may be a simpler approach, but it's not evident to me.
That said, I suspect (as I mention in the document) that the design will evolve over time as we gain a better understanding of how well it plays with the rest of the JIT.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as it would unnecessarily break up the codegen for the case where it's loaded from the stack.

I'm not sure I understand this part. In any case, if it cannot be done up-front then it's probably not worth doing. The reason I think such decomposition might be useful is that it may allow more node ordering flexibility, I've seen this happening with long arguments in my long helper call experiments. And of course, it would avoid this concept of "field list" but then I'm not sure it can avoid it completely for this advantage to be relevant.

Decomposing later would mean that we'd either have to change the fgArgInfo (making it not match the actual signature), or have some other mechanism to map back from these "partial" arguments to their associated fgArgInfo.

Yes, if done I'd think it should be reflected in arg info. The call node should look exactly as if it was a call with 3 int arguments, except differences required by the ABI (e.g. passing those args on stack instead of regs). And yes, it would be different from the actual signature, does that matter? I kind of expect it not to be needed after import/morph.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't forget cases where multiple struct fields are passed in a single register and need to be packed/unpacked to/from multiple promoted registers.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't forget cases where multiple struct fields are passed in a single register and need to be packed/unpacked to/from multiple promoted registers.

Right, and it's not only about fields passed in registers, it also affects structs passed on the stack if multiple fields end up in the same stack slot. So it's not really about "decomposing" arguments into primitive type fields/args but rather decomposing into registers and stack slots. Unfortunately that would likely be way too complicated to do as it also requires generating IR to pack such fields.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it also affects structs passed on the stack if multiple fields end up in the same stack slot.

We already have code generator support for passing a promoted struct on the stack. This is easier than passing in registers, since there's no packing involved - it simply stores (or pushes in the case of x86) each field on the stack at the appropriate offset.

@sdmaclea
Copy link

You may also want to comment/design handling the argument shifting when invoking indirectly?

Can you elaborate? I'm not sure what you mean.

When making an indirect call through reflection, there is some funny business where the arguments get shifted right to insert an extra argument. @jkotas or @janvorli would probably better explain in it. It probably would only affect the caller, as the callee would see the arguments in the right place.

I'm not sure if it affects this rework or not.

@CarolEidt
Copy link
Author

When making an indirect call through reflection, there is some funny business where the arguments get shifted right to insert an extra argument. @jkotas or @janvorli would probably better explain in it. It probably would only affect the caller, as the callee would see the arguments in the right place.

Right - thanks. IIUC those are the "non-standard" args. They would be added to the argTable and the arg list on the call during the proposed fgInitArgInfo() method.

@jkotas
Copy link
Member

jkotas commented May 25, 2018

ome funny business where the arguments get shifted right to insert an extra argument

I guess you meant delegate shuffle thunks. I do not think they are affected in any way by this proposals.

@CarolEidt
Copy link
Author

I guess you meant delegate shuffle thunks. I do not think they are affected in any way by this proposals.

Actually, I think they are, unless I'm confused about what you mean. When calls get transformed to use these kinds of thunks, we have to pre-pend arguments to the arglist, and that's currently done in fgMorphArgs(). This proposal extracts that (and the rest of the argTable creation) to a separate method.

@CarolEidt CarolEidt merged commit fc8b156 into dotnet:master May 29, 2018
@CarolEidt
Copy link
Author

I believe I incorporated all the feedback so far, but feel free to suggest additional modifications.

@CarolEidt CarolEidt deleted the StructAbiDoc branch May 29, 2018 19:02
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants