Page MenuHomePhabricator

[LoopFusion] Ensure that both loops are guarded or neither are guarded.
AcceptedPublic

Authored by kbarton on Dec 16 2019, 2:04 PM.

Details

Summary

This patch modifies the current conditions for loop fusion by ensuring that
either both fusion candidates have a guard, or neither have a guard. This
prevents us from inadvertantly attempting to fuse a guarded loop with a
non-guarded loop.

Event Timeline

kbarton created this revision.Dec 16 2019, 2:04 PM
Herald added a project: Restricted Project. · View Herald TranscriptDec 16 2019, 2:04 PM
Whitney added inline comments.Dec 18 2019, 3:43 PM
llvm/test/Transforms/LoopFusion/cannot_fuse.ll
421

optional:
define void @only_one_guarded(i32* noalias %A, i32* noalias %B, i1 zeroext %cond) {

Whitney added inline comments.Jan 21 2020, 7:11 PM
llvm/lib/Transforms/Scalar/LoopFuse.cpp
1070

haveIdenticalGuards() is not called with both candidates unguarded, so (!FC0.GuardBranch && !FC1.GuardBranch) is impossible.

LGTM. @Whitney?

llvm/lib/Transforms/Scalar/LoopFuse.cpp
738–739

[style] (FC0->GuardBranch || FC1->GuardBranch) && !haveIdenticalGuards(*FC0, *FC1)

1069–1070

The check can still make sense for defensive programming (or replaced by an assertion).

[style] !FC0.GuardBranch != !FC1.GuardBranch

LGTM. @Whitney?

I know that @kbarton is working on a patch which allow guarded loops fuse with unguarded loops as long as they are control flow equivalent.
I think that's a better solution. And with that, this patch is no longer needed.
However, I don't mind having this patch committed in the meantime.

Meinersbur accepted this revision.Feb 17 2020, 3:51 PM

OK then; Kit may either abandon or commit this patch.

This revision is now accepted and ready to land.Feb 17 2020, 3:51 PM