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

[Dan Gohman]

On Jul 15, 2009, at 10:21 PM, Eli Friedman wrote:


> On Wed, Jul 15, 2009 at 9:38 PM, Nick Lewycky<nicholas at mxc.ca> wrote:
>
>>> 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.
>>
>
> Attached is another suggestion.  Replaces isTrapping with a similar
> method named isSafeToSpeculativelyExecute.  Hopefully, this should be
> perfectly clear.

The isSafeToSpeculativelyExecute in this patch misses
code for FRem and FDiv.  Also, instead of being conservative with
void-typed instructions, it'd be better to just add the handful of
instruction kinds to the switch. It looks like it'd just be Store,
Free, and the terminators.

Otherwise, this patch looks good to me.

Dan