[mips] Generate memory dependencies for byVal arguments

[smaksimovic]

Summary

There were no memory dependencies made between stores generated when lowering formal arguments and loads generated when call lowering byVal arguments which made the Post-RA scheduler place a load before a matching store.

• Make the fixed object stored to mutable so that the load instructions can have their memory dependencies added
• Set the frame object as isAliased which clears the underlying objects vector in ScheduleDAGInstrs::buildSchedGraph(). This results in addition of all stores as dependenies for loads.

This problem appeared when passing a byVal parameter coupled with a fastcc function call.

[smaksimovic]

Modified the test file to check only for the one offending SW/LW pair. The previous version was incorrect and it seemed difficult to match all the stores in arbitrary order followed by all loads also occurring arbitrarily.

[sdardis]

test/CodeGen/Mips/fastcc_byval.ll

1 ↗

(On Diff #114004)

-filetype=asm is unneeded here.

[smaksimovic]

The previous version was found to cause problems elsewhere, the reason being different underlying objects which resulted in wrong ordering of two instructions.
This way we can keep the parameters in MachinePointerInfo() for the stores that are created in MipsTargetLowering::copyByValRegs().
Setting the frame object as isAliased clears the underlying objects vector in ScheduleDAGInstrs::buildSchedGraph() resulting in addition of all stores as dependenies for loads. The desired dependency is there now, the problem being the other ones which are not are there as well.

[sdardis]

LGTM. Add a comment to test file, describing the purpose of the test.

lib/Target/Mips/MipsISelLowering.cpp

4105 ↗

(On Diff #116172)

Expand the comment here on why we're marking the object as mutable + isAliased.

llvm/trunk/lib/Target/Mips/MipsISelLowering.cpp

if (RegAreaSize)
     FrameObjOffset =
         (int)ABI.GetCalleeAllocdArgSizeInBytes(State.getCallingConv()) -
         (int)((ByValArgRegs.size() - FirstReg) * GPRSizeInBytes);
   else
     FrameObjOffset = VA.getLocMemOffset();
 
   // Create frame object.
   EVT PtrTy = getPointerTy(DAG.getDataLayout());
-  int FI = MFI.CreateFixedObject(FrameObjSize, FrameObjOffset, true);
+  // Make the fixed object stored to mutable so that the load instructions
+  // referencing it have their memory dependencies added.
+  // Set the frame object as isAliased which clears the underlying objects
+  // vector in ScheduleDAGInstrs::buildSchedGraph() resulting in addition of all
+  // stores as dependencies for loads referencing this fixed object.
+  int FI = MFI.CreateFixedObject(FrameObjSize, FrameObjOffset, false, true);
   SDValue FIN = DAG.getFrameIndex(FI, PtrTy);
   InVals.push_back(FIN);
 
   if (!NumRegs)
     return;
 
   // Copy arg registers.
   MVT RegTy = MVT::getIntegerVT(GPRSizeInBytes * 8);

llvm/trunk/test/CodeGen/Mips/fastcc_byval.ll

+; RUN: llc -mtriple=mipsel-linux-gnu -O3 -relocation-model=pic < %s | FileCheck %s
+
+; Test that a load comes after a store to the same memory location when passing
+; a byVal parameter to a function which has a fastcc function call
+
+%struct.str = type { i32, i32, [3 x i32*] }
+
+declare fastcc void @_Z1F3str(%struct.str* noalias nocapture sret %agg.result, %struct.str* byval nocapture readonly align 4 %s)
+
+define i32 @_Z1g3str(%struct.str* byval nocapture readonly align 4 %s) {
+; CHECK-LABEL: _Z1g3str:
+; CHECK: sw  $7, [[OFFSET:[0-9]+]]($sp)
+; CHECK: lw  ${{[0-9]+}}, [[OFFSET]]($sp)
+entry:
+  %ref.tmp = alloca %struct.str, align 4
+  %0 = bitcast %struct.str* %ref.tmp to i8*
+  call void @llvm.lifetime.start.p0i8(i64 20, i8* nonnull %0)
+  call fastcc void @_Z1F3str(%struct.str* nonnull sret %ref.tmp, %struct.str* byval nonnull align 4 %s)
+  %cl.sroa.3.0..sroa_idx2 = getelementptr inbounds %struct.str, %struct.str* %ref.tmp, i32 0, i32 1
+  %cl.sroa.3.0.copyload = load i32, i32* %cl.sroa.3.0..sroa_idx2, align 4
+  call void @llvm.lifetime.end.p0i8(i64 20, i8* nonnull %0)
+  ret i32 %cl.sroa.3.0.copyload
+}
+
+declare void @llvm.lifetime.start.p0i8(i64, i8* nocapture)
+
+declare void @llvm.lifetime.end.p0i8(i64, i8* nocapture)

llvm/trunk/test/CodeGen/Mips/o32_cc_byval.ll

 ; CHECK-NEXT:    addu $gp, $2, $25
 ; CHECK-NEXT:    sw $7, 44($sp)
 ; CHECK-NEXT:    sw $6, 40($sp)
 ; CHECK-NEXT:    sw $5, 20($sp)
 ; CHECK-NEXT:    sw $4, 16($sp)
 ; CHECK-NEXT:    lw $7, 52($sp)
 ; CHECK-NEXT:    lw $6, 48($sp)
 ; CHECK-NEXT:    lw $5, 44($sp)
-; CHECK-NEXT:    lw $4, 40($sp)
 ; CHECK-NEXT:    lw $25, %call16(f6)($gp)
 ; CHECK-NEXT:    jalr $25
-; CHECK-NEXT:    nop
+; CHECK-NEXT:    lw $4, 40($sp)
 ; CHECK-NEXT:    lw $ra, 28($sp) # 4-byte Folded Reload
 ; CHECK-NEXT:    jr $ra
 ; CHECK-NEXT:    addiu $sp, $sp, 32
 entry:
   tail call void @f6(%struct.S4* byval %a1, i64 %a0) nounwind
   ret void
 }
 
 declare void @f6(%struct.S4* nocapture byval, i64)