llvm dev - Apr 2007 - [LLVMdev] llvm-gcc Bug, Looking for Advice on Fix

Ok, I've tracked down the problem I've had bootstrapping
llvm-gcc.  The culprit is in TreeToLLVM::EmitMemCpy:

void TreeToLLVM::EmitMemCpy(Value *DestPtr, Value *SrcPtr, Value *Size,
                             unsigned Align) {
   const Type *SBP = PointerType::get(Type::Int8Ty);
   const Type *IntPtr = TD.getIntPtrType();
   Value *Ops[4] = {
     CastToType(Instruction::BitCast, DestPtr, SBP),
     CastToType(Instruction::BitCast, SrcPtr, SBP),
     CastToSIntType(Size, IntPtr),
     ConstantInt::get(Type::Int32Ty, Align)
   };

   new CallInst(Intrinsic::getDeclaration(TheModule,
                                          (IntPtr == Type::Int32Ty) ?
                                          Intrinsic::memcpy_i32 :
                                          Intrinsic::memcpy_i64),
	       Ops, 4, "", CurBB);
}

The problem is that Intrinsic::getDeclaration takes four
parameters but is only passed two:

Function *Intrinsic::getDeclaration(Module *M, ID id, const Type **Tys,
                                     unsigned numTys)

So it sees bogus data from Tys and numTys.  This probably works
in the official sources because the "Ops, 4" parameters to CallInst
just happen to be pushed on the stack in just the right place
for Intrinsic::getDeclaration.  But apparently our changes to llvm
here upset that delicate balance.

So the obvious fix is this:

   new CallInst(Intrinsic::getDeclaration(TheModule,
                                          (IntPtr == Type::Int32Ty) ?
                                          Intrinsic::memcpy_i32 :
                                          Intrinsic::memcpy_i64,
					 Ops, 4),
	       "", CurBB);

But when I try to build I get:

compiler/llvm-gcc/gcc/llvm-convert.cpp:1203: error: cannot convert 
'llvm::Value**' to 'const llvm::Type**' for argument '3'
to
'llvm::Function* llvm::Intrinsic::getDeclaration(llvm::Module*, 
llvm::Intrinsic::ID, const llvm::Type**, unsigned int)'

Ok, that makes sense.  It's an illegal conversion.  So my question is,
what's the right fix?  The current code uses this CallInst constructor:

CallInst::CallInst(Value *Func, Value* const *Args, unsigned NumArgs,
                    const std::string &Name, BasicBlock *InsertAtEnd)

What's the intent of the TreeToLLVM::EmitMemCpy code?  I don't know
enough about the gcc frontend to be sure.

I'll submit a patch as soon as I better understand what's being done
here.

                                       -Dave

David Greene wrote:
> The problem is that Intrinsic::getDeclaration takes four
> parameters but is only passed two:
> 
> Function *Intrinsic::getDeclaration(Module *M, ID id, const Type **Tys,
>                                      unsigned numTys)
It turns out that this happens all over llvm-convert.c.  Tys and numTys
default to zero, which is why it builds.  The problem is, getDeclaration
calls getType(id, Tys, numTys) where this code is executed:

   case Intrinsic::memcpy_i32:		// llvm.memcpy.i32
   case Intrinsic::memmove_i32:		// llvm.memmove.i32
     ResultTy = Type::getPrimitiveType(Type::VoidTyID);
     ArgTys.push_back(Tys[1]);
     ArgTys.push_back(Tys[2]);
     ArgTys.push_back(IntegerType::get(32));
     ArgTys.push_back(IntegerType::get(32));

Oops, Tys is 0!

Similar code exists for llvm.memcpy.i64, so it's not the case that
the select expression returned the "wrong" answer.

How the heck did this ever work?

                                         -Dave

David Greene wrote:
> How the heck did this ever work?
The answer is that it worked because someone here (who shall remain
nameless  :)) updated our llvm intrinsics without updating llvm-gcc.

Actually, it was the right thing to do -- I just didn't know it had
been done.  So now that this question is answered I can go off and
generate some patches.

                                -Dave