[PATCH] D36800: Add rewrite by-reference parameter pass

Tobias Grosser via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Aug 16 22:25:21 PDT 2017


grosser marked 6 inline comments as done.
grosser added a comment.

Thanks for the nice review. I addressed all but one comment, which can likely be improved in a subsequent commit. Will go ahead and commit this now.



================
Comment at: lib/Transform/RewriteByReferenceParameters.cpp:42
+
+    if (Inst.getParent() == Entry)
+      return;
----------------
bollu wrote:
> Could you please document why this is necessary?
I generalized the code for this to not be necessary.


================
Comment at: lib/Transform/RewriteByReferenceParameters.cpp:65
+
+    auto *Alloca = dyn_cast<AllocaInst>(BitCast->getOperand(0));
+
----------------
bollu wrote:
> Perhaps we can make this slightly nicer with the pattern matching utilities. If not, I believe we should at least use the `Operator` versions so this can work on constant versions of the operands as well.
 Not sure what you mean. As this seems to be an extension of this functionality, I will go ahead without this and we can improve this in a subsequent commit.


https://reviews.llvm.org/D36800





More information about the llvm-commits mailing list