[PATCH] D25022: Register Calling Convention - CodeGen Basic Additions

[Oren Ben Simhon via llvm-commits]

oren_ben_simhon marked 9 inline comments as done.
oren_ben_simhon added a comment.



In https://reviews.llvm.org/D25022#559221, @rnk wrote:

> In https://reviews.llvm.org/D25022#557751, @asl wrote:
>
> > Can't you yield the same outcome via 'inreg' attribute?
>
>
> It will have similar effects, but it is not ABI compatible with Intel's __regcall.


Also, InReg attribute tells backend to save an argument into a register, 
but doesn’t tell you: how, where, in which order?
To answer all these questions __regcall spec was defined.



> mkuper wrote in X86CallingConv.td:53
> GRP_8 and GPR_16 are only used for return values, right?
> When will you use anything but AL/AX to return?

You are right, GPR8/GPR16 are used just for return values.
In regcall calling convention there are cases in which you return a value in multiple registers.
Some examples:

- v64i1 (mask) in IA32 is splitted into two registers
- a structure that is returned, will be splitted to multiple registers according to its fields.

> mkuper wrote in X86CallingConv.td:95
> If I'm reading this correctly, with this definition, you'll pass f80 in an incompatible way - which sounds fairly bad. The right thing to do would be to either implement it in one go, or leave a TODO, but refuse to compile cases which use f80.
> Otherwise, once you want to fix this, you'll have an unsolvable compatibility problem, since you can't be compatible with both old versions of clang, and with ICC.

As you said, interoperability is not possible. At this moment, this is true due to many reasons.

I had to split the feature to multiple small reviews, because there are 3 additional features that required a lot of changes:

- v64i1 - split values to ultiple registers
- v*i1 types - mask registers support
- fp80 - update floating point support to enable register based arguments

So, I believe that it is better to handle this gracefully for the time being.

> mkuper wrote in X86CallingConv.td:125
> Why <0, 0> vs. <0, 4>? Should the 4 have been 0 as well?

In the case of f80 on 32 bit architecture, we know that the stack alignment is 4 bytes.
So there is no need to pass 0 as the second parameter (which is translated to another function query of the type)
See also CC_X86_32_Common.

> mkuper wrote in X86CallingConv.td:128
> How can we even get here? Is regcall supported for FP on non-SSE platforms?

Yes, regcall supports non-sse platforms. In that case, FP will be saved on the stack.

> rnk wrote in X86CallingConv.td:1120
> Do you anticipate that users will use this convention in a context where SSE is disabled? If so, you should look at r282819 and do something similar.
> 
> Honestly, we should refactor this code. We should probably compute these lists by filtering out register classes in X86RegisterInfo rather than table generating N different lists for every calling convention.

Yes, SSE disabled is possible. I will make similar changes to what you did in r282819.
I wanted to ask you, why did you update the returned SaveList in x86RegisterInfo.cpp but not the corresponding RegMask?

> mkuper wrote in avx512-regcall-NoMask.ll:347
> Have you checked what happens when you try to pass a 256-bit vector without AVX, 512-bit without AVX512, etc?
> I assume there's no compatibility requirement for these cases, but it'd be good to check it does something that makes sense.

Indeed this is a nice test. Such test is part of sse_regcall.ll

Repository:
  rL LLVM

https://reviews.llvm.org/D25022