[all-commits] [llvm/llvm-project] 9228f2: [CGP] Consolidate logic for getIVIncrement and isI...

Philip Reames via All-commits all-commits at lists.llvm.org
Sat Mar 13 14:59:36 PST 2021


  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: 9228f2f3225bb7276d33af7658f6ba19c3037f4d
      https://github.com/llvm/llvm-project/commit/9228f2f3225bb7276d33af7658f6ba19c3037f4d
  Author: Philip Reames <listmail at philipreames.com>
  Date:   2021-03-13 (Sat, 13 Mar 2021)

  Changed paths:
    M llvm/lib/CodeGen/CodeGenPrepare.cpp

  Log Message:
  -----------
  [CGP] Consolidate logic for getIVIncrement and isIVIncrement

This fixes the bug demonstrated by the test case in the commit message of 8d20f2c2 (which was a revert of cf82700).  The root issue was that we have two transforms which are inverses of each other.  We use one for simple induction variables (where we can use the post-inc form), and the other for everything else.  The problem was that the two transforms could disagree about whether something was an induction variable.

The reverted commit made a change to one of the matcher routines which was used for one of the two transforms without updating the other matcher.  However, it's worth noting the existing code w/o the reverted change also has cases where the decision could differ between the two paths.

The fix is simply to consolidate the code such that two paths must agree by construction, and to add an assert to catch any potential future re-divergence.

Triggering the infinite loop requires side stepping the SunkAddrs cache.  The SunkAddrs cache has the effect of suppressing the iteration in the common case, but there are codepaths through CGP which restart iteration and clear this cache.

Unfortunately, I have not been able to construct a standalone IR test case for this.  The original test case is a c++ program which when compiled by clang demonstrates the infinite loop, but all of my attempts at extracting an IR test case runnable through opt/llc have failed to reproduce.  (Including capturing the IR at point of the transform itself!)  I have no idea what weird state clang is creating here.

I also tried creating a test case by hand, but gave up after about an hour of trying to find the right combination to dance through multiple transforms to create the end result needed to trip the bug.




More information about the All-commits mailing list