I am confused by this bit of code in instcombine:
09789 if (GetElementPtrInst *GEPI = dyn_cast<GetElementPtrInst>(Op)) {
09790 const Value *GEPI0 = GEPI->getOperand(0);
09791 // TODO: Consider a target hook for valid address spaces for this
xform.
09792 if (isa<ConstantPointerNull>(GEPI0) &&
09793 cast<PointerType>(GEPI0->getType())->getAddressSpace() == 0) {
09794 // Insert a new store to null instruction before the load to
indicate
09795 // that this code is not reachable. We do this instead of
inserting
09796 // an unreachable instruction directly because we cannot modify
the
09797 // CFG.
09798 new StoreInst(UndefValue::get(LI.getType()),
09799 Constant::getNullValue(Op->getType()), &LI);
09800 return ReplaceInstUsesWith(LI, UndefValue::get(LI.getType()));
09801 }
09802 }
First, what happens to the StoreInst? It looks like it is not attached
anywhere.
Second, I am seeing this happen in a block that is definitely reachable.
Later on the null GEP is removed because it isInstructionTriviallyDead.
But the undef store to null remains since the block is in fact reachable.
This is obviously bad.
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?
What is this code trying to do, anyway?
-Dave
I am confused by this bit of code in instcombine:
09789 if (GetElementPtrInst *GEPI = dyn_cast<GetElementPtrInst>(Op)) {
09790 const Value *GEPI0 = GEPI->getOperand(0);
09791 // TODO: Consider a target hook for valid address spaces for this
xform.
09792 if (isa<ConstantPointerNull>(GEPI0) &&
09793 cast<PointerType>(GEPI0->getType())->getAddressSpace() == 0) {
09794 // Insert a new store to null instruction before the load to
indicate
09795 // that this code is not reachable. We do this instead of
inserting
09796 // an unreachable instruction directly because we cannot modify
the
09797 // CFG.
09798 new StoreInst(UndefValue::get(LI.getType()),
09799 Constant::getNullValue(Op->getType()), &LI);
09800 return ReplaceInstUsesWith(LI, UndefValue::get(LI.getType()));
09801 }
09802 }
First, what happens to the StoreInst? It looks like it is not attached
anywhere.
The &LI argument causes it to be inserted before the load.
Second, I am seeing this happen in a block that is definitely reachable.
Later on the null GEP is removed because it isInstructionTriviallyDead.
But the undef store to null remains since the block is in fact reachable.
This is obviously bad.
This xform doesn't have anything to do with block reachability. It effectively transforms "load from null pointer" into an unreachable instruction. Load from null pointer is undefined, so this is a safe xform regardless of where it occurs.
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? 
-Chris
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
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
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