[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