Re: [Mesa-dev] [PATCH] ac: Use old kill intrinsics for LLVM 6.

[Bas Nieuwenhuizen]

On Tue, Jan 30, 2018 at 12:32 AM, Samuel Pitoiset
 wrote:
>
>
> On 01/30/2018 12:03 AM, Bas Nieuwenhuizen wrote:
>>
>> llvm.amdgcn.kill is currently broken for SGPRs. The old intrinsic
>> had that issue too, but did not fold the preceding comparison into
>> the machine instruction in LLVM. As the preceding comparison is
>> often a float comparison, this results in a VGPR, essentially hiding
>> the issue.
>>
>> I have a fix outstanding for LLVM to make the new intrinsic work,
>> but as getting stuff reviewed, committed and cherry-picked in LLVM
>> takes quite a while, I'd like to be safe and disable the new
>> intrinsic with LLVM 6.
>>
>> We can always revert when the fix is in LLVM.
>
>
> Yes, I do agree.
>
> Reviewed-by: Samuel Pitoiset 

The LLVM fix got in and was cherry-picked suprisingly quick, so I will
drop this patch.
>
>
>>
>> Fixes: ad2b3b2a9c "ac: replace llvm.AMDGPU.kilp by llvm.amdgcn.kill with
>> LLVM 6"
>> ---
>>
>> The LLVM fix is available at https://reviews.llvm.org/D42302
>>
>>   src/amd/common/ac_llvm_build.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/src/amd/common/ac_llvm_build.c
>> b/src/amd/common/ac_llvm_build.c
>> index 5e08508fed..a34eda1037 100644
>> --- a/src/amd/common/ac_llvm_build.c
>> +++ b/src/amd/common/ac_llvm_build.c
>> @@ -1479,7 +1479,7 @@ LLVMValueRef ac_build_wqm_vote(struct
>> ac_llvm_context *ctx, LLVMValueRef i1)
>> void ac_build_kill_if_false(struct ac_llvm_context *ctx, LLVMValueRef
>> i1)
>>   {
>> -   if (HAVE_LLVM >= 0x0600) {
>> +   if (HAVE_LLVM >= 0x0700) {
>> ac_build_intrinsic(ctx, "llvm.amdgcn.kill", ctx->voidt,
>>, 1, 0);
>> return;
>>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] ac: Use old kill intrinsics for LLVM 6.

[Samuel Pitoiset]

On 01/30/2018 12:03 AM, Bas Nieuwenhuizen wrote:

llvm.amdgcn.kill is currently broken for SGPRs. The old intrinsic
had that issue too, but did not fold the preceding comparison into
the machine instruction in LLVM. As the preceding comparison is
often a float comparison, this results in a VGPR, essentially hiding
the issue.

I have a fix outstanding for LLVM to make the new intrinsic work,
but as getting stuff reviewed, committed and cherry-picked in LLVM
takes quite a while, I'd like to be safe and disable the new
intrinsic with LLVM 6.

We can always revert when the fix is in LLVM.


Yes, I do agree.

Reviewed-by: Samuel Pitoiset 



Fixes: ad2b3b2a9c "ac: replace llvm.AMDGPU.kilp by llvm.amdgcn.kill with LLVM 6"
---

The LLVM fix is available at https://reviews.llvm.org/D42302

  src/amd/common/ac_llvm_build.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/amd/common/ac_llvm_build.c b/src/amd/common/ac_llvm_build.c
index 5e08508fed..a34eda1037 100644
--- a/src/amd/common/ac_llvm_build.c
+++ b/src/amd/common/ac_llvm_build.c
@@ -1479,7 +1479,7 @@ LLVMValueRef ac_build_wqm_vote(struct ac_llvm_context 
*ctx, LLVMValueRef i1)
  
  void ac_build_kill_if_false(struct ac_llvm_context *ctx, LLVMValueRef i1)

  {
-   if (HAVE_LLVM >= 0x0600) {
+   if (HAVE_LLVM >= 0x0700) {
ac_build_intrinsic(ctx, "llvm.amdgcn.kill", ctx->voidt,
   , 1, 0);
return;


___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] ac: Use old kill intrinsics for LLVM 6.

[Bas Nieuwenhuizen]

llvm.amdgcn.kill is currently broken for SGPRs. The old intrinsic
had that issue too, but did not fold the preceding comparison into
the machine instruction in LLVM. As the preceding comparison is
often a float comparison, this results in a VGPR, essentially hiding
the issue.

I have a fix outstanding for LLVM to make the new intrinsic work,
but as getting stuff reviewed, committed and cherry-picked in LLVM
takes quite a while, I'd like to be safe and disable the new
intrinsic with LLVM 6.

We can always revert when the fix is in LLVM.

Fixes: ad2b3b2a9c "ac: replace llvm.AMDGPU.kilp by llvm.amdgcn.kill with LLVM 6"
---

The LLVM fix is available at https://reviews.llvm.org/D42302

 src/amd/common/ac_llvm_build.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/amd/common/ac_llvm_build.c b/src/amd/common/ac_llvm_build.c
index 5e08508fed..a34eda1037 100644
--- a/src/amd/common/ac_llvm_build.c
+++ b/src/amd/common/ac_llvm_build.c
@@ -1479,7 +1479,7 @@ LLVMValueRef ac_build_wqm_vote(struct ac_llvm_context 
*ctx, LLVMValueRef i1)
 
 void ac_build_kill_if_false(struct ac_llvm_context *ctx, LLVMValueRef i1)
 {
-   if (HAVE_LLVM >= 0x0600) {
+   if (HAVE_LLVM >= 0x0700) {
ac_build_intrinsic(ctx, "llvm.amdgcn.kill", ctx->voidt,
   , 1, 0);
return;
-- 
2.16.1

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev