[llvm-commits] [llvm] r159146 - in /llvm/trunk: lib/Transforms/InstCombine/InstructionCombining.cpp lib/Transforms/Scalar/SimplifyCFGPass.cpp lib/VMCore/Verifier.cpp test/Transforms/InstCombine/objsize-64.ll test/Transforms/SimplifyCFG/invoke.ll

[Duncan Sands]

Hi Nuno,

>>>   - instcombine:  invoke new  -> invoke expect(0, 0)  (an arbitrary NOOP
>>> intrinsic;  only done if the allocated memory is unused, of course)
>>
>> What is this about?  If you are saying that malloc is nounwind, why aren't you
>> marking it nounwind in SimplifyLibCalls.
>
> What the code is doing is deleting sequences like:
> p = new foo;
> .. no real uses of p..
> delete p; // optional

thanks for explaining.  You should really add a testcase testing this: the one
in the patch just seems to check invoke of undef and invoke of null.  (The lack
of a testcase was the reason I wasn't sure what you were doing).

>> In any case, by removing the restriction that you can't do invoke on an
>> intrinsic you are making a major (historically speaking change).  It should
>> thus be done as part of a different commit, and accompanied with a bunch of
>> tests checking that codegen doesn't crash if you try to codegen code that
>> does invoke of an intrinsic.

You didn't address this point.

>> Finally, doing thus funky intrinsic thing seems a bizarre approach to me,
>> there has to be a better way.
>>
>> I think plenty of people made these remarks already, but I didn't notice
>> you answering those objections - maybe I just missed it?
>
> So let first explain why I proposed this approach. Instcombine cannot modify the
> CFG, but removing an invoke instruction modifies the CFG.  The problem now is
> what do I do if I want to remove an invoke statement in instcombine?  You may
> say you don't.  But then you cannot optimize a bunch of things, including the
> elimination of useless 'new' calls as I described above.
> So then I came with the idea of inserting a dummy no-op intrinsic, which will be
> removed by SimplifyCFG.

The fact you have to jump through hoops in instcombine to do this just tells me
that you shouldn't be doing it in instcombine.  Please do it somewhere else.
SimplifyLibCalls would be logical, since this is really a libcall optimization,
other there is SimplifyCFG.  There's also PruneEH!

Ciao, Duncan.