This is an archive of the discontinued LLVM Phabricator instance.

llvm.noalias - Add IRBuilder support
Needs ReviewPublic

Authored by hfinkel on Apr 30 2015, 8:12 AM.

Details

Reviewers
chandlerc
Summary

This is part of the series started by D9375, and adds functions in IRBuilder to create the llvm.noalias intrinsics (this is used by later patches).

There is a workaround for some kinds of pointers we can't currently mangle. I don't think this is worth fixing in the mangling code because soon we'll have only an opaque pointer type and this will be irrelevant.

Diff Detail

Event Timeline

hfinkel updated this revision to Diff 24712.Apr 30 2015, 8:12 AM
hfinkel retitled this revision from to llvm.noalias - Add IRBuilder support.
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 added inline comments.May 12 2015, 3:41 PM
include/llvm/IR/IRBuilder.h
472

CreateNoAlias sounds confusing. Maybe, CreateNoAliasMarker? CreateNoAliasIntrinsic?

lib/IR/IRBuilder.cpp
206

I would rather see you just bitcast everything. Having one weird special case when we're moving to unified pointers anyways seems unjustified.

Very minor preference here; feel free to ignore.

217

Use getModule

220

If the original pointer is noalias, should the return also be noalias? I note that this is a different noalias per discussion on another thread. Slightly confusing to say the least.

Other attributes: nonnull, dereferenceable, etc..? I'd be fine with instcombine rules instead.

224

Using BitCastInst::Create might simplify these two lines.

reames resigned from this revision.Oct 8 2015, 10:24 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 added inline comments.Jul 7 2016, 8:08 PM
include/llvm/IR/IRBuilder.h
472

None of the other functions that create intrinsics have 'Intrinsic' in name, and so it seems weird to do it for this one. The best I can think of is 'CreateNoAliasPointer'.

lib/IR/IRBuilder.cpp
224

There is no such function (and the InsertPt variable is not an Instruction*, but a BB iterator ).

hfinkel updated this revision to Diff 63346.Jul 8 2016, 4:00 PM

Rebased, the function name was changed in response to Philip's feedback, and a unit test was added.

majnemer added inline comments.
lib/IR/IRBuilder.cpp
209

auto *STy

hfinkel updated this revision to Diff 73361.Oct 3 2016, 3:37 PM

Rebased (and addressed reviewer feedback).

troyj added a subscriber: troyj.Aug 22 2018, 9:30 AM