[PATCH] D77689: [X86] Codegen for preallocated

Arthur Eubanks via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue May 5 09:41:42 PDT 2020


aeubanks added a comment.

In D77689#2016748 <https://reviews.llvm.org/D77689#2016748>, @jdoerfert wrote:

> Nit: The commit name is still call setup.
>  Did the lang ref patch land already? We should link it in the commit message.


Done.
Is there a way to automatically update the Phab change with the commit message if you're only uploading one commit?

> The `llvm/lib/Transforms/IPO` look straight forward except the one in `llvm/lib/Transforms/IPO/GlobalOpt.cpp`. I would suggest to split that one off and add a comment to the new code explaining what is happening and why. It also seems that not all IPO passes have tests with the new attribute. Don't wait for me to review the rest or to accept this.

Specifically for `GlobalOpt.cpp`, I realized that I probably shouldn't have done that since we haven't implemented the part where you strip off the operand bundle and replace the calls to the intrinsics with something like `alloca`. So I removed that and added a TODO.
In general I went through all the IPO transforms and did some archaeology to see what tests they had. I did end up finding more tests to copy/add to, but some also never had tests (not a great excuse)... Anyway thanks for the quick review!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77689/new/

https://reviews.llvm.org/D77689





More information about the llvm-commits mailing list