[PATCH] D135780: [IR] Switch everything to use memory attribute
Nikita Popov via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Oct 27 02:40:28 PDT 2022
nikic added a comment.
In D135780#3885060 <https://reviews.llvm.org/D135780#3885060>, @nhaehnle wrote:
> Ordinarily, I would say this patch is a bit too big for comfort. That said, I did look at most of the llvm changes minus the attributor and sampled some of the test changes, and it looked good to me.
Something I considered originally was to try and support both the old attributes and the new one at the same time, to allow a more incremental change. However, it is hard to make sure that the resulting scheme is correct, e.g. code that drops things like argmemonly for correctness reasons would also have to drop it from the memory attribute, and doing that transparently would only be possible if we add a big backwards-compatibility layer into the core AttributeList and AttributeSet implementation, which maps things like "remove ArgMemOnly" to a more complex operation. Trying to do that seemed like a pretty bad idea. It also wouldn't do anything about the test churn, which makes up most of the patch. I don't have good ideas on how to split this down further.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D135780/new/
https://reviews.llvm.org/D135780
More information about the llvm-commits
mailing list