[PATCH, PowerPC] Fix invalid displacement created by LocalStackAlloc

[Ulrich Weigand]

Hello,

the recent change to clang to enable dynamic stack reallocation for
over-aligned byval parameters has triggered a pre-existing code-gen bug in
LocalStackAlloc on PowerPC in one of my test cases.  (This has nothing to
do with the dynamic realloc as such, the extra local copies simply enlarged
the stack frame in the test case, so that stack displacements now start
running out of range.)

The bug manifests in a MI instruction with out-of-range displacement after
the LocalStackAlloc pass:
        %vreg17<def> = LD 33184, %vreg31; mem:LD8[%g](align=32)
G8RC:%vreg17 G8RC_and_G8RC_NOX0:%vreg31
(In final assembler output the top bits are stripped off, resulting in a
negative offset loading from below the stack pointer.)

This instruction was created by PPCRegisterInfo::resolveFrameIndex, which
adds an offset to the FI offset already present in the instruction before,
without verifying whether the resulting total offset is still in range.
Now, the LocalStackAlloc common code will actually call another target
routine, PPCRegisterInfo::isFrameOffsetLegal, to let the target decide
whether the replacement is valid.  However, in the PowerPC implementation
of that routine, we only check that whether the offset *delta* is itself in
range, not whether resulting new *total* offset (FI offset already in the
instruction *plus* the delta offset) is in range.

Common code however clearly expects the target to validate the total
offset, and implementations of isFrameOffsetLegal on other targets agree
with that, so it seems this is simply a bug in the PowerPC implementation.

Note that there is another call to isFrameOffsetLegal from *within* the
PowerPC back-end (needsFrameBaseReg), where the *caller* already adds the
instruction offset before calling isFrameOffsetLegal.   When changing
isFrameOffsetLegal to perform this addition itself, that needsFrameBaseReg
must then be adapted as well.

The following patch implements this, which fixes my test case (and passed
regression test & bootstrap).

Is this OK to commit or am I missing something here?

(See attached file: diff-llvm-fix-lsaoffset)


Mit freundlichen Gruessen / Best Regards

Ulrich Weigand

--
  Dr. Ulrich Weigand | Phone: +49-7031/16-3727
  STSM, GNU/Linux compilers and toolchain
  IBM Deutschland Research & Development GmbH
  Vorsitzende des Aufsichtsrats: Martina Koederitz | Geschäftsführung: Dirk
Wittkopp
  Sitz der Gesellschaft: Böblingen | Registergericht: Amtsgericht
Stuttgart, HRB 243294
-------------- next part --------------
A non-text attachment was scrubbed...
Name: diff-llvm-fix-lsaoffset
Type: application/octet-stream
Size: 5253 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140711/8ac75d2c/attachment.obj>