[llvm-commits] [llvm] r163678 - in /llvm/trunk: lib/CodeGen/StackColoring.cpp test/CodeGen/X86/StackColoring.ll

[Nuno Lopes]

Wait, doesn't this mean that you don't need the lifetime markers at all??
If you remove the markers that you cannot prove that are ok, then it's 
equivalent to not having this information at all, and then use only the 
computed liveness intervals.

Nuno

----- Original Message -----
> Author: nadav
> Date: Tue Sep 11 23:57:37 2012
> New Revision: 163678
>
> URL: http://llvm.org/viewvc/llvm-project?rev=163678&view=rev
> Log:
> Stack coloring: remove lifetime intervals which contain escaped allocas.
>
> The input program may contain intructions which are not inside lifetime
> markers. This can happen due to a bug in the compiler or due to a bug in
> user code (for example, returning a reference to a local variable).
> This commit adds checks that all of the instructions in the function and
> invalidates lifetime ranges which do not contain all of the instructions.
>
>
> Modified:
>    llvm/trunk/lib/CodeGen/StackColoring.cpp
>    llvm/trunk/test/CodeGen/X86/StackColoring.ll
>
> Modified: llvm/trunk/lib/CodeGen/StackColoring.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/StackColoring.cpp?rev=163678&r1=163677&r2=163678&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/StackColoring.cpp (original)
> +++ llvm/trunk/lib/CodeGen/StackColoring.cpp Tue Sep 11 23:57:37 2012
> @@ -158,6 +158,14 @@
>   /// slots to use the joint slots.
>   void remapInstructions(DenseMap<int, int> &SlotRemap);
>
> +  /// The input program may contain intructions which are not inside 
> lifetime
> +  /// markers. This can happen due to a bug in the compiler or due to a 
> bug in
> +  /// user code (for example, returning a reference to a local variable).
> +  /// This procedure checks all of the instructions in the function and
> +  /// invalidates lifetime ranges which do not contain all of the 
> instructions
> +  /// which access that frame slot.
> +  void removeInvalidSlotRanges();
> +
>   /// Map entries which point to other entries to their destination.
>   ///   A->B->C becomes A->C.
>    void expungeSlotMap(DenseMap<int, int> &SlotRemap, unsigned NumSlots);
> @@ -543,6 +551,43 @@
>   DEBUG(dbgs()<<"Fixed "<<FixedInstr<<" machine instructions.\n");
> }
>
> +void StackColoring::removeInvalidSlotRanges() {
> +  MachineFunction::iterator BB, BBE;
> +  MachineBasicBlock::iterator I, IE;
> +  for (BB = MF->begin(), BBE = MF->end(); BB != BBE; ++BB)
> +    for (I = BB->begin(), IE = BB->end(); I != IE; ++I) {
> +
> +      if (I->getOpcode() == TargetOpcode::LIFETIME_START ||
> +          I->getOpcode() == TargetOpcode::LIFETIME_END || 
> I->isDebugValue())
> +        continue;
> +
> +      // Check all of the machine operands.
> +      for (unsigned i = 0 ; i <  I->getNumOperands(); ++i) {
> +        MachineOperand &MO = I->getOperand(i);
> +
> +        if (!MO.isFI())
> +          continue;
> +
> +        int Slot = MO.getIndex();
> +
> +        if (Slot<0)
> +          continue;
> +
> +        if (Intervals[Slot]->empty())
> +          continue;
> +
> +        // Check that the used slot is inside the calculated lifetime 
> range.
> +        // If it is not, warn about it and invalidate the range.
> +        LiveInterval *Interval = Intervals[Slot];
> +        SlotIndex Index = Indexes->getInstructionIndex(I);
> +        if (Interval->find(Index) == Interval->end()) {
> +          Intervals[Slot]->clear();
> +          DEBUG(dbgs()<<"Invalidating range #"<<Slot<<"\n");
> +        }
> +      }
> +    }
> +}
> +
> void StackColoring::expungeSlotMap(DenseMap<int, int> &SlotRemap,
>                                    unsigned NumSlots) {
>   // Expunge slot remap map.
> @@ -617,6 +662,8 @@
>   // Propagate the liveness information.
>   calculateLiveIntervals(NumSlots);
>
> +  removeInvalidSlotRanges();
> +
>   // Maps old slots to new slots.
>   DenseMap<int, int> SlotRemap;
>   unsigned RemovedSlots = 0;
>
> Modified: llvm/trunk/test/CodeGen/X86/StackColoring.ll
> URL: 
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/StackColoring.ll?rev=163678&r1=163677&r2=163678&view=diff
> ==============================================================================
> --- llvm/trunk/test/CodeGen/X86/StackColoring.ll (original)
> +++ llvm/trunk/test/CodeGen/X86/StackColoring.ll Tue Sep 11 23:57:37 2012
> @@ -7,7 +7,6 @@
> ;YESCOLOR: subq  $136, %rsp
> ;NOCOLOR: subq  $264, %rsp
>
> -
> define i32 @myCall_w2(i32 %in) {
> entry:
>   %a = alloca [17 x i8*], align 8
> @@ -328,7 +327,6 @@
>
> ;YESCOLOR: subq  $272, %rsp
> ;NOCOLOR: subq  $272, %rsp
> -
> define i32 @myCall_end_before_begin(i32 %in, i1 %d) {
> entry:
>   %a = alloca [17 x i8*], align 8
> @@ -352,11 +350,37 @@
>   ret i32 0
> }
>
> +; Check that we don't assert and crash even when there are allocas
> +; outside the declared lifetime regions.
> +;YESCOLOR: bad_range
> +;NOCOLOR:  bad_range
> +define void @bad_range() nounwind ssp {
> +entry:
> +  %A.i1 = alloca [100 x i32], align 4
> +  %B.i2 = alloca [100 x i32], align 4
> +  %A.i = alloca [100 x i32], align 4
> +  %B.i = alloca [100 x i32], align 4
> +  %0 = bitcast [100 x i32]* %A.i to i8*
> +  call void @llvm.lifetime.start(i64 -1, i8* %0) nounwind
> +  %1 = bitcast [100 x i32]* %B.i to i8*
> +  call void @llvm.lifetime.start(i64 -1, i8* %1) nounwind
> +  call void @bar([100 x i32]* %A.i, [100 x i32]* %B.i) nounwind
> +  call void @llvm.lifetime.end(i64 -1, i8* %0) nounwind
> +  call void @llvm.lifetime.end(i64 -1, i8* %1) nounwind
> +  br label %block2
> +
> +block2:
> +  ; I am used outside the marked lifetime.
> +  call void @bar([100 x i32]* %A.i, [100 x i32]* %B.i) nounwind
> +  ret void
> +}
> +
> +
> declare void @bar([100 x i32]* , [100 x i32]*) nounwind
>
> declare void @llvm.lifetime.start(i64, i8* nocapture) nounwind
>
> declare void @llvm.lifetime.end(i64, i8* nocapture) nounwind
>
> - declare i32 @foo(i32, i8*)
> +declare i32 @foo(i32, i8*)
>