[all-commits] [llvm/llvm-project] 9afb1c: Revert "Outline non returning functions unless a l...

Vedant Kumar via All-commits all-commits at lists.llvm.org
Mon Oct 5 14:10:45 PDT 2020


  Branch: refs/heads/master
  Home:   https://github.com/llvm/llvm-project
  Commit: 9afb1c566e8cb396da495e2fbbbc53e1814cc3a1
      https://github.com/llvm/llvm-project/commit/9afb1c566e8cb396da495e2fbbbc53e1814cc3a1
  Author: Vedant Kumar <vsk at apple.com>
  Date:   2020-10-05 (Mon, 05 Oct 2020)

  Changed paths:
    M llvm/lib/Transforms/IPO/HotColdSplitting.cpp
    R llvm/test/Transforms/HotColdSplit/longjmp-nosplit.ll
    R llvm/test/Transforms/HotColdSplit/longjmp-split.ll
    R llvm/test/Transforms/HotColdSplit/sjlj-nosplit.ll
    R llvm/test/Transforms/HotColdSplit/sjlj-split.ll
    R llvm/test/Transforms/HotColdSplit/split-assert-fail.ll

  Log Message:
  -----------
  Revert "Outline non returning functions unless a longjmp"

This reverts commit 20797989ea190f2ef22d13c5a7a0535fe9afa58b.

This patch (https://reviews.llvm.org/D69257) cannot complete a stage2
build due to the change:

```
CI->getCalledFunction()->getName().contains("longjmp")
```

There are several concrete issues here:

  - The callee may not be a function, so `getCalledFunction` can assert.
  - The called value may not have a name, so `getName` can assert.
  - There's no distinction made between "my_longjmp_test_helper" and the
    actual longjmp libcall.

At a higher level, there's a serious layering problem here. The
splitting pass makes policy decisions in a general way (e.g. based on
attributes or profile data). Special-casing certain names breaks the
layering. It subverts the work of library maintainers (who may now need
to opt-out of unexpected optimization behavior for any affected
functions) and can lead to inconsistent optimization behavior (as not
all llvm passes special-case ".*longjmp.*" in the same way).

The patch may need significant revision to address these issues.

But the immediate issue is that this crashes while compiling llvm's unit
tests in a stage2 build (due to the `getName` problem).




More information about the All-commits mailing list