This is an archive of the discontinued LLVM Phabricator instance.

Use dereferencable attribute in Clang for C++ references
ClosedPublic

Authored by hfinkel on Jul 9 2014, 8:25 PM.

Details

Summary

This is the Clang companion patch to http://reviews.llvm.org/D4449 which adds a dereferencable function parameter attribute (like the nonnull attribute, but specifying that the pointer is dereferencable like the value of an alloca is dereferencable). This patch causes the dereferencable attribute to appear on reference parameters (and return values) just like nonnull does currently.

As mentioned in the llvm patch summary, my motivating use case is C++ references that are passes as function parameters and used in loops. For example,

void foo(int * restrict a, int * restrict b, int &c, int n) {

for (int i = 0; i < n; ++i)
  if (a[i] > 0)
    a[i] = c*b[i];

}

Currently, we generate a load of 'c' in every loop iteration because we can't prove that there is no control dependence on the dereferencability of 'c' from the if condition. But because 'c' is a reference, we know it must point to something, and since nothing else in the loop can alias it, it is safe to load it in the preheader.

Diff Detail

Event Timeline

hfinkel updated this revision to Diff 11239.Jul 9 2014, 8:25 PM
hfinkel retitled this revision from to Use dereferencable attribute in Clang for C++ references.
hfinkel updated this object.
hfinkel edited the test plan for this revision. (Show Details)
hfinkel added reviewers: rsmith, chandlerc, nlewycky.
hfinkel added a subscriber: Unknown Object (MLST).
hfinkel updated this revision to Diff 11294.Jul 10 2014, 2:14 PM

Updated for changes in the LLVM patch (in response to Nick's review).

There also need to be a lot of regression-test updates (not yet included here).

hfinkel updated this revision to Diff 11307.Jul 10 2014, 9:47 PM

Updated patch (associated with the updated LLVM patch). There is no longer a size limit, so that concern no longer applies.

I've changed the logic slightly, so that:

  • We add the dereferenceable attribute for complete (constant-sized) types.
  • Otherwise, in addrspace(0), we add nonnull

(this is a slight change in behavior, because we used to add nonnull even for non-addrspace(0) references, but I think this is wrong (and I'm missing a test case for this, and I'll certainly add one)).

Alternatively, we could add dereferenceable(1) for incomplete types instead of falling back to nonnull. Preferences?

hfinkel updated this revision to Diff 11310.Jul 10 2014, 10:50 PM

Fixed up the remaining regression tests (now all pass), and added a new test for dereferenceable/nonnull on non-addrspace(0) references.

hfinkel accepted this revision.Jul 18 2014, 9:05 AM
hfinkel added a reviewer: hfinkel.

Richard accepted, will close.

This revision is now accepted and ready to land.Jul 18 2014, 9:05 AM
hfinkel closed this revision.Jul 18 2014, 9:06 AM

r213386, thanks!