Flags should be different from an expand parameter and a divided parameter

[llvmbot]

Bugzilla Link	1643
Resolution	FIXED
Resolved on	Mar 06, 2010 14:00
Version	trunk
OS	All
Reporter	LLVM Bugzilla Contributor
CC	@lattner,@sunfishcode

Extended Description

Currently, when doing a call or for formal arguments, SelectionDAG gives a OrigAlignmentFlag to:

• arguments whose type is expanded (eg i8 -> i32)
• broken up arguments (eg i64 -> i32 + i32)

However the two cases need to have a different flag in the PPC32 ELF ABI. In this target, arguments are aligned when they are double word, for example:

declare void @​foo(i32 %a, i64 %c)

%a goes into register 1
%c goes into registers 3 and 4

The PPC code generator sees three arguments for this function of type i32 and the last argument has the flag OrigAlignmentFlag. Currently, it can not know if the last argument is an i64 divided into two i32 or if the last argument is i1, i8 or i16 which are expanded.

[lattner]

Interesting. What do you suggest?

[llvmbot]

Well this would need to break the OrigAligment flag into two flags: "Devided" flag and "Expanded" flag. This is the temporary hack I did on my linux/ppc32 box.

However, it will probably break many backends, so I didn't commit it.

We could change the flags, and send on the list a warning mail for backend developers. I'm not really familiar with the Codegen part, so I would not rely on my "devided flag" implementation.

[lattner]

Evan, any thoughts?

[llvmbot]

Seems like there are a number of ways to "fix" this.

1. Break OrigAligment into 2 as Nicolas suggested.
2. Give the flag one more bit and use the high bit to indicate whether it's expanded or promoted.
3. Add another flag OrigType.

I am not sure if original type is ever needed. If not, then I am okay with either #​1 or #​2.

[llvmbot]

New ExpandedFlag for expanded parameters

[llvmbot]

Does this patch look correct? I'll apply it if there are no objections.

[llvmbot]

Does this patch look correct? I'll apply it if there are no objections.

Ah, no, I'm using a bit that is already used by OrigAligment. Does someone know what bit I could use (hence what bit can be removed from an existing flag, since they're all already used).

[llvmbot]

Use the first bit flag of the second part of a divided argument
This patch uses the first bit of the flag. Since the 1 to n parts of a divided argument do not use any flags except OrigAlignment, this should be correct.

[lattner]

Evan, please review this patch when you get a chance, thanks!

[llvmbot]

this should be correct.

Actually, this patch is still incorrect. The next argument can either be a divided arg or a zext i8.

So I think I really need a new flag in the enum. Evan, can I take a flag from ByValSize (hence the limit of byvalsize is divided by two)? Or do you have a better solution?

[llvmbot]

Why not go with solution #​3? If the frontend provides the OrigType (before it's expanded or divided), would that solve the problem? If the original type is smaller, than obviously it's expanded, or else it's divided. I think that's the cleanest solution. What do you think?

[llvmbot]

I'm very OK with solution #​3 but how do you model it? Where do you put the new flag?

[llvmbot]

I think we can get away with just encoding a OriginalSize parameter flag. Since this is only needed for scalar (with power-of-2 size?), can we just add a 3-bit field? That means we must steal 3 bits from ByValSize which is hoarding 32-bits. Any issues, Chris, Dale?

[llvmbot]

I have just expanded ParamFlags from 32 to 64 bits, so there should be ample room for a new field now.

[llvmbot]

To clarify, there is no need to steal bits from any existing field.