[LLVMbugs] [PATCH] calculateFrameObjectOffsets

[Vladimir Prus]

Chris Lattner wrote:

> @@ -203,26 +202,51 @@
> -  // Check to see if there are any fixed sized objects that are
> preallocated in -  // the local area.  We currently don't support filling
> in holes in between -  // fixed sized objects, so we just skip to the end
> of the last fixed sized +  if (StackGrowsDown)
> +      Offset = -Offset;
> +  assert(Offset >= 0);
>
> Please put a string into the assertion (assert(Offset >= 0 && "foo")) so
> that we get a potentially useful indication of what when wrong if this
> ever triggers.

Done.

>    for (int i = FFI->getObjectIndexBegin(); i != 0; ++i) {
> -    int FixedOff = -FFI->getObjectOffset(i);
> -    if (FixedOff > Offset) Offset = FixedOff;
> +    int FixedOff;
> +    if (StackGrowsDown) {
> +        // The maximum distance from the stack pointer is the lower
> address of +        // the object -- which is exactly its offset.
> +        FixedOff = FFI->getObjectOffset(i);
>
> Okay, it looks like you are missing a - in the StackGrowsDown case.
> Before FixedOff was -FFI->getObjectOffset(i), now it's not.  Is this
> intentional?

No. I've removed the negation to see if this will break any test and did not 
put it back. (Besides, the "Olden" test seem to work fine anyway) -- most 
likely because fixed object in local area are somewhat rare case.

> +    } else {
> +        // The maximim distance from the stach pointer is the upper
> address of +        // the object.
>
> 2 spello's.

Fixed.

> Otherwise it looks good.  Resend and we'd be happy to apply it for you,
> thanks!!

Here's the revised version.

Thanks,
Volodya
-------------- next part --------------
A non-text attachment was scrubbed...
Name: PrologueEpilogue.diff
Type: text/x-diff
Size: 5354 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-bugs/attachments/20040610/c7c2fb10/attachment.diff>