Bug fix for shrink-wrap

[weimingz]

Summary

This patch fixes a case that not handled properly.
See bug report: https://llvm.org/bugs/show_bug.cgi?id=25824

[qcolombet]

Hi Weiming,

Please comment the new code and add a test case.

Cheers,
-Quentin

[weimingz]

This patch is more of a workaround to fix a bug. It seems to me the conditions of the safe point is not strong enough.
Since it's not SSA, the dominance relationship can't guarantee that the save point is before the def.

[weimingz]

In D15504#310102, @qcolombet wrote:

Hi Weiming,

Please comment the new code and add a test case.

Cheers,
-Quentin

Sure. I'm currently reducing the test case. It's exposed by a LTO compilation, so the original function is large. :D

[weimingz]

In D15504#310102, @qcolombet wrote:

Hi Weiming,

Please comment the new code and add a test case.

Cheers,
-Quentin

Hi Quentin,

I updated bugzilla with a test reduced from a 150,000 lines of IR. :D

I'm still trying to write a even simpler one with inline asm so we can check into unit test.

[qcolombet]

Hi Weiming,

Could you try if the attached patch fixes the problem?
If it does, that should be a much safer workaround.

Thanks,
-Quentin

• shrink-post-loop.patch1 KBDownload
• msg-28761-15.txt545 BDownload

[weimingz]

Hi Quentin,

Yes, but I need to change one line because otherwise, it will crash with other inputs.

else ==> else if (MLI->getLoopFor(Restore)) {

// If the loop does not exit, ....
SmallVector<...> ExitBlocks;

MLI->getLoopFor(Restore)->getExitingBlocks(ExitBlocks); ==> need to check if Restore is in loop
}

In D15504#310456, @qcolombet wrote:

Hi Weiming,

Could you try if the attached patch fixes the problem?
If it does, that should be a much safer workaround.

Thanks,
-Quentin

• shrink-post-loop.patch1 KBDownload
• msg-28761-15.txt545 BDownload

[qcolombet]

Thanks!

I’ll clean-up the patch and push with your additional fix.

Q.

[qcolombet]

Hi Weiming,

You can abandon this diff, the problem was fixed by r255613.

Thanks for reporting the problem!

Cheers,
-Quentin

lib/CodeGen/ShrinkWrap.cpp

if (Restore == &MBB) {
     }
   }
 
   if (!Restore) {
     DEBUG(dbgs() << "Restore point needs to be spanned on several blocks\n");
     return;
   }
 
+  if (Save == Restore && Save != &MBB)
+    if (auto L = MLI->getLoopFor(Save))
+      if (L->getHeader() == Save)
+        // Bail out.
+        Save = nullptr;
+
   // Make sure Save and Restore are suitable for shrink-wrapping:
   // 1. all path from Save needs to lead to Restore before exiting.
   // 2. all path to Restore needs to go through Save from Entry.
   // We achieve that by making sure that:
   // A. Save dominates Restore.
   // B. Restore post-dominates Save.
   // C. Save and Restore are in the same loop.
   bool SaveDominatesRestore = false;