[PATCH] D153924: [OpenMP] Allow exceptions in target regions when offloading to GPUs

Mon Sep 11 14:57:58 PDT 2023

aeubanks added inline comments.

================
Comment at: clang/test/OpenMP/amdgpu_throw_trap.cpp:4
+// RUN: %clang_cc1 -fopenmp -triple amdgcn-amd-amdhsa -fopenmp-is-target-device %s -emit-llvm -S -Wno-openmp-target-exception -o - | FileCheck -check-prefix=DEVICE %s
+// RUN: %clang_cc1 -fopenmp -triple x86_64-pc-linux-gnu -fopenmp-is-target-device -fcxx-exceptions %s -emit-llvm -S -Wno-openmp-target-exception -o - | FileCheck -check-prefix=HOST %s
+// DEVICE: s_trap
----------------
thakis wrote:
> aeubanks wrote:
> > thakis wrote:
> > > This test fails if X86 isn't in `LLVM_TARGETS_TO_BUILD` and the host system is some non-x86 system (e.g. arm64).
> > > 
> > > (This is the only test in check-clang that fails then.)
> > > 
> > > Should this test grow a `REQUIRES: x86-registered-target`? Should it use `%itanium_abi_triple` instead of `x86_64-pc-linux-gnu`? (It seems to pass when replacing `x86_64-pc-linux-gnu` with `%itanium_abi_triple` on my arm mac.)
> > added x86-registered-target in 238a1ef44f4f2361205e538b3cb7ebc5ec70894d
> Is that better than `%itanium_abi_triple`?
I was worried about LLVM failing if the calculated `%itanium_abi_triple` wasn't supported in that build of LLVM, but TIL that clang/LLVM can handle triples it doesn't recognize all the way until the codegen phase. But IIUC optimizations can change depending on whether or not LLVM recognizes the triple so it's still a little inconsistent.

so yeah `%itanium_abi_triple` would probably work, but it seems susceptible to configuration differences

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153924