[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

[Benjamin Kramer]

On 26.06.2012, at 09:13, Duncan Sands <baldrick at free.fr> wrote:

> 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!

SimplifyLibCalls was split out of instcombine so it can be disabled
when targeting a freestanding environment. The targetlibraryinfo
infrastructure fixed this problem and a long-term goal is to merge the
pass back into instcombine. Changing the CFG inside simplifylibcalls
would make this much harder.

- Ben