[PATCH] D145721: [HIP] clang should pass `-mno-amdgpu-ieee` to -cc1

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In D145721#4182834 <https://reviews.llvm.org/D145721#4182834>, @tra wrote:

>> clang should pass `-mno-amdgpu-ieee` to -cc1
>
> It would be useful to have some details on why we should pass that option.

`-mamdgpu-ieee` was introduced by https://reviews.llvm.org/D77013 but I forgot to let clang driver pass it to clang -cc1. Recently we found that.



================
Comment at: clang/test/Driver/hip-options.hip:133
+// IEEE-ON-NOT: clang{{.*}} "-triple" "amdgcn-amd-amdhsa" {{.*}} "-mamdgpu-ieee"
+// IEEE-ON-NOT: clang{{.*}} "-triple" "amdgcn-amd-amdhsa" {{.*}} "-mno-amdgpu-ieee"
+
----------------
tra wrote:
> Nit: Both lines could be collapsed into matching `"-m{{(no-)?}}amdgpu-ieee"` Either way is fine.
will do


================
Comment at: clang/test/Driver/hip-options.hip:137-138
+// RUN:   --cuda-gpu-arch=gfx906  %s 2>&1 | FileCheck -check-prefixes=IEEE-OFF,IEEE-OFF-NEG %s
+// IEEE-OFF-NEG-NOT: clang{{.*}} "-triple" "amdgcn-amd-amdhsa" {{.*}} "-mamdgpu-ieee"
+// IEEE-OFF-DAG: clang{{.*}} "-triple" "amdgcn-amd-amdhsa" {{.*}} "-mno-amdgpu-ieee"
----------------
tra wrote:
> This looks odd. 
> If the DAG line matches on the first clang invocation, the NEG-NOT will always succeed, because it will have nothing to check.
> 
> If clang invocation with `-no-mamdgpu-ieee` happens to be before the invocation with `-mamdgpu-ieee`, then the NEG-NOT will not succeed.
> 
> Do I understand it correctly that the idea here is to make sure that only `-mno-amdgpu-ieee` is ever passed to cc1?
> What's the purpose of -DAG? Do you expect to see multiple cc1 with `"-triple" "amdgcn-amd-amdhsa"` ? AFAICT there should be only one for gfx906 and DAG should not be needed.
> 
> The -NOT check I'd do in a separate RUN.
> 
> 
The `-NEG` check is supposed to be a separate run. I will add a separate run for it and remove `-DAG`


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

https://reviews.llvm.org/D145721