[all-commits] [llvm/llvm-project] 1fee71: [funcattrs] Fix incorrect readnone/readonly infere...

[Philip Reames via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: 1fee7195c99a3cbeabb1ade35b00d974b4829e58
      https://github.com/llvm/llvm-project/commit/1fee7195c99a3cbeabb1ade35b00d974b4829e58
  Author: Philip Reames <listmail at philipreames.com>
  Date:   2021-12-21 (Tue, 21 Dec 2021)

  Changed paths:
    M llvm/lib/Transforms/IPO/FunctionAttrs.cpp
    M llvm/test/Transforms/FunctionAttrs/readattrs.ll

  Log Message:
  -----------
  [funcattrs] Fix incorrect readnone/readonly inference on captured arguments

This fixes a bug where we would infer readnone/readonly for a function which passed a value to a function which could capture it. With the value captured in memory, the function could reload the value from memory after the call, and write to it. Inferring the argument as readnone or readonly is unsound.

@jdoerfert apparently noticed this about two years ago, and tests were checked in with 76467c4, but the issue appears to have never gotten fixed.

Since this seems like this issue should break everything, let me explain why the case is actually fairly narrow. The main inference loop over the argument SCCs only analyzes nocapture arguments. As such, we can only hit this when construction the partial SCCs. Due to that restriction, we can only hit this when we have either a) a function declaration with a manually annotated argument, or b) an immediately self recursive call.

It's also worth highlighting that we do have cases we can infer readonly/readnone on a capturing argument validly. The easiest example is a function which simply returns its argument without ever accessing it.

Differential Revision: https://reviews.llvm.org/D115961


  Commit: b7b308c50ae51629a63e759cd1171714eca7784c
      https://github.com/llvm/llvm-project/commit/b7b308c50ae51629a63e759cd1171714eca7784c
  Author: Philip Reames <listmail at philipreames.com>
  Date:   2021-12-21 (Tue, 21 Dec 2021)

  Changed paths:
    M llvm/lib/Transforms/IPO/FunctionAttrs.cpp
    M llvm/test/Transforms/FunctionAttrs/out-of-bounds-iterator-bug.ll

  Log Message:
  -----------
  [funcattrs] Infer access attributes for vararg arguments

This change allows us to infer access attributes (readnone, readonly) on arguments passed to vararg functions. Since there isn't a formal argument corresponding to the parameter, they'll never be considered part of the speculative SCC, but they can still benefit from attributes on the call site or the callee function.

The main motivation here is just to simplify the code, and remove some special casing. Previously, an indirect vararg call could return more precise results than an direct vararg call which is just weird.

Differential Revision: https://reviews.llvm.org/D115964


Compare: https://github.com/llvm/llvm-project/compare/55c71c9eac9b...b7b308c50ae5