[PATCH] D75893: [OpenMP] Add !range metadata to loads from omp.(ub/lb)

[Kaushik Kulkarni via Phabricator via llvm-commits]

kaushikcfd marked an inline comment as done.
kaushikcfd added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:249
+            }
+          };
+
----------------
jdoerfert wrote:
> kaushikcfd wrote:
> > jdoerfert wrote:
> > > You need to verify there is only a single load/store and the value is an alloca without initializer and the type is as expected.
> > > verify there is only a single load/store
> > There might be multiple loads/store to `%omp.ub` arising from `EnsureUpperBound`. So, we only pick the store just before and the load just after the `static_for` invocation.
> > 
> > >  value is an alloca without initializer
> > Is the "value" begin discussed here `V`?
> If there are multiple, how do you ensure you pick up the right one? We probably need to find all and use the upper/lower bound of all stored values to be correct. We also need a test for multiple loads/stores.
> 
> I mean we assume `OMPLBVal` and `OMPUBVal` are alloca's, right? We need to make sure we only pattern match a situation we can "control"
Agreed, the current implementation is broken. I've added a FIXME for it. Looks like we should do some dataflow analysis to figure out the correct range.

> I mean we assume OMPLBVal and OMPUBVal are alloca's, right? 
Yep, made the assumption explicit in the latest revision.


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

https://reviews.llvm.org/D75893