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

[Daniel Berlin]

On Sat, Sep 15, 2012 at 9:03 AM, Nadav Rotem <nrotem at apple.com> wrote:
>
>
> On Sep 15, 2012, at 14:24, Nuno Lopes <nunoplopes at sapo.pt> wrote:
>
>> 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.
>>
>
> We use the lifetime markers to calculate the liveness intervals and throw away the intervals which are found to be broken. We need the markers to calculate the intervals.

As Duncan asked, shouldn't you just fix the producers of such broken
intervals (be they compilers or user code)?
For example, if someone returns a reference to a local variable,
beside being undefined, the code will often get broken by other
optimizations anyway.

This won't be a surprise either, since both clang and GCC warn about
it by default in the case of returning a reference to a local.

Besides Duncan's concern about cost, note that there are always
wonderfully exciting ways people will come up with broken/illegal code
that "breaks" a particular pass, so if you go down this route, you may
find other things you have to check for as well.  As they say, it's
impossible to make anything foolproof, because fools are so damn
ingenious.



>
>
>> 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*)
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits