Page MenuHomePhabricator

[LoopUnroll] Properly update loopinfo for runtime unrolling by 2
ClosedPublic

Authored by mkuper on Jan 25 2017, 4:02 PM.

Details

Summary

Even when we don't actually create a remainder loop (that is, when we unroll by 2), we may have nested loops that we need to create.
This is complicated by the fact the remainder may either live in an outer loop or in the top-level, which means we may create new top-level loops.

I'm not too happy with this patch, better suggestions are welcome.

Diff Detail

Repository
rL LLVM

Event Timeline

mkuper created this revision.Jan 25 2017, 4:02 PM
chandlerc accepted this revision.Jan 25 2017, 4:14 PM

I understand why this is a bit gross, but I dunno... I don't have any better ideas. Ship it? (Feel free to wait for mzolotukhin or others if you're not sure.)

lib/Transforms/Utils/LoopUnrollRuntime.cpp
328 ↗(On Diff #85830)

indent looks off here? maybe just phab...

This revision is now accepted and ready to land.Jan 25 2017, 4:14 PM
fhahn accepted this revision.Jan 25 2017, 4:21 PM

LGTM. I also looked into creating a patch for this issue and I think this patch addresses the issue relatively nicely.

lib/Transforms/Utils/LoopUnroll.cpp
205 ↗(On Diff #85830)

Unnecessary { and }, not sure if you want to keep them. In your change in LoopUnrollRuntime.cpp you don't use them for single statement blocks.

Ok, thanks.
If two people think it's good enough, it's going on. @mzolotukhin , feel free to suggest improvements post-commit. :-)

lib/Transforms/Utils/LoopUnrollRuntime.cpp
328 ↗(On Diff #85830)

No, not phab, my bad, thanks.

This revision was automatically updated to reflect the committed changes.