[llvm-commits] [llvm] r147036 - in /llvm/trunk: lib/Analysis/ValueTracking.cpp lib/Transforms/Utils/SimplifyCFG.cpp test/Transforms/SimplifyCFG/SpeculativeExec.ll

[Eli Friedman]

On Wed, Dec 21, 2011 at 1:04 PM, Nick Lewycky <nicholas at mxc.ca> wrote:
> On 12/21/2011 11:02 AM, Dan Gohman wrote:
>>
>> On Dec 20, 2011, at 9:52 PM, Nick Lewycky wrote:
>>
>>> Author: nicholas
>>> Date: Tue Dec 20 23:52:02 2011
>>> New Revision: 147036
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=147036&view=rev
>>> Log:
>>> Make some intrinsics safe to speculatively execute.
>>>
>>> Modified:
>>>     llvm/trunk/lib/Analysis/ValueTracking.cpp
>>>     llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp
>>>     llvm/trunk/test/Transforms/SimplifyCFG/SpeculativeExec.ll
>>>
>>> Modified: llvm/trunk/lib/Analysis/ValueTracking.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/ValueTracking.cpp?rev=147036&r1=147035&r2=147036&view=diff
>>> ==============================================================================
>>> --- llvm/trunk/lib/Analysis/ValueTracking.cpp (original)
>>> +++ llvm/trunk/lib/Analysis/ValueTracking.cpp Tue Dec 20 23:52:02 2011
>>> @@ -1912,11 +1912,31 @@
>>>        return false;
>>>      return LI->getPointerOperand()->isDereferenceablePointer();
>>>    }
>>> -  case Instruction::Call:
>>> +  case Instruction::Call: {
>>> +   if (const IntrinsicInst *II = dyn_cast<IntrinsicInst>(Inst)) {
>>> +     switch (II->getIntrinsicID()) {
>>> +       case Intrinsic::bswap:
>>> +       case Intrinsic::ctlz:
>>> +       case Intrinsic::ctpop:
>>> +       case Intrinsic::cttz:
>>> +       case Intrinsic::objectsize:
>>> +       case Intrinsic::sadd_with_overflow:
>>> +       case Intrinsic::smul_with_overflow:
>>> +       case Intrinsic::ssub_with_overflow:
>>> +       case Intrinsic::uadd_with_overflow:
>>> +       case Intrinsic::umul_with_overflow:
>>> +       case Intrinsic::usub_with_overflow:
>>> +         return true;
>>> +       // TODO: some fp intrinsics are marked as having the same error handling
>>> +       // as libm. They're safe to speculate when they won't error.
>>> +       // TODO: are convert_{from,to}_fp16 safe?
>>> +       // TODO: can we list target-specific intrinsics here?
>>
>> Just checking mayHaveSideEffects() should cover almost everything here.
>
> Good point! Is there anything that mayHaveSideEffects() would return
> true on that wouldn't be safe to speculate? The comment on
> mayHaveSideEffects claims that a call to malloc would return false, but
> that's a lie, suggesting that an audit of users of these two functions
> is due...

mayHaveSideEffects is essentially equivalent to mayWriteToMemory, and
that is clearly not the same thing as being safe to speculate.

-Eli