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

[Duncan Sands]

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.

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
>