[llvm-commits] [llvm] r75567 - /llvm/trunk/lib/VMCore/Instruction.cpp

[Nick Lewycky]

Dan Gohman wrote:
> On Jul 15, 2009, at 2:20 AM, Owen Anderson wrote:
> 
> 
>> On Jul 15, 2009, at 2:07 AM, Eli Friedman wrote:
>>
>>
>>> On Wed, Jul 15, 2009 at 1:39 AM, Duncan Sands<baldrick at free.fr>  
>>> wrote:
>>>
>>>>> FreeInst is trapping.
>>>>>
>>>>
>>>> Really?
>>>>
>>>
>>> Anything that touches memory is trapping because it'll crash on an
>>>
>>> invalid pointer.
>>>
>> Does free() actually dereference the pointer?  I don't think so,  
>> ergo no trapping.
>>
>> It's probably undefined what happens when you free an invalid  
>> pointer, so we can choose to do whatever we want with it, but I see  
>> no particular reason to require preserving trapping behavior for it.
> 
> isTrapping() also includes things like div and rem which have undefined
> behavior when the remainder is 0. The point of isTrapping is not to
> test for actual trapping, but rather to test for anything that would
> make naively moving an instruction around the CFG dangerous.
> 
> mayHaveSideEffects() doesn't do the same thing, since it only tests for
> mayWriteToMemory() || mayThrow().

And some day will grow "|| !willreturn", once we have a willreturn attr 
to complement noreturn.

> Perhaps mayHaveSideEffects() should be renamed to
> mayHaveDefinedSideEffects(), and isTrapping() should be renamed to
> mayHaveUndefinedBehavior()?

mayHaveUndefinedBehavior is a great choice here. That perfectly captures 
what it's doing.

I'm not as thrilled about mayHaveDefinedSideEffects though, I think the 
current name means the same thing and has the benefit of clarity through 
brevity.

Nick