[PATCH] D114957: [AMDGPU] Change llvm.amdgcn.image.bvh.intersect.ray to take vec3 args

Yaxun Liu via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Dec 2 10:01:52 PST 2021


yaxunl added a comment.

In D114957#3166936 <https://reviews.llvm.org/D114957#3166936>, @foad wrote:

> In D114957#3166858 <https://reviews.llvm.org/D114957#3166858>, @yaxunl wrote:
>
>> In D114957#3166817 <https://reviews.llvm.org/D114957#3166817>, @foad wrote:
>>
>>> This is a flag-day change to the signatures of the LLVM intrinsics and the OpenCL builtins. Is that OK?
>>
>> This breaks users' code. If we have to do this, at least let clang emit a pre-defined macro e.g. `__amdgcn_bvh_use_vec3__`=1 so that users can make their code work before and after the change.
>
> I don't know anything about OpenCL macros. Is it good enough to put this in `AMDGPUTargetInfo::getTargetDefines`:
>
>   if (Opts.OpenCL)
>     Builder.defineMacro("__amdgcn_bvh_use_vec3__");
>
> Does it need tests, documentation, etc?

Yes you can add it to `AMDGPUTargetInfo::getTargetDefines` but without `if (Opts.OpenCL)` since HIP uses it too.

You can add a test to https://github.com/llvm/llvm-project/blob/b2a2c38349a18b89b04d342632d5ea02f86dfdd6/clang/test/Preprocessor/predefined-arch-macros.c#L3731


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114957



More information about the llvm-commits mailing list