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

================
Comment at: clang/test/OpenMP/target_data_codegen.cpp:355-356
 // Region 00
+// CK2-DAG: [[DEV:%[^,]+]] = sext i32 [[DEVi32:%[^,]+]] to i64
+// CK2-DAG: [[DEVi32]] = load i32, ptr %{{[^,]+}},
 // CK2: br i1 %{{[^,]+}}, label %[[IFTHEN:[^,]+]], label %[[IFELSE:[^,]+]]
----------------
jsjodin wrote:
> TIFitis wrote:
> > When both if clause and device clause are present, the device clause 
> > argument is inadvertently brought outside the `IfThen` region as we emit 
> > the `llvm:Value` from the `Clang::Expr` for it before making call to 
> > createTargetData.
> > 
> > I don't think this change would affect any use cases.
> > When both if clause and device clause are present, the device clause 
> > argument is inadvertently brought outside the `IfThen` region as we emit 
> > the `llvm:Value` from the `Clang::Expr` for it before making call to 
> > createTargetData.
> > 
> > I don't think this change would affect any use cases.
> 
> Is it at all possible that the load could cause an exception if moved outside 
> the if?
> 
Int pointer is not allowed inside device clause, so I don't think think the 
load can cause exceptions.

Also this would fix a scenario similar to the if clause, where in the following 
code `target_begin` would get device as //10// and `target_end` mapper call 
would get device as //100//.

```
int arg = 10;
#pragma omp target data map(to: arg) if(arg < 20) device(arg)
  {arg = 100;}
```



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150860

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to