PR4174

Jakub_Staszak1 · August 21, 2009, 9:06pm

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

Eli_Friedman1 · August 21, 2009, 9:31pm

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

Jakub_Staszak1 · August 21, 2009, 11:53pm

Another two "hacks" are needed: for BranchInst and for IntrinsicInst.

Patch attached.

Regards

Eli_Friedman1 · August 22, 2009, 12:02am

IntrinsicInst isn't necessarily safe...

-Eli

Jakub_Staszak1 · August 22, 2009, 12:05am

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

-Jakub

Eli_Friedman1 · August 22, 2009, 12:27am

Checking for isa<DbgInfoIntrinsic> is safe.

-Eli

Jakub_Staszak1 · August 22, 2009, 12:47am

Ah, right Thanks for your help. I hope that everything is OK now.

New patch attached.

-Jakub

Eli_Friedman1 · August 22, 2009, 1:00am

Looks fine, I think. I should probably consider some changes to
isSafeToSpeculativelyExecute, though...

That said, I'm not entirely sure how this is supposed to interact with
LoopIndexSplit::cleanBlock... I never really quite understood what
it's doing.

-Eli

Eli_Friedman1 · August 22, 2009, 1:01am

Umm, make that "I'm not entirely sure whether you should be doing what
you're currently doing or using cleanBlock here".

-Eli