[llvm-commits] [llvm] r165726 - in /llvm/trunk: include/llvm-c/ include/llvm/ include/llvm/Target/ include/llvm/Transforms/Utils/ lib/Analysis/ lib/CodeGen/ lib/CodeGen/AsmPrinter/ lib/CodeGen/SelectionDAG/ lib/ExecutionEngine/ lib/ExecutionEngin

[Villmow, Micah]

> -----Original Message-----
> From: llvm-commits-bounces at cs.uiuc.edu [mailto:llvm-commits-
> bounces at cs.uiuc.edu] On Behalf Of Sean Silva
> Sent: Thursday, October 11, 2012 3:14 PM
> To: Chris Lattner
> Cc: llvm-commits at cs.uiuc.edu; Micah Villmow
> Subject: Re: [llvm-commits] [llvm] r165726 - in /llvm/trunk:
> include/llvm-c/ include/llvm/ include/llvm/Target/
> include/llvm/Transforms/Utils/ lib/Analysis/ lib/CodeGen/
> lib/CodeGen/AsmPrinter/ lib/CodeGen/SelectionDAG/ lib/ExecutionEngine/
> lib/ExecutionEngin
> 
> >     case Instruction::PtrToInt: {
> >       GenericValue GV = getConstantValue(Op0);
> >       unsigned AS = cast<PtrToIntInst>(CE)->getPointerAddressSpace();
> >       uint32_t PtrWidth = TD->getPointerSizeInBits(AS);
> >       GV.IntVal = APInt(PtrWidth, uintptr_t(GV.PointerVal));
> >
> > This is completely incorrect, because "CE" is a ConstantExpr, not an
> Instruction.  This error is repeated in several other places.  Please
> fix soon or revert the patch.
> 
> This is very interesting. I would have expected this to fail to
> compile. On a closer investigation, it looks like this is going
> through `PtrToIntInst::classof(const Value *V)`, which is definitely
> undesirable. I'll investigate if there is a way to turn this into a
> compile-time error.
[Villmow, Micah] Yeah, that would be very useful. :)
> 
> 
> -- Sean Silva
> 
> On Thu, Oct 11, 2012 at 5:19 PM, Chris Lattner <clattner at apple.com>
> wrote:
> >
> > On Oct 11, 2012, at 10:21 AM, Micah Villmow <villmow at gmail.com>
> wrote:
> >
> >> Author: mvillmow
> >> Date: Thu Oct 11 12:21:41 2012
> >> New Revision: 165726
> >>
> >> URL: http://llvm.org/viewvc/llvm-project?rev=165726&view=rev
> >> Log:
> >> Add in the first iteration of support for llvm/clang/lldb to allow
> variable per address space pointer sizes to be optimized correctly.
> >
> > Hi Micah,
> >
> > You've added this code to ExecutionEngine.cpp:
> >
> >     case Instruction::PtrToInt: {
> >       GenericValue GV = getConstantValue(Op0);
> >       unsigned AS = cast<PtrToIntInst>(CE)->getPointerAddressSpace();
> >       uint32_t PtrWidth = TD->getPointerSizeInBits(AS);
> >       GV.IntVal = APInt(PtrWidth, uintptr_t(GV.PointerVal));
> >
> > This is completely incorrect, because "CE" is a ConstantExpr, not an
> Instruction.  This error is repeated in several other places.  Please
> fix soon or revert the patch.
> >
> > Was this patch reviewed?
> >
> > -Chris
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits