[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