[PATCH] D130466: [LICM] - Add option to allow data races
Shubham Narlawar via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Sep 28 12:59:45 PDT 2022
gsocshubham added a comment.
In D130466#3783514 <https://reviews.llvm.org/D130466#3783514>, @nikic wrote:
> I believe the logic is now correct, but testing could be improved. Some suggestions:
>
> - Use a single file with two RUN lines, only one of which has `-licm-force-thread-model-single`. Combined with `--check-prefixes`, this will make it clear in which cases behavior differs based on the flag and in which cases it is the same.
Added 2 RUN lines in all the LIT tests.
> - The current test cases look too complicated. For globals, I believe a very simple loop that just loads / stores the global inside the loop should be enough. Without the flag this will lead to only load promotion, and with it to store promotion.
I have used a simple loop to achieve above.
> - We should have a variant of that test with a constant global (should have no store promotion, independent of the flag).
Done.
> - We should have a variant of the test with a function argument (unknown if writable, so should have no store promotion).
Done.
> - We should have a variant of the test with an alloca that is captured (store promotion legal with the flag, because capture does not impact thread safety).
Done.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D130466/new/
https://reviews.llvm.org/D130466
More information about the llvm-commits
mailing list