This is an archive of the discontinued LLVM Phabricator instance.

llvm.noalias - Use noalias intrinsics when inlining (and update them when cloning metadata)
Needs ReviewPublic

Authored by hfinkel on Apr 30 2015, 9:22 AM.

Details

Reviewers
chandlerc
Summary

This is part of the series started by D9375, and have two related changes:

  1. When inlining a function, and cloning the alias scope metadata, we need to clone the corresponding metadata within the calls to llvm.noalias too.
  2. When inlining a function, instead of adding noalias metadata and alias.scope metadata (and doing the corresponding pointer-derivation and capturing analysis), add noalias metadata and use llvm.noalias intrinsics. This is more expressive (keeps the necessary dominance information), and is simpler (and faster).

Diff Detail

Event Timeline

hfinkel updated this revision to Diff 24739.Apr 30 2015, 9:22 AM
hfinkel retitled this revision from to llvm.noalias - Use noalias intrinsics when inlining (and update them when cloning metadata).
hfinkel updated this object.
hfinkel edited the test plan for this revision. (Show Details)
hfinkel added reviewers: chandlerc, reames.
hfinkel added a subscriber: Unknown Object (MLST).
reames resigned from this revision.Oct 8 2015, 10:25 AM
reames removed a reviewer: reames.

Resigning as a reviewer to get a very stale review off my list of blocking tasks in phabricator. Please reopen when desired.

hfinkel updated this revision to Diff 63347.Jul 8 2016, 4:03 PM

Rebased.

majnemer added inline comments.
lib/Transforms/Utils/InlineFunction.cpp
408–409

Formatting looks like it got weird.

hfinkel added inline comments.Jul 9 2016, 5:06 PM
lib/Transforms/Utils/InlineFunction.cpp
408–409

Indeed; tabs :(

hfinkel updated this revision to Diff 63407.Jul 9 2016, 5:07 PM

Remove tabs.

majnemer added inline comments.Jul 10 2016, 1:02 AM
lib/Transforms/Utils/InlineFunction.cpp
311–313

const auto *

404–406

Ditto.

429–430

I'd use the range variant, args()

462

Does it make sense to hoist this out of the loop and clear it on each iteration?

484

You could reduce indentation by doing a continue; if !isa<Instruction>.

488

auto *

hfinkel added inline comments.Oct 3 2016, 4:01 PM
lib/Transforms/Utils/InlineFunction.cpp
462

Actually, this is not needed at all. ArrayRef has a single-element constructor, and this array only ever has one element (oops).

hfinkel updated this revision to Diff 73366.Oct 3 2016, 4:05 PM

Rebased (and addressed reviewer feedback)

yakush added a subscriber: yakush.Aug 30 2017, 7:36 AM
marksl added a subscriber: marksl.Jul 31 2018, 11:19 AM
troyj added a subscriber: troyj.Aug 22 2018, 9:30 AM

Compare following testcases (which should have an equivalent result):

void set_nnn(int* pA, int* pB, int* dummy) {
  int* lpA; lpA=pA;
  int* lpB; lpB=pB;
  int* ldummy; ldummy=dummy;
  *lpA=42;
  *lpB=43;

  *ldummy=99;
}
 
int test_rr_nnn(int* pA, int* pB, int* dummy) {
  int* __restrict lpA; lpA=pA;
  int* __restrict lpB; lpB=pB;

  set_nnn(lpA,lpB,dummy);
  return *lpA;
}

versus:

int test_rr_local(int* pA, int* pB, int* dummy) {
  int* __restrict lpA; lpA=pA;
  int* __restrict lpB; lpB=pB;
  int* ldummy; ldummy=dummy;
  { // inlined code:
    int* l2pA; l2pA=lpA;
    int* l2pB; l2pB=lpB;
    int* l2dummy; l2dummy=ldummy;
    *l2pA=42;
    *l2pB=43;

    *l2dummy=99;
  }
  return *lpA;
}

After inlining, 'test_rr_nnn' will not contain !noalias references for the three store instructions that have been inlined.

While inlining, even when the callee does not have noalias args or noalias/scope metadata, it might be needed to add the information to the load/stores

lib/Transforms/Utils/InlineFunction.cpp
318

This early exit blocks propagation of the call's metadata, when the callee has no annotated instructions. This has the effect of blocking local restrict for the inlined call.