[PATCH] D69257: Outline non returning functions unless a longjmp

[Vedant Kumar via Phabricator via llvm-commits]

vsk added a comment.

FYI, 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).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69257/new/

https://reviews.llvm.org/D69257