Hi, I ran into a case where ScalarReplAggregates can not promote an array of vectors into registers, e..g, %a = alloca [8 x <2 x float>], align 8 %arrayidx74597 = getelementptr [8 x <2 x float>]* %a, i32 0, i32 1, i32 0 ; <float*> [#uses=2] %tmp76 = load float* %arrayidx74597, align 8 %arrayidx74598 = getelementptr [8 x <2 x float>]* %a, i32 0, i32 1, i32 1 ; <float*> [#uses=2] %tmp77 = load float* %arrayidx74598, align 4 Though we could change the algorithm to look through the vector index, it lead to an interesting question about if we should allow a getelementptr (GEP) into a vector. A vector is not like an short array of elements. It is more of an entity in itself. When dealing with vectors, it seems to me that it is cleaner to think of them as an entity and GEP to the vector and then use extract element to access the element of the vector instead of using a GEP to get a pointer into a vector and loading it directly. If a client wants to do this, it would seem cleaner to force the client to bitcast it to an array and then do a GEP to the element. This would it more similar to what clients have to do to get to a pointer to a byte in a word today. For ScalarReplAggr, we would want to promote the vector itself and not to treat it as promoting the float elements. I would like to make it illegal to GEP into a vector as I think it is cleaner and more consistent. Opinions? Comments? -- Mon Ping
On Oct 14, 2008, at 9:50 AM, Mon Ping Wang wrote:> Hi, > > I ran into a case where ScalarReplAggregates can not promote an array > of vectors into registers, e..g, > %a = alloca [8 x <2 x float>], align 8 > %arrayidx74597 = getelementptr [8 x <2 x float>]* %a, i32 0, > i32 1, i32 0 ; <float*> [#uses=2] > %tmp76 = load float* %arrayidx74597, align 8 > %arrayidx74598 = getelementptr [8 x <2 x float>]* %a, i32 0, > i32 1, i32 1 ; <float*> [#uses=2] > %tmp77 = load float* %arrayidx74598, align 4 > > Though we could change the algorithm to look through the vector index, > it lead to an interesting question about if we should allow a > getelementptr (GEP) into a vector. > > A vector is not like an short array of elements. It is more of an > entity in itself. When dealing with vectors, it seems to me that it > is cleaner to think of them as an entity and GEP to the vector and > then use extract element to access the element of the vector instead > of using a GEP to get a pointer into a vector and loading it > directly. If a client wants to do this, it would seem cleaner to > force the client to bitcast it to an array and then do a GEP to the > element. This would it more similar to what clients have to do to get > to a pointer to a byte in a word today. For ScalarReplAggr, we would > want to promote the vector itself and not to treat it as promoting the > float elements. > > I would like to make it illegal to GEP into a vector as I think it is > cleaner and more consistent. Opinions? Comments?I completely agree with you. Our current approach is similar to allowing GEP to index into the 3rd byte of an i32 for example. -Chris
Hi Mon Ping,> I would like to make it illegal to GEP into a vector as I think it is > cleaner and more consistent. Opinions? Comments?now that arrays are first class types, I think vectors should become a subclass of ArrayType. This would get rid of a lot of duplicated code, and also fix a bunch of problems with vectors (eg: that sizes are wrong; currently we think that a <n x i1> has length n bits, and that a vector <n x x86_fp80> has length 80*n bits, both of which are untenable). From this point of view you have to allow GEP into a vector; the conclusion I suppose is that codegen needs to replace GEP+load or GEP+store with an extract or insert operation. Ciao, Duncan.
On Oct 14, 2008, at 9:50 AM, Mon Ping Wang wrote:> Hi, > > I ran into a case where ScalarReplAggregates can not promote an array > of vectors into registers, e..g, > %a = alloca [8 x <2 x float>], align 8 > %arrayidx74597 = getelementptr [8 x <2 x float>]* %a, i32 0, > i32 1, i32 0 ; <float*> [#uses=2] > %tmp76 = load float* %arrayidx74597, align 8 > %arrayidx74598 = getelementptr [8 x <2 x float>]* %a, i32 0, > i32 1, i32 1 ; <float*> [#uses=2] > %tmp77 = load float* %arrayidx74598, align 4 > > Though we could change the algorithm to look through the vector index, > it lead to an interesting question about if we should allow a > getelementptr (GEP) into a vector. > > A vector is not like an short array of elements. It is more of an > entity in itself. When dealing with vectors, it seems to me that it > is cleaner to think of them as an entity and GEP to the vector and > then use extract element to access the element of the vector instead > of using a GEP to get a pointer into a vector and loading it > directly. If a client wants to do this, it would seem cleaner to > force the client to bitcast it to an array and then do a GEP to the > element. This would it more similar to what clients have to do to get > to a pointer to a byte in a word today. For ScalarReplAggr, we would > want to promote the vector itself and not to treat it as promoting the > float elements. > > I would like to make it illegal to GEP into a vector as I think it is > cleaner and more consistent. Opinions? Comments?I agree that disallowing GEP from indexing into vector elements seems a little cleaner and more consistent. I have used GEPs to index into vectors once (in code not in the LLVM tree), but I'm pretty sure that code could be rewritten to avoid needing that. It's currently illegal to bitcast directly to or from an array type though, so the workaround for people who need to index into vector elements with a GEP is to bitcast the GEP's pointer operand. Dan
On Oct 14, 2008, at 11:02 AM, Duncan Sands wrote:> Hi Mon Ping, > >> I would like to make it illegal to GEP into a vector as I think it is >> cleaner and more consistent. Opinions? Comments? > > now that arrays are first class types, I think vectors should become > a subclass of ArrayType. This would get rid of a lot of duplicated > code, and also fix a bunch of problems with vectors (eg: that sizes > are wrong; currently we think that a <n x i1> has length n bits, and > that a vector <n x x86_fp80> has length 80*n bits, both of which are > untenable).I'm happy about factoring the code better, but a vectortype isn't an arraytype (isa<ArrayType>(V) should be false). Maybe a common base class (like sequential type) would be better?> From this point of view you have to allow GEP into a > vector; the conclusion I suppose is that codegen needs to replace > GEP+load or GEP+store with an extract or insert operation.With that logic, there is no difference at all between an array and vector... I disagree very strongly about this. -Chris
Hi, I didn't know that about bitcast. This becomes important if someone wants a pointer to a vector element as we would either need to support the bistcast or we just don't allow that. For most vector code that I'm aware of, no one really wants to take an address of vector element as by doing so, one is kind of scalarizing the vector which would be bad for performance. If someone has a case where this is useful, I would like to see it as it make help us make a better decision in this area. -- Mon Ping On Oct 14, 2008, at 11:38 AM, Dan Gohman wrote:> > On Oct 14, 2008, at 9:50 AM, Mon Ping Wang wrote: > >> Hi, >> >> I ran into a case where ScalarReplAggregates can not promote an array >> of vectors into registers, e..g, >> %a = alloca [8 x <2 x float>], align 8 >> %arrayidx74597 = getelementptr [8 x <2 x float>]* %a, i32 0, >> i32 1, i32 0 ; <float*> [#uses=2] >> %tmp76 = load float* %arrayidx74597, align 8 >> %arrayidx74598 = getelementptr [8 x <2 x float>]* %a, i32 0, >> i32 1, i32 1 ; <float*> [#uses=2] >> %tmp77 = load float* %arrayidx74598, align 4 >> >> Though we could change the algorithm to look through the vector >> index, >> it lead to an interesting question about if we should allow a >> getelementptr (GEP) into a vector. >> >> A vector is not like an short array of elements. It is more of an >> entity in itself. When dealing with vectors, it seems to me that it >> is cleaner to think of them as an entity and GEP to the vector and >> then use extract element to access the element of the vector instead >> of using a GEP to get a pointer into a vector and loading it >> directly. If a client wants to do this, it would seem cleaner to >> force the client to bitcast it to an array and then do a GEP to the >> element. This would it more similar to what clients have to do to >> get >> to a pointer to a byte in a word today. For ScalarReplAggr, we would >> want to promote the vector itself and not to treat it as promoting >> the >> float elements. >> >> I would like to make it illegal to GEP into a vector as I think it is >> cleaner and more consistent. Opinions? Comments? > > I agree that disallowing GEP from indexing into vector elements seems > a little cleaner and more consistent. I have used GEPs to index into > vectors once (in code not in the LLVM tree), but I'm pretty sure that > code could be rewritten to avoid needing that. > > It's currently illegal to bitcast directly to or from an array type > though, so the workaround for people who need to index into vector > elements with a GEP is to bitcast the GEP's pointer operand. > > Dan > > _______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev