[PATCH] D93503: [GVN] Propagate llvm.access.group metadata of loads

[KAWASHIMA Takahiro via Phabricator via llvm-commits]

kawashima-fj marked 2 inline comments as done.
kawashima-fj added a comment.

I'm sorry for the late response. I (and my colleague) updated the test.

In D93503#2484226 <https://reviews.llvm.org/D93503#2484226>, @fhahn wrote:

> Thanks for cleaning up the test. It left a few additional small suggestions. I think for that test it would be good to add CHECK lines for all code generated during PRE. Given it is quite small, you could use `llvm/utils/update_test_checks.py` to auto-generate the check lines. Also, I think it would be good to pre-commit the test with check lines for current trunk and then update the patch to just show the diff in CHECK lines with this patch.

You mean I should push two commits to the GitHub repository, right?
The first one is only the test file with CHECK-lines generated by `update_test_checks.py` with current `master` branch.
The second one is update of the GVN source file and CHECK-lines generated by `update_test_checks.py` with the new source code.
I'm newbie to LLVM. Your suggestion is very helpful. Thanks.



================
Comment at: llvm/test/Transforms/GVN/PRE/load-pre-metadata-accsess-group.ll:31
+!1 = distinct !{}
+!2 = distinct !{!2, !3, !4}
+!3 = !{!"llvm.loop.parallel_accesses", !1}
----------------
fhahn wrote:
> Is the `!llvm.loop` metadata needed?
`llvm.loop` is removed.


================
Comment at: llvm/test/Transforms/GVN/PRE/load-pre-metadata-accsess-group.ll:33
+!3 = !{!"llvm.loop.parallel_accesses", !1}
+!4 = !{!"llvm.loop.vectorize.enable", i1 true}
----------------
fhahn wrote:
> That should not be needed.
`llvm.loop.vectorize.enable` is removed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93503