LLVM's type legalizer is changing the types of BUILD_VECTORs in a way that seems wrong to me, but I'm not sure if this is a bug or if some targets may be relying on it. On a 32-bit target, the default action for legalizing i8 and i16 types is to promote them. If you then have a BUILD_VECTOR to construct a legal vector type composed of i8 or i16 values, the type legalizer will look at the BUILD_VECTOR operands and decide that it needs to promote them to i32 types. You end up with a BUILD_VECTOR that constructs a vector of i32 values that are then bitcast to the original vector type. This works fine for SSE, where it appears that BUILD_VECTORs are intentionally canonicalized to use i32 elements for the benefit of CSE. I'm looking at implementing something where I think I'd like to keep the original vector types. Is this behavior in the type legalizer something that should be changed?
Hi Bob,> LLVM's type legalizer is changing the types of BUILD_VECTORs in a way > that seems wrong to me, but I'm not sure if this is a bug or if some > targets may be relying on it. > > On a 32-bit target, the default action for legalizing i8 and i16 types > is to promote them. If you then have a BUILD_VECTOR to construct a > legal vector type composed of i8 or i16 values, the type legalizer > will look at the BUILD_VECTOR operands and decide that it needs to > promote them to i32 types. You end up with a BUILD_VECTOR that > constructs a vector of i32 values that are then bitcast to the > original vector type. > > This works fine for SSE, where it appears that BUILD_VECTORs are > intentionally canonicalized to use i32 elements for the benefit of > CSE. I'm looking at implementing something where I think I'd like to > keep the original vector types. Is this behavior in the type > legalizer something that should be changed?another way this could be done is to say that the operands of a BUILD_VECTOR don't have to have the same type as the element type of the built vector. Then when the type legalizer sees a v4i16 = BUILD_VECTOR(i16, i16, i16, i16) it can turn this into a v4i16 = BUILD_VECTOR(i32, i32, i32, i32) and it will be happy (all result and operand types are legal). This requires changing the definition of BUILD_VECTOR slightly. Targets will need to understand that only the bottom 16 bits of the operands are to be used, but I doubt that's a problem. Would this solve your problem? Ciao, Duncan. PS: Can you please give a concrete example where the current behavior causes trouble for you?
On Mon, Feb 2, 2009 at 11:31 AM, Bob Wilson <bob.wilson at apple.com> wrote:> LLVM's type legalizer is changing the types of BUILD_VECTORs in a way > that seems wrong to me, but I'm not sure if this is a bug or if some > targets may be relying on it. > > On a 32-bit target, the default action for legalizing i8 and i16 types > is to promote them.This isn't true on x86.> If you then have a BUILD_VECTOR to construct a > legal vector type composed of i8 or i16 values, the type legalizer > will look at the BUILD_VECTOR operands and decide that it needs to > promote them to i32 types. You end up with a BUILD_VECTOR that > constructs a vector of i32 values that are then bitcast to the > original vector type.I'm pretty sure the target-independent legalizer does no such thing. Can you point to the code you're talking about? That said, I do recall the x86 backend does a similar platform-specific transformation with constants... do you care about x86 in particular?> This works fine for SSE, where it appears that BUILD_VECTORs are > intentionally canonicalized to use i32 elements for the benefit of > CSE. I'm looking at implementing something where I think I'd like to > keep the original vector types. Is this behavior in the type > legalizer something that should be changed?I'm not sure what behavior you're talking about... -Eli
On Feb 2, 2009, at 1:51 PM, Eli Friedman wrote:> On Mon, Feb 2, 2009 at 11:31 AM, Bob Wilson <bob.wilson at apple.com> > wrote: >> LLVM's type legalizer is changing the types of BUILD_VECTORs in a way >> that seems wrong to me, but I'm not sure if this is a bug or if some >> targets may be relying on it. >> >> On a 32-bit target, the default action for legalizing i8 and i16 >> types >> is to promote them. > > This isn't true on x86.No, it's not, but it is that way on PowerPC.> >> If you then have a BUILD_VECTOR to construct a >> legal vector type composed of i8 or i16 values, the type legalizer >> will look at the BUILD_VECTOR operands and decide that it needs to >> promote them to i32 types. You end up with a BUILD_VECTOR that >> constructs a vector of i32 values that are then bitcast to the >> original vector type. > > I'm pretty sure the target-independent legalizer does no such thing. > Can you point to the code you're talking about?In DAGTypeLegalizer::run(), the loop after the "ScanOperands" label looks at the operands of the BUILD_VECTOR and the getTypeAction method returns PromoteInteger (because i8 and i16 are promoted to fit in 32- bit registers on PPC).> > > That said, I do recall the x86 backend does a similar > platform-specific transformation with constants... do you care about > x86 in particular? > >> This works fine for SSE, where it appears that BUILD_VECTORs are >> intentionally canonicalized to use i32 elements for the benefit of >> CSE. I'm looking at implementing something where I think I'd like to >> keep the original vector types. Is this behavior in the type >> legalizer something that should be changed? > > I'm not sure what behavior you're talking about...If I have a BUILD_VECTOR that creates a v16i8 type, and v16i8 is a legal type for the target that does not require any promotion or expansion, I would not expect the type legalizer to change it. But, it does. It looks at the i8 elements and decides that they need to be promoted to i32, so it changes the BUILD_VECTOR to create a v4i32 vector, which is then bitcast back to v16i8. I'm not specifically working on SSE right now, I was just explaining why this issue doesn't arise for that target.
On Feb 2, 2009, at 1:22 PM, Duncan Sands wrote:> another way this could be done is to say that the operands of a > BUILD_VECTOR don't have to have the same type as the element type > of the built vector. Then when the type legalizer sees a > v4i16 = BUILD_VECTOR(i16, i16, i16, i16) it can turn this into a > v4i16 = BUILD_VECTOR(i32, i32, i32, i32) and it will be happy > (all result and operand types are legal). This requires changing > the definition of BUILD_VECTOR slightly. Targets will need to > understand that only the bottom 16 bits of the operands are to > be used, but I doubt that's a problem. Would this solve your > problem?That might be good. An alternative would be to leave the current definition of BUILD_VECTOR but change the type legalizer to ignore the BUILD_VECTOR operands. I would prefer the latter. Unless the BUILD_VECTOR is being expanded and the individual element values need to be instantiated separately, I don't see why the element types would need to be legal on their own. I'm new to LLVM so maybe there are some constraints that I don't know about. But, it seems like if a target says that a particular vector type is legal and if it does not lower a BUILD_VECTOR of that type, then nothing more needs to be done to the vector elements.> > PS: Can you please give a concrete example where the current > behavior causes trouble for you?Well, at this point, I've already worked around the immediate issue. Changing it is not urgent. I may or may not run into this again later in the project I am working on. For now I wanted to raise the issue because I found the current behavior surprising and it took me a while to work out why it was happening. Thanks for thinking about it.