[PATCH] D45022: [X86] Mark all byval parameters as aliased

Sanjay Patel via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed May 2 08:46:43 PDT 2018


spatel added a comment.

In https://reviews.llvm.org/D45022#1084793, @wristow wrote:

> A couple minor test-tweak comments in-line.
>  As before, the code-change looks safe and conservative to me, in terms of the aliasing.  And my guess is that conservative aspect is fine, in that it won't seriously impact performance.  So I'm happy to say LGTM.  Does anyone with more experience in this area have any concerns?


I don't have any better suggestions about the code change, but let me chime in with a general comment about the test: it would be better to commit the test in trunk without this code change, so we have the baseline (miscompile). That way, if there are any temptations to revert the code change because of a perf problem, it will be clear that we would be reintroducing a miscompile.

Also, please use utils/update_mir_test_checks.py or utils/update_llc_test_checks.py to generate the CHECK lines. That's better than adding assertions by hand in almost all cases.


https://reviews.llvm.org/D45022





More information about the llvm-commits mailing list