On Friday 04 April 2008 13:07, Chris Lattner wrote:> > So how does the undef store to null appear in the IR when it isn't > > attached anywhere and how can I get rid of it? > > Don't do undefined behavior? :)I don't think it's undefined behavior. Right before instcombine, we have this: %r60 = load <2 x i64>* %"$LCS_1", align 16 ; <<2 x i64>> [#uses=2] ; srcLine 41 %r61 = extractelement <2 x i64> %r60, i32 0 ; <i64> [#uses=1] ; srcLine 41 %r62 = getelementptr <2 x double>* null, i32 0, i64 %r61 ; <double*> [#uses=1] ; srcLine 41 %r63 = load double* %r62 ; <double> [#uses=1] ; srcLine 41 So we're loading a vector of pointers and then using getelementptr as basically a reg-reg copy with a cast (I think). Yes, it's a little weird, but legal AFAICS. I'm not sure if the above code is really kosher. It is very strange to see the typing of the extractelement and GEP here. But instcombine only looks at the first operand to GEP, sees it's null and just assumes it's useless code. Not the case in this example. So I think this is actually a bug in instcombine and maybe elsewhere as well. What do you think? -Dave
On Friday 04 April 2008 13:42, David Greene wrote:> On Friday 04 April 2008 13:07, Chris Lattner wrote: > > > So how does the undef store to null appear in the IR when it isn't > > > attached anywhere and how can I get rid of it? > > > > Don't do undefined behavior? :) > > I don't think it's undefined behavior. Right before instcombine, we have > this: > > %r60 = load <2 x i64>* %"$LCS_1", align 16 ; <<2 x i64>> [#uses=2] ; > srcLine 41 > %r61 = extractelement <2 x i64> %r60, i32 0 ; <i64> [#uses=1] ; srcLine > 41 %r62 = getelementptr <2 x double>* null, i32 0, i64 %r61 ; <double*> > [#uses=1] ; srcLine 41 > %r63 = load double* %r62 ; <double> [#uses=1] ; srcLine 41 > > So we're loading a vector of pointers and then using getelementptr as > basically a reg-reg copy with a cast (I think). Yes, it's a little weird, > but legal AFAICS. > > I'm not sure if the above code is really kosher. It is very strange to see > the typing of the extractelement and GEP here.Actually, there are multiple problems. The first zero index on the GEP is wrong. I believe that it should be the following: %r62 = getelementptr <2 x double>* null, i64 %r61 This is essentially the equivalent of a bitcast, then. The second problem is the typing of the GEP and the following load. Clearly that's wrong. The GEP should return a double *. These are problems in our compiler, not LLVM. But I do want to ask about the legality of the revised GEP above. Is that legal code? If so, instcombine should not be removing it as dead code. -Dave
On Fri, 4 Apr 2008, David Greene wrote:>> So we're loading a vector of pointers and then using getelementptr as >> basically a reg-reg copy with a cast (I think). Yes, it's a little weird, >> but legal AFAICS. >> >> I'm not sure if the above code is really kosher. It is very strange to see >> the typing of the extractelement and GEP here. > > Actually, there are multiple problems. The first zero index on the GEP is > wrong. I believe that it should be the following: > > %r62 = getelementptr <2 x double>* null, i64 %r61 > > This is essentially the equivalent of a bitcast, then.Right. I guess it would be acceptable to turn into into an inttoptr instruction.> These are problems in our compiler, not LLVM. But I do want to ask > about the legality of the revised GEP above. Is that legal code? If so, > instcombine should not be removing it as dead code.I don't think it's really legal, GEP shouldn't be used that way. -Chris -- http://nondot.org/sabre/ http://llvm.org/