llvm dev - Aug 2009 - [LLVMdev] PR4174

On Aug 21, 2009, at 8:46 PM, Eli Friedman wrote:
> On Fri, Aug 21, 2009 at 3:29 PM, Jakub Staszak<kuba at gcc.gnu.org>  
> wrote:
>>
>> On Aug 21, 2009, at 7:31 PM, Eli Friedman wrote:
>>
>>> On Fri, Aug 21, 2009 at 2:06 PM, Jakub Staszak<kuba at
gcc.gnu.org>
>>> wrote:
>>>>
>>>> Hello,
>>>>
>>>> This patch fixes PR4174. Two test-cases included: original one
from
>>>> bugzilla
>>>> and a little bit complicated made be myself.
>>>
>>> I think you want isSafeToSpeculativelyExecute rather than
>>> mayHaveSideEffects.  Otherwise, looks fine.
>>
>> With isSafeToSpeculativelyExecute "make check" fails on some
tests,
>> i.e:
>> test/Transforms/LoopIndexSplit/SaveLastValue-2007-08-17.ll
>> because of PHI (isSafeToSpeculativelyExecute == false).
>
> isSafeToSpeculativelyExecute returns false for PHI nodes because it
> isn't really clear what to do in that case... here, it's safe, so
> isSafeToSpeculativelyExecute || isa<PHINode> is probably best.
Another two "hacks" are needed: for BranchInst and for IntrinsicInst.

Patch attached.

Regards
-- 
Jakub Staszak
-------------- next part --------------
A non-text attachment was scrubbed...
Name: pr4174-2.patch
Type: application/octet-stream
Size: 2940 bytes
Desc: not available
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20090821/0d23ec2b/attachment.obj>

On Aug 21, 2009, at 10:02 PM, Eli Friedman wrote:
> On Fri, Aug 21, 2009 at 4:53 PM, Jakub Staszak<kuba at gcc.gnu.org>  
> wrote:
>>
>> On Aug 21, 2009, at 8:46 PM, Eli Friedman wrote:
>>
>>> On Fri, Aug 21, 2009 at 3:29 PM, Jakub Staszak<kuba at
gcc.gnu.org>
>>> wrote:
>>>>
>>>> On Aug 21, 2009, at 7:31 PM, Eli Friedman wrote:
>>>>
>>>>> On Fri, Aug 21, 2009 at 2:06 PM, Jakub Staszak<kuba at
gcc.gnu.org>
>>>>> wrote:
>>>>>>
>>>>>> Hello,
>>>>>>
>>>>>> This patch fixes PR4174. Two test-cases included:
original one
>>>>>> from
>>>>>> bugzilla
>>>>>> and a little bit complicated made be myself.
>>>>>
>>>>> I think you want isSafeToSpeculativelyExecute rather than
>>>>> mayHaveSideEffects.  Otherwise, looks fine.
>>>>
>>>> With isSafeToSpeculativelyExecute "make check" fails
on some
>>>> tests, i.e:
>>>> test/Transforms/LoopIndexSplit/SaveLastValue-2007-08-17.ll
>>>> because of PHI (isSafeToSpeculativelyExecute == false).
>>>
>>> isSafeToSpeculativelyExecute returns false for PHI nodes because it
>>> isn't really clear what to do in that case... here, it's
safe, so
>>> isSafeToSpeculativelyExecute || isa<PHINode> is probably
best.
>>
>> Another two "hacks" are needed: for BranchInst and for
IntrinsicInst.
>>
>> Patch attached.
>
> IntrinsicInst isn't necessarily safe...

Without IntrinsicInst we break test/Transforms/LoopIndexSplit/ 
SplitValue-2007-08-24-dbg.ll.

-Jakub