[PATCH] D130466: [LICM] - Add option to allow data races

[Aditya Kumar via Phabricator via llvm-commits]

hiraditya added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/LICM.cpp:2119
   if (!SafeToInsertStore) {
-    if (IsKnownThreadLocalObject)
+    if (IsKnownThreadLocalObject || AllowDataRaces)
       SafeToInsertStore = true;
----------------
xbolva00 wrote:
> xbolva00 wrote:
> > gsocshubham wrote:
> > > gsocshubham wrote:
> > > > xbolva00 wrote:
> > > > > fhahn wrote:
> > > > > > gsocshubham wrote:
> > > > > > > efriedma wrote:
> > > > > > > > I think you need a few more safety checks, even if you're assuming the program is single-threaded.  Just because it's legal to load from a memory location, doesn't mean it's legal to store to it.  For example, you can't write to a constant.
> > > > > > > I am not sure about writing to a constant. Can you elaborate with an example where it would fail?
> > > > > > This doesn't like quite right. You are just comparing the enum value, not the actual configured ThreadModel I think. You likely have to check the value returned by `getThreadModel`.
> > > > > and no tests?
> > > > `getThreadModel()` is declared here - https://github.com/llvm/llvm-project/blob/d0541b47000739c68c540170c6b9790ec1ea3b77/llvm/include/llvm/CodeGen/CommandFlags.h#L42
> > > > 
> > > > Defination is present in clang - https://github.com/llvm/llvm-project/blob/448adfee05b737a26dda34e7ae2cd4948760fff0/clang/include/clang/Driver/ToolChain.h#L573
> > > > 
> > > > ` virtual std::string getThreadModel() const { return "posix"; }`
> > > > 
> > > > I am not sure on how can I use `getThreadModel()` in LICM.cpp. Is there any other way to know ThreadModel?
> > > > 
> > > I have added a test here - `hoist-load-without-store.ll` where load is hoisted out of loop.
> > > 
> > > I did not get what do you mean by no tests?
> > So maybe clang can run llvm with
> > 
> > -mllvm -your-new-flag-to-allow-data-races 
> > 
> > if threadmodel is single?
> Tests that we perform this optimization only when threadmodel is single
Yeah, we need test checks the optimization was performed when threadmodel is single, and does not do the optimization otherwise.


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

https://reviews.llvm.org/D130466