[llvm-commits] [llvm] r159385 - in /llvm/trunk: lib/Transforms/Scalar/SimplifyCFGPass.cpp test/Transforms/SimplifyCFG/invoke.ll
Nuno Lopes
nunoplopes at sapo.pt
Fri Jun 29 08:45:47 PDT 2012
Quoting Duncan Sands <baldrick at free.fr>:
> Hi Nuno,
>
>> make simplifyCFG erase invokes to readonly/readnone functions
>
> this is wrong, please don't do this! Being readonly/readnone is
> orthogonal to being nounwind.
Sorry, I guess my commit log was not explicit enough.
I'm just removing invokes to functions that are both readnone and
nounwind. The code doesn't touch functions that may unwind.
My reasoning was a bit simpler than yours (maybe naive, though), which
was that if a function can throw an exception, then we cannot delete a
call to it, since then we'll never possibly see that exception,
meaning that we changed the semantics of the program.
BTW, thanks for your explanation! Always learning :)
Nuno
> To convince you of this, consider the following simple way that
> some basic exception handling could be implemented: every function returns an
> extra boolean value. If "true" is returned in it (indicating that
> an exception
> was thrown) then the caller branches to the appropriate code: either
> a landing
> pad if called from an invoke, or, if an ordinary call was used, to code that
> returns "true" in the caller's boolean (i.e. continues to unwind).
>
> Notice how in this scheme it is perfectly possible for a function to
> be readnone
> but throw an exception (i.e. return "true").
>
> Note that these extra return values don't have to be present at the IR level,
> codegen can add them as its way of implementing exception handling. In fact
> it might be nice if codegen offered this scheme as an option, for languages
> that don't need all the dwarf bells and whistles. In fact vmkit does this
> (I don't know if it does it at the IR level or via codegen) because it is
> faster for java (which throws exceptions a lot) than using dwarf.
>
> It is true that the dwarf implementation of exception handling
> writes to memory
> all over the place, and thus is incompatible with readonly/readnone.
> But that's
> just due to how the dwarf implementation works, and thus shouldn't
> be baked into
> the IR as your patch does.
>
> In short, at the IR level it would be wrong to do this, because it is
> presupposing how codegen will implement exception handling. Maybe
> it is OK to
> do this optimization at the codegen level once you know that dwarf exception
> handling has been selected?
>
> Ciao, Duncan.
>
> PS: a front-end can always output nounwind on functions it knows are
> readnone/readonly if it wants those semantics.
> PPS: this was endlessly debated on the GCC mailing list, and the
> conclusion for
> LLVM was to decouple readnone/readonly and nounwind, which lets
> front-ends make
> their own decision.
>
>>
>> Modified:
>> llvm/trunk/lib/Transforms/Scalar/SimplifyCFGPass.cpp
>> llvm/trunk/test/Transforms/SimplifyCFG/invoke.ll
>>
>> Modified: llvm/trunk/lib/Transforms/Scalar/SimplifyCFGPass.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/SimplifyCFGPass.cpp?rev=159385&r1=159384&r2=159385&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Transforms/Scalar/SimplifyCFGPass.cpp (original)
>> +++ llvm/trunk/lib/Transforms/Scalar/SimplifyCFGPass.cpp Thu Jun 28
>> 17:32:27 2012
>> @@ -89,7 +89,6 @@
>>
>> /// ChangeToCall - Convert the specified invoke into a normal call.
>> static void ChangeToCall(InvokeInst *II) {
>> - BasicBlock *BB = II->getParent();
>> SmallVector<Value*, 8> Args(II->op_begin(), II->op_end() - 3);
>> CallInst *NewCall = CallInst::Create(II->getCalledValue(),
>> Args, "", II);
>> NewCall->takeName(II);
>> @@ -100,10 +99,7 @@
>>
>> // Follow the call by a branch to the normal destination.
>> BranchInst::Create(II->getNormalDest(), II);
>> -
>> - // Update PHI nodes in the unwind destination
>> - II->getUnwindDest()->removePredecessor(BB);
>> - BB->getInstList().erase(II);
>> + II->eraseFromParent();
>> }
>>
>> static bool MarkAliveBlocks(BasicBlock *BB,
>> @@ -163,7 +159,12 @@
>> ChangeToUnreachable(II, true);
>> Changed = true;
>> } else if (II->doesNotThrow()) {
>> - ChangeToCall(II);
>> + if (II->use_empty() && II->onlyReadsMemory()) {
>> + // jump to the normal destination branch.
>> + BranchInst::Create(II->getNormalDest(), II);
>> + II->eraseFromParent();
>> + } else
>> + ChangeToCall(II);
>> Changed = true;
>> }
>> }
>>
>> Modified: llvm/trunk/test/Transforms/SimplifyCFG/invoke.ll
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/SimplifyCFG/invoke.ll?rev=159385&r1=159384&r2=159385&view=diff
>> ==============================================================================
>> --- llvm/trunk/test/Transforms/SimplifyCFG/invoke.ll (original)
>> +++ llvm/trunk/test/Transforms/SimplifyCFG/invoke.ll Thu Jun 28
>> 17:32:27 2012
>> @@ -3,7 +3,7 @@
>>
>> declare i32 @__gxx_personality_v0(...)
>> declare void @__cxa_call_unexpected(i8*)
>> -declare i64 @llvm.objectsize.i64(i8*, i1) nounwind readonly
>> +declare i32 @read_only() nounwind readonly
>>
>>
>> ; CHECK: @f1
>> @@ -43,3 +43,42 @@
>> tail call void @__cxa_call_unexpected(i8* %1) noreturn nounwind
>> unreachable
>> }
>> +
>> +; CHECK: @f3
>> +define i32 @f3() nounwind uwtable ssp {
>> +; CHECK-NEXT: entry
>> +entry:
>> +; CHECK-NEXT: ret i32 3
>> + %call = invoke i32 @read_only()
>> + to label %invoke.cont unwind label %lpad
>> +
>> +invoke.cont:
>> + ret i32 3
>> +
>> +lpad:
>> + %0 = landingpad { i8*, i32 } personality i8* bitcast (i32 (...)*
>> @__gxx_personality_v0 to i8*)
>> + filter [0 x i8*] zeroinitializer
>> + %1 = extractvalue { i8*, i32 } %0, 0
>> + tail call void @__cxa_call_unexpected(i8* %1) noreturn nounwind
>> + unreachable
>> +}
>> +
>> +; CHECK: @f4
>> +define i32 @f4() nounwind uwtable ssp {
>> +; CHECK-NEXT: entry
>> +entry:
>> +; CHECK-NEXT: call i32 @read_only()
>> + %call = invoke i32 @read_only()
>> + to label %invoke.cont unwind label %lpad
>> +
>> +invoke.cont:
>> +; CHECK-NEXT: ret i32 %call
>> + ret i32 %call
>> +
>> +lpad:
>> + %0 = landingpad { i8*, i32 } personality i8* bitcast (i32 (...)*
>> @__gxx_personality_v0 to i8*)
>> + filter [0 x i8*] zeroinitializer
>> + %1 = extractvalue { i8*, i32 } %0, 0
>> + tail call void @__cxa_call_unexpected(i8* %1) noreturn nounwind
>> + unreachable
>> +}
More information about the llvm-commits
mailing list