[llvm] r212832 - [PowerPC] Fix invalid displacement created by LocalStackAlloc

Ulrich Weigand ulrich.weigand at de.ibm.com
Fri Jul 11 10:19:31 PDT 2014


Author: uweigand
Date: Fri Jul 11 12:19:31 2014
New Revision: 212832

URL: http://llvm.org/viewvc/llvm-project?rev=212832&view=rev
Log:
[PowerPC] Fix invalid displacement created by LocalStackAlloc

This commit fixes a bug in PPCRegisterInfo::isFrameOffsetLegal that
could result in the LocalStackAlloc pass creating an MI instruction
out-of-range displacement:
        %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.)

Common code expects the isFrameOffsetLegal routine to verify whether
adding a given offset to the offset already present in the instruction
results in a valid displacement.  However, on PowerPC the routine
did not take the already present instruction offset into account.

This commit fixes isFrameOffsetLegal to add the instruction offset,
and updates a local caller (needsFrameBaseReg) to no longer add the
instruction offset itself before calling isFrameOffsetLegal.

Reviewed by Hal Finkel.


Added:
    llvm/trunk/test/CodeGen/PowerPC/resolvefi-disp.ll
Modified:
    llvm/trunk/lib/Target/PowerPC/PPCRegisterInfo.cpp

Modified: llvm/trunk/lib/Target/PowerPC/PPCRegisterInfo.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/PowerPC/PPCRegisterInfo.cpp?rev=212832&r1=212831&r2=212832&view=diff
==============================================================================
--- llvm/trunk/lib/Target/PowerPC/PPCRegisterInfo.cpp (original)
+++ llvm/trunk/lib/Target/PowerPC/PPCRegisterInfo.cpp Fri Jul 11 12:19:31 2014
@@ -885,16 +885,6 @@ bool PPCRegisterInfo::
 needsFrameBaseReg(MachineInstr *MI, int64_t Offset) const {
   assert(Offset < 0 && "Local offset must be negative");
 
-  unsigned FIOperandNum = 0;
-  while (!MI->getOperand(FIOperandNum).isFI()) {
-    ++FIOperandNum;
-    assert(FIOperandNum < MI->getNumOperands() &&
-           "Instr doesn't have FrameIndex operand!");
-  }
-
-  unsigned OffsetOperandNo = getOffsetONFromFION(*MI, FIOperandNum);
-  Offset += MI->getOperand(OffsetOperandNo).getImm();
-
   // It's the load/store FI references that cause issues, as it can be difficult
   // to materialize the offset if it won't fit in the literal field. Estimate
   // based on the size of the local frame and some conservative assumptions
@@ -985,6 +975,16 @@ void PPCRegisterInfo::resolveFrameIndex(
 
 bool PPCRegisterInfo::isFrameOffsetLegal(const MachineInstr *MI,
                                          int64_t Offset) const {
+  unsigned FIOperandNum = 0;
+  while (!MI->getOperand(FIOperandNum).isFI()) {
+    ++FIOperandNum;
+    assert(FIOperandNum < MI->getNumOperands() &&
+           "Instr doesn't have FrameIndex operand!");
+  }
+
+  unsigned OffsetOperandNo = getOffsetONFromFION(*MI, FIOperandNum);
+  Offset += MI->getOperand(OffsetOperandNo).getImm();
+
   return MI->getOpcode() == PPC::DBG_VALUE || // DBG_VALUE is always Reg+Imm
          (isInt<16>(Offset) && (!usesIXAddr(*MI) || (Offset & 3) == 0));
 }

Added: llvm/trunk/test/CodeGen/PowerPC/resolvefi-disp.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/PowerPC/resolvefi-disp.ll?rev=212832&view=auto
==============================================================================
--- llvm/trunk/test/CodeGen/PowerPC/resolvefi-disp.ll (added)
+++ llvm/trunk/test/CodeGen/PowerPC/resolvefi-disp.ll Fri Jul 11 12:19:31 2014
@@ -0,0 +1,71 @@
+; RUN: llc -mtriple=powerpc64-unknown-linux-gnu -mcpu=pwr7 -print-after=localstackalloc <%s >%t 2>&1 && FileCheck <%t %s
+
+; Due to a bug in isFrameOffsetLegal we ended up with resolveFrameIndex creating
+; addresses with out-of-range displacements.  Verify that this no longer happens.
+; CHECK-NOT: LD {{3276[8-9]}}
+; CHECK-NOT: LD {{327[7-9][0-9]}}
+; CHECK-NOT: LD {{32[8-9][0-9][0-9]}}
+; CHECK-NOT: LD {{3[3-9][0-9][0-9][0-9]}}
+; CHECK-NOT: LD {{[4-9][0-9][0-9][0-9][0-9]}}
+; CHECK-NOT: LD {{[1-9][0-9][0-9][0-9][0-9][0-9]+}}
+
+target datalayout = "e-m:e-i64:64-n32:64"
+target triple = "powerpc64le-unknown-linux-gnu"
+
+%struct.S2760 = type { <2 x float>, %struct.anon, i32, [28 x i8] }
+%struct.anon = type { [11 x %struct.anon.0], i64, [6 x { i64, i64 }], [24 x i8] }
+%struct.anon.0 = type { [30 x %union.U4DI], i8, [0 x i16], [30 x i8] }
+%union.U4DI = type { <4 x i64> }
+
+ at s2760 = external global %struct.S2760
+ at fails = external global i32
+
+define void @check2760(%struct.S2760* noalias sret %agg.result, %struct.S2760* byval align 16, %struct.S2760* %arg1, %struct.S2760* byval align 16) {
+entry:
+  %arg0 = alloca %struct.S2760, align 32
+  %arg2 = alloca %struct.S2760, align 32
+  %arg1.addr = alloca %struct.S2760*, align 8
+  %ret = alloca %struct.S2760, align 32
+  %b1 = alloca %struct.S2760, align 32
+  %b2 = alloca %struct.S2760, align 32
+  %2 = bitcast %struct.S2760* %arg0 to i8*
+  %3 = bitcast %struct.S2760* %0 to i8*
+  call void @llvm.memcpy.p0i8.p0i8.i64(i8* %2, i8* %3, i64 11104, i32 16, i1 false)
+  %4 = bitcast %struct.S2760* %arg2 to i8*
+  %5 = bitcast %struct.S2760* %1 to i8*
+  call void @llvm.memcpy.p0i8.p0i8.i64(i8* %4, i8* %5, i64 11104, i32 16, i1 false)
+  store %struct.S2760* %arg1, %struct.S2760** %arg1.addr, align 8
+  %6 = bitcast %struct.S2760* %ret to i8*
+  call void @llvm.memset.p0i8.i64(i8* %6, i8 0, i64 11104, i32 32, i1 false)
+  %7 = bitcast %struct.S2760* %b1 to i8*
+  call void @llvm.memset.p0i8.i64(i8* %7, i8 0, i64 11104, i32 32, i1 false)
+  %8 = bitcast %struct.S2760* %b2 to i8*
+  call void @llvm.memset.p0i8.i64(i8* %8, i8 0, i64 11104, i32 32, i1 false)
+  %b = getelementptr inbounds %struct.S2760* %arg0, i32 0, i32 1
+  %g = getelementptr inbounds %struct.anon* %b, i32 0, i32 1
+  %9 = load i64* %g, align 8
+  %10 = load i64* getelementptr inbounds (%struct.S2760* @s2760, i32 0, i32 1, i32 1), align 8
+  %cmp = icmp ne i64 %9, %10
+  br i1 %cmp, label %if.then, label %if.end
+
+if.then:                                          ; preds = %entry
+  %11 = load i32* @fails, align 4
+  %inc = add nsw i32 %11, 1
+  store i32 %inc, i32* @fails, align 4
+  br label %if.end
+
+if.end:                                           ; preds = %if.then, %entry
+  %12 = load i64* getelementptr inbounds (%struct.S2760* @s2760, i32 0, i32 1, i32 1), align 8
+  %b3 = getelementptr inbounds %struct.S2760* %ret, i32 0, i32 1
+  %g4 = getelementptr inbounds %struct.anon* %b3, i32 0, i32 1
+  store i64 %12, i64* %g4, align 8
+  %13 = bitcast %struct.S2760* %agg.result to i8*
+  %14 = bitcast %struct.S2760* %ret to i8*
+  call void @llvm.memcpy.p0i8.p0i8.i64(i8* %13, i8* %14, i64 11104, i32 32, i1 false)
+  ret void
+}
+
+declare void @llvm.memcpy.p0i8.p0i8.i64(i8* nocapture, i8* nocapture readonly, i64, i32, i1)
+
+declare void @llvm.memset.p0i8.i64(i8* nocapture, i8, i64, i32, i1)
+





More information about the llvm-commits mailing list