Hello, This patch fixes PR4174. Two test-cases included: original one from bugzilla and a little bit complicated made be myself. It seems that LoopIndexSplit doesn't handle some cases, I'll try to send some patch this week. Regards -- Jakub Staszak -------------- next part -------------- A non-text attachment was scrubbed... Name: pr4174.patch Type: application/octet-stream Size: 3229 bytes Desc: not available URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20090821/0e07e86f/attachment.obj>
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.> It seems that LoopIndexSplit doesn't handle some cases, I'll try to send > some patch this week.You mean like PR3913? Or are you referring to something else? -Eli
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 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... -Eli