[llvm-commits] [llvm] r136759 - in /llvm/trunk: lib/CodeGen/AsmPrinter/DwarfDebug.cpp test/CodeGen/ARM/2011-08-02-MergedGlobalDbg.ll

[Bob Wilson]

See comments below.  I'm about to commit a revised version of this.

On Aug 2, 2011, at 6:25 PM, Devang Patel wrote:

> Author: dpatel
> Date: Tue Aug  2 20:25:46 2011
> New Revision: 136759
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=136759&view=rev
> Log:
> Use byte offset, instead of element number, to access merged global.
> 
> Added:
>    llvm/trunk/test/CodeGen/ARM/2011-08-02-MergedGlobalDbg.ll
> Modified:
>    llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
> 
> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp?rev=136759&r1=136758&r2=136759&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp (original)
> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp Tue Aug  2 20:25:46 2011
> @@ -955,7 +955,8 @@
> static const ConstantExpr *getMergedGlobalExpr(const Value *V) {
>   const ConstantExpr *CE = dyn_cast_or_null<ConstantExpr>(V);
>   if (!CE || CE->getNumOperands() != 3 ||
> -      CE->getOpcode() != Instruction::GetElementPtr)
> +      CE->getOpcode() != Instruction::GetElementPtr || 
> +      !isa<PointerType>(CE->getOperand(0)->getType()))

This is checking the wrong thing.  The first operand of a GEP is always a pointer, and there's already a check that it's a GlobalValue.  What needs to be checked here is that it's a StructType because you're assuming that below.

>     return NULL;
> 
>   // First operand points to a global value.
> @@ -974,6 +975,23 @@
>   return CE;
> }
> 
> +// getMergedGlobalElementOffset - If CE is accessing a merged global
> +// then find byte offset of the element accessed by CE. This must be
> +// used only CE returned by getMergedGlobalExpr(). See above.
> +static uint64_t getMergedGlobalElementOffset(const TargetData &TD,
> +                                             const ConstantExpr *CE) {
> +  assert (getMergedGlobalExpr(CE) && "This is not a merged global!");
> +  uint64_t e = cast<ConstantInt>(CE->getOperand(2))->getZExtValue();
> +  if (e == 0) return 0;
> +
> +  uint64_t Offset = 0;
> +  const PointerType *PTy = dyn_cast<PointerType>(CE->getOperand(0)->getType());
> +  const StructType *STy = dyn_cast<StructType>(PTy->getElementType());

These should be casts, not dyn_casts.  You're not checking if the dyn_casts succeed anyway.

> +  for (uint64_t i = 0; i != e; ++i)
> +    Offset += TD.getTypeAllocSize(STy->getElementType(i));
> +  return Offset;
> +}

This should use TD.getIndexedOffset().

Again, I'm just going to make these changes myself, so no need to respond -- unless you disagree, of course.