[all-commits] [llvm/llvm-project] b7806c: [SLP] Explicit track required stacksave/alloca dep...

Philip Reames via All-commits all-commits at lists.llvm.org
Sun Mar 20 13:59:02 PDT 2022


  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: b7806c8b3764f04d02dc99b04f0cccff92e6c43e
      https://github.com/llvm/llvm-project/commit/b7806c8b3764f04d02dc99b04f0cccff92e6c43e
  Author: Philip Reames <listmail at philipreames.com>
  Date:   2022-03-20 (Sun, 20 Mar 2022)

  Changed paths:
    M llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
    M llvm/test/Transforms/SLPVectorizer/X86/stacksave-dependence.ll

  Log Message:
  -----------
  [SLP] Explicit track required stacksave/alloca dependency

The semantics of an inalloca alloca instruction requires that it not be reordered with a preceeding stacksave intrinsic call.  Unfortunately, there's no def/use edge or memory dependence edge.  (THe memory point is slightly subtle, but in general a new allocation can't alias with a call which executes strictly before it comes into existance.)

I'd tried to tackle this same case previously in 689babdf6, but the fix chosen there turned out to be incomplete.  As such, this change contains a fully revert of the first fix attempt.

This was noticed when investigating problems which surfaced with D118538, but this is definitely an existing bug.  This time around, I managed to reduce a couple of additional cases, including one which was being actively miscompiled even without the new scheduling change.  (See test diffs)

Compile time wise, we only spend extra time when seeing a stacksave (rare), and even then we walk the block at most once per schedule window extension.  Likely a non-issue.




More information about the All-commits mailing list