[llvm] r175553 - Fix a bug in mayHaveSideEffects. Functions that do not return are now considered as instructions with side effects.

[Hal Finkel]

----- Original Message -----
> From: "Duncan Sands" <baldrick at free.fr>
> To: llvm-commits at cs.uiuc.edu
> Sent: Wednesday, February 20, 2013 7:10:32 AM
> Subject: Re: [llvm] r175553 - Fix a bug in mayHaveSideEffects. Functions that	do not return are now considered as
> instructions with side effects.
> 
> Hi Nadav,
> 
> On 19/02/13 21:02, Nadav Rotem wrote:
> > Author: nadav
> > Date: Tue Feb 19 14:02:09 2013
> > New Revision: 175553
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=175553&view=rev
> > Log:
> >
> > Fix a bug in mayHaveSideEffects. Functions that do not return are
> > now considered as instructions with side effects.
> 
> what's special about functions where we were able to tell it doesn't
> return,
> as compared to functions that don't return but we weren't clever
> enough to
> realize it?  For example, in your testcase, what if the loop was
> tricky enough
> that we didn't realize it looped forever, even though it did?
> 
> I'm saying that your patch doesn't solve the "I called an infinite
> looping
> function" problem, it just makes it less obvious.

I think that we may want to adopt an N1528-like viewpoint on this and say that functions that don't return have undefined behavior/semantics unless explicitly marked.

http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1528.htm

 -Hal

> 
> In short, I think this is the wrong way round.  I reckon a better
> strategy
> would be to introduce a new attribute "finite" (we can argue about
> the name),
> which means that we know or proved that the function completes in
> finite time.
> Then functions which are not "finite" would be considered to have
> side-effects.
> 
> This way, functions which obviously infinite loop and functions which
> also
> infinite loop but we can't tell will both be considered to have
> side-effects.
> 
> Ciao, Duncan.
> 
> >
> > rdar://13227456
> >
> >
> > Added:
> >      llvm/trunk/test/Transforms/FunctionAttrs/noreturn.ll
> > Modified:
> >      llvm/trunk/include/llvm/IR/Instruction.h
> >      llvm/trunk/lib/IR/Instruction.cpp
> >
> > Modified: llvm/trunk/include/llvm/IR/Instruction.h
> > URL:
> > http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/Instruction.h?rev=175553&r1=175552&r2=175553&view=diff
> > ==============================================================================
> > --- llvm/trunk/include/llvm/IR/Instruction.h (original)
> > +++ llvm/trunk/include/llvm/IR/Instruction.h Tue Feb 19 14:02:09
> > 2013
> > @@ -309,6 +309,12 @@ public:
> >     ///
> >     bool mayThrow() const;
> >
> > +  /// mayReturn - Return true if this is a function that may
> > return.
> > +  /// this is true for all normal instructions. The only exception
> > +  /// is functions that are marked with the 'noreturn' attribute.
> > +  ///
> > +  bool mayReturn() const;
> > +
> >     /// mayHaveSideEffects - Return true if the instruction may
> >     have side effects.
> >     ///
> >     /// Note that this does not consider malloc and alloca to have
> >     side
> > @@ -316,7 +322,7 @@ public:
> >     /// instructions which don't used the returned value.  For
> >     cases where this
> >     /// matters, isSafeToSpeculativelyExecute may be more
> >     appropriate.
> >     bool mayHaveSideEffects() const {
> > -    return mayWriteToMemory() || mayThrow();
> > +    return mayWriteToMemory() || mayThrow() || !mayReturn();
> >     }
> >
> >     /// clone() - Create a copy of 'this' instruction that is
> >     identical in all
> >
> > Modified: llvm/trunk/lib/IR/Instruction.cpp
> > URL:
> > http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/Instruction.cpp?rev=175553&r1=175552&r2=175553&view=diff
> > ==============================================================================
> > --- llvm/trunk/lib/IR/Instruction.cpp (original)
> > +++ llvm/trunk/lib/IR/Instruction.cpp Tue Feb 19 14:02:09 2013
> > @@ -455,14 +455,18 @@ bool Instruction::mayWriteToMemory() con
> >     }
> >   }
> >
> > -/// mayThrow - Return true if this instruction may throw an
> > exception.
> > -///
> >   bool Instruction::mayThrow() const {
> >     if (const CallInst *CI = dyn_cast<CallInst>(this))
> >       return !CI->doesNotThrow();
> >     return isa<ResumeInst>(this);
> >   }
> >
> > +bool Instruction::mayReturn() const {
> > +  if (const CallInst *CI = dyn_cast<CallInst>(this))
> > +    return !CI->doesNotReturn();
> > +  return true;
> > +}
> > +
> >   /// isAssociative - Return true if the instruction is
> >   associative:
> >   ///
> >   ///   Associative operators satisfy:  x op (y op z) === (x op y)
> >   op z
> >
> > Added: llvm/trunk/test/Transforms/FunctionAttrs/noreturn.ll
> > URL:
> > http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/FunctionAttrs/noreturn.ll?rev=175553&view=auto
> > ==============================================================================
> > --- llvm/trunk/test/Transforms/FunctionAttrs/noreturn.ll (added)
> > +++ llvm/trunk/test/Transforms/FunctionAttrs/noreturn.ll Tue Feb 19
> > 14:02:09 2013
> > @@ -0,0 +1,18 @@
> > +; RUN: opt < %s -functionattrs -instcombine -S | FileCheck %s
> > +
> > +define void @endless_loop() noreturn nounwind readnone ssp uwtable
> > {
> > +entry:
> > +  br label %while.body
> > +
> > +while.body:
> > +  br label %while.body
> > +}
> > +;CHECK: @main
> > +;CHECK: endless_loop
> > +;CHECK: ret
> > +define i32 @main() noreturn nounwind ssp uwtable {
> > +entry:
> > +  tail call void @endless_loop()
> > +  unreachable
> > +}
> > +
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> >
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>