Page MenuHomePhabricator

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
554

CreateNoAlias sounds confusing. Maybe, CreateNoAliasMarker? CreateNoAliasIntrinsic?

lib/IR/IRBuilder.cpp
347

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.

358

Use getModule

361

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.

365

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
554

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
365

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
350

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