[PATCH] D130466: [LICM] - Add option to force thread model single

[Shubham Narlawar via Phabricator via llvm-commits]

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.
> - 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.
> - We should have a variant of that test with a constant global (should have no store promotion, independent of the flag).
> - We should have a variant of the test with a function argument (unknown if writable, so should have no store promotion).
> - 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).

Should I keep both prefix checks with and without flag? That is increasing number of lines of code in the testcase.

After reducing the testcase with bare minumum load/store licm opt - Below are number of lines

promote-sink-atomic-store-arg.ll - 30 lines
promote-sink-store-arg.ll - 30 lines
promote-sink-store-capture.ll - 40 lines
promote-sink-store-constant-global.ll - 20 lines
promote-sink-store-global.ll - 20 lines
promote-sink-store.ll - 35 lines

+ checks generated by `llvm/utils`
+ these includes spaces in between


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

https://reviews.llvm.org/D130466