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

[Kaushik Kulkarni via Phabricator via llvm-commits]

kaushikcfd marked 2 inline comments as done.
kaushikcfd added a comment.

In D75893#1932813 <https://reviews.llvm.org/D75893#1932813>, @jdoerfert wrote:

>


Thanks for taking a look at it!

> The entire potentially reachable thing breaks down if you put the `omp for` into another loop, doesn't it? Everything becomes potentially reachable from everything.

Unfortunately yes, it would've led to not setting the ranges of any loads from omp.(lb/ub).

> The situation you are after is, as far as I understand, as follows:
> 
>   store x in UB
>   call __kmpc_for_init_4(...)
>   v = load UB
>   store min(v, x) in UB
> 
> 
> correct?
> 
> Two alternative approaches:
> 
> 1. move the min computation (if necessary) into `__kmpc_for_init_4` so we don't see it here. All we would see is a single store. (=make sure the call never returns a value higher than the given upper bound) [preferred if plausible]

The newest patch does some minor changes in Clang to have it covered in our pattern-matching, which turned out  to be easier than this. Lmk if you have any problems with it.



================
Comment at: llvm/test/Transforms/OpenMP/set_bound_ranges.ll:2
+; RUN: opt -openmpopt -S < %s | FileCheck %s
+; RUN: opt -passes=openmpopt -S < %s | FileCheck %s
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
----------------
jdoerfert wrote:
> manually run -simplifycfg on this file to get rid of the control flow in favor of selects and then -instcombine to remove redundant loads.
Running these passes might not check the true IR fed from Clang to -openmpopt, which is the use case we are shooting for.


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

https://reviews.llvm.org/D75893