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

[Dávid Bolvanský via Phabricator via llvm-commits]

xbolva00 added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/LICM.cpp:2119
   if (!SafeToInsertStore) {
-    if (IsKnownThreadLocalObject)
+    if (IsKnownThreadLocalObject || AllowDataRaces)
       SafeToInsertStore = true;
----------------
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?


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

https://reviews.llvm.org/D130466