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