Re: [Mesa-dev] [PATCH 2/2] radv: ensure export arguments are always float

2018-12-13 Thread Samuel Pitoiset



On 12/13/18 3:45 PM, Rhys Perry wrote:

(accidently sent an incomplete email)

Seems my LLVM configuration was messed up and I might have used my
distro's LLVM too.

LLVM 8 and 7 with a release build passes.

A debug build of 8 (and my messed up builds of 7 and 8 which I thought
were release ones) results in an assert.


Just tried to reproduce with a LLVM 8 debug build, I don't get an 
assertion. Maybe I messed up my build too?



On Thu, 13 Dec 2018 at 08:38, Samuel Pitoiset  wrote:




On 12/6/18 3:18 PM, Rhys Perry wrote:

./deqp-vk 
--deqp-case=dEQP-VK.spirv_assembly.instruction.graphics.16bit_storage.input_output_int_32_to_16.scalar_uint0_frag
should crash with something like:
deqp-vk: lib/IR/Instructions.cpp:2590: static llvm::CastInst*
llvm::CastInst::Create(llvm::Instruction::CastOps, llvm::Value*,
llvm::Type*, const llvm::Twine&, llvm::Instruction*): Assertion
`castIsValid(op, S, Ty) && "Invalid cast!"' failed.
because it's trying to zext/sext a half float to a i32.

and ./deqp-vk 
--deqp-case=dEQP-VK.spirv_assembly.instruction.graphics.16bit_storage.input_output_int_32_to_16.scalar_uint0_vert
should crash with something like:
deqp-vk: lib/IR/Instructions.cpp:348: void
llvm::CallInst::init(llvm::FunctionType*, llvm::Value*,
llvm::ArrayRef,
llvm::ArrayRef >, const
llvm::Twine&): Assertion `(i >= FTy->getNumParams() ||
FTy->getParamType(i) == Args[i]->getType()) && "Calling a function
with a bad signature!"' failed.
because it's calling the export intrinsic with incorrect argument types.

For both tests, it seems to only assert with LLVM 8 for some reason.


I guess you use a debug llvm build? Can you figure out what change
introduces this crash?


On Thu, 6 Dec 2018 at 13:31, Samuel Pitoiset  wrote:




On 12/6/18 2:15 PM, Rhys Perry wrote:

So that the signature is correct and consistent, the inputs to a export
intrinsic should always be 32-bit floats.

This and the previous commit fixes a large amount crashes from
dEQP-VK.spirv_assembly.instruction.graphics.16bit_storage.input_output_int_*
tests



They don't crash for me? Please explain how to reproduce.


Fixes: b722b29f10d ('radv: add support for 16bit input/output')
Signed-off-by: Rhys Perry 
---
src/amd/vulkan/radv_nir_to_llvm.c | 6 +-
1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/src/amd/vulkan/radv_nir_to_llvm.c 
b/src/amd/vulkan/radv_nir_to_llvm.c
index 0c91118e5a..90bcc8dbfe 100644
--- a/src/amd/vulkan/radv_nir_to_llvm.c
+++ b/src/amd/vulkan/radv_nir_to_llvm.c
@@ -2464,12 +2464,8 @@ si_llvm_init_export_args(struct radv_shader_context *ctx,
} else
memcpy(>out[0], values, sizeof(values[0]) * 4);

- for (unsigned i = 0; i < 4; ++i) {
- if (!(args->enabled_channels & (1 << i)))
- continue;
-
+ for (unsigned i = 0; i < 4; ++i)
args->out[i] = ac_to_float(>ac, args->out[i]);
- }
}

static void


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


Re: [Mesa-dev] [PATCH 2/2] radv: ensure export arguments are always float

2018-12-13 Thread Rhys Perry
(accidently sent an incomplete email)

Seems my LLVM configuration was messed up and I might have used my
distro's LLVM too.

LLVM 8 and 7 with a release build passes.

A debug build of 8 (and my messed up builds of 7 and 8 which I thought
were release ones) results in an assert.
On Thu, 13 Dec 2018 at 08:38, Samuel Pitoiset  wrote:
>
>
>
> On 12/6/18 3:18 PM, Rhys Perry wrote:
> > ./deqp-vk 
> > --deqp-case=dEQP-VK.spirv_assembly.instruction.graphics.16bit_storage.input_output_int_32_to_16.scalar_uint0_frag
> > should crash with something like:
> > deqp-vk: lib/IR/Instructions.cpp:2590: static llvm::CastInst*
> > llvm::CastInst::Create(llvm::Instruction::CastOps, llvm::Value*,
> > llvm::Type*, const llvm::Twine&, llvm::Instruction*): Assertion
> > `castIsValid(op, S, Ty) && "Invalid cast!"' failed.
> > because it's trying to zext/sext a half float to a i32.
> >
> > and ./deqp-vk 
> > --deqp-case=dEQP-VK.spirv_assembly.instruction.graphics.16bit_storage.input_output_int_32_to_16.scalar_uint0_vert
> > should crash with something like:
> > deqp-vk: lib/IR/Instructions.cpp:348: void
> > llvm::CallInst::init(llvm::FunctionType*, llvm::Value*,
> > llvm::ArrayRef,
> > llvm::ArrayRef >, const
> > llvm::Twine&): Assertion `(i >= FTy->getNumParams() ||
> > FTy->getParamType(i) == Args[i]->getType()) && "Calling a function
> > with a bad signature!"' failed.
> > because it's calling the export intrinsic with incorrect argument types.
> >
> > For both tests, it seems to only assert with LLVM 8 for some reason.
>
> I guess you use a debug llvm build? Can you figure out what change
> introduces this crash?
>
> > On Thu, 6 Dec 2018 at 13:31, Samuel Pitoiset  
> > wrote:
> >>
> >>
> >>
> >> On 12/6/18 2:15 PM, Rhys Perry wrote:
> >>> So that the signature is correct and consistent, the inputs to a export
> >>> intrinsic should always be 32-bit floats.
> >>>
> >>> This and the previous commit fixes a large amount crashes from
> >>> dEQP-VK.spirv_assembly.instruction.graphics.16bit_storage.input_output_int_*
> >>> tests
> >>>
> >>
> >> They don't crash for me? Please explain how to reproduce.
> >>
> >>> Fixes: b722b29f10d ('radv: add support for 16bit input/output')
> >>> Signed-off-by: Rhys Perry 
> >>> ---
> >>>src/amd/vulkan/radv_nir_to_llvm.c | 6 +-
> >>>1 file changed, 1 insertion(+), 5 deletions(-)
> >>>
> >>> diff --git a/src/amd/vulkan/radv_nir_to_llvm.c 
> >>> b/src/amd/vulkan/radv_nir_to_llvm.c
> >>> index 0c91118e5a..90bcc8dbfe 100644
> >>> --- a/src/amd/vulkan/radv_nir_to_llvm.c
> >>> +++ b/src/amd/vulkan/radv_nir_to_llvm.c
> >>> @@ -2464,12 +2464,8 @@ si_llvm_init_export_args(struct 
> >>> radv_shader_context *ctx,
> >>>} else
> >>>memcpy(>out[0], values, sizeof(values[0]) * 4);
> >>>
> >>> - for (unsigned i = 0; i < 4; ++i) {
> >>> - if (!(args->enabled_channels & (1 << i)))
> >>> - continue;
> >>> -
> >>> + for (unsigned i = 0; i < 4; ++i)
> >>>args->out[i] = ac_to_float(>ac, args->out[i]);
> >>> - }
> >>>}
> >>>
> >>>static void
> >>>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 2/2] radv: ensure export arguments are always float

2018-12-13 Thread Rhys Perry
Seems my LLVM configuration was messed up and I might have used my
distro's LLVM too.

On Thu, 13 Dec 2018 at 08:38, Samuel Pitoiset  wrote:
>
>
>
> On 12/6/18 3:18 PM, Rhys Perry wrote:
> > ./deqp-vk 
> > --deqp-case=dEQP-VK.spirv_assembly.instruction.graphics.16bit_storage.input_output_int_32_to_16.scalar_uint0_frag
> > should crash with something like:
> > deqp-vk: lib/IR/Instructions.cpp:2590: static llvm::CastInst*
> > llvm::CastInst::Create(llvm::Instruction::CastOps, llvm::Value*,
> > llvm::Type*, const llvm::Twine&, llvm::Instruction*): Assertion
> > `castIsValid(op, S, Ty) && "Invalid cast!"' failed.
> > because it's trying to zext/sext a half float to a i32.
> >
> > and ./deqp-vk 
> > --deqp-case=dEQP-VK.spirv_assembly.instruction.graphics.16bit_storage.input_output_int_32_to_16.scalar_uint0_vert
> > should crash with something like:
> > deqp-vk: lib/IR/Instructions.cpp:348: void
> > llvm::CallInst::init(llvm::FunctionType*, llvm::Value*,
> > llvm::ArrayRef,
> > llvm::ArrayRef >, const
> > llvm::Twine&): Assertion `(i >= FTy->getNumParams() ||
> > FTy->getParamType(i) == Args[i]->getType()) && "Calling a function
> > with a bad signature!"' failed.
> > because it's calling the export intrinsic with incorrect argument types.
> >
> > For both tests, it seems to only assert with LLVM 8 for some reason.
>
> I guess you use a debug llvm build? Can you figure out what change
> introduces this crash?
>
> > On Thu, 6 Dec 2018 at 13:31, Samuel Pitoiset  
> > wrote:
> >>
> >>
> >>
> >> On 12/6/18 2:15 PM, Rhys Perry wrote:
> >>> So that the signature is correct and consistent, the inputs to a export
> >>> intrinsic should always be 32-bit floats.
> >>>
> >>> This and the previous commit fixes a large amount crashes from
> >>> dEQP-VK.spirv_assembly.instruction.graphics.16bit_storage.input_output_int_*
> >>> tests
> >>>
> >>
> >> They don't crash for me? Please explain how to reproduce.
> >>
> >>> Fixes: b722b29f10d ('radv: add support for 16bit input/output')
> >>> Signed-off-by: Rhys Perry 
> >>> ---
> >>>src/amd/vulkan/radv_nir_to_llvm.c | 6 +-
> >>>1 file changed, 1 insertion(+), 5 deletions(-)
> >>>
> >>> diff --git a/src/amd/vulkan/radv_nir_to_llvm.c 
> >>> b/src/amd/vulkan/radv_nir_to_llvm.c
> >>> index 0c91118e5a..90bcc8dbfe 100644
> >>> --- a/src/amd/vulkan/radv_nir_to_llvm.c
> >>> +++ b/src/amd/vulkan/radv_nir_to_llvm.c
> >>> @@ -2464,12 +2464,8 @@ si_llvm_init_export_args(struct 
> >>> radv_shader_context *ctx,
> >>>} else
> >>>memcpy(>out[0], values, sizeof(values[0]) * 4);
> >>>
> >>> - for (unsigned i = 0; i < 4; ++i) {
> >>> - if (!(args->enabled_channels & (1 << i)))
> >>> - continue;
> >>> -
> >>> + for (unsigned i = 0; i < 4; ++i)
> >>>args->out[i] = ac_to_float(>ac, args->out[i]);
> >>> - }
> >>>}
> >>>
> >>>static void
> >>>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 2/2] radv: ensure export arguments are always float

2018-12-13 Thread Samuel Pitoiset



On 12/6/18 3:18 PM, Rhys Perry wrote:

./deqp-vk 
--deqp-case=dEQP-VK.spirv_assembly.instruction.graphics.16bit_storage.input_output_int_32_to_16.scalar_uint0_frag
should crash with something like:
deqp-vk: lib/IR/Instructions.cpp:2590: static llvm::CastInst*
llvm::CastInst::Create(llvm::Instruction::CastOps, llvm::Value*,
llvm::Type*, const llvm::Twine&, llvm::Instruction*): Assertion
`castIsValid(op, S, Ty) && "Invalid cast!"' failed.
because it's trying to zext/sext a half float to a i32.

and ./deqp-vk 
--deqp-case=dEQP-VK.spirv_assembly.instruction.graphics.16bit_storage.input_output_int_32_to_16.scalar_uint0_vert
should crash with something like:
deqp-vk: lib/IR/Instructions.cpp:348: void
llvm::CallInst::init(llvm::FunctionType*, llvm::Value*,
llvm::ArrayRef,
llvm::ArrayRef >, const
llvm::Twine&): Assertion `(i >= FTy->getNumParams() ||
FTy->getParamType(i) == Args[i]->getType()) && "Calling a function
with a bad signature!"' failed.
because it's calling the export intrinsic with incorrect argument types.

For both tests, it seems to only assert with LLVM 8 for some reason.


I guess you use a debug llvm build? Can you figure out what change 
introduces this crash?



On Thu, 6 Dec 2018 at 13:31, Samuel Pitoiset  wrote:




On 12/6/18 2:15 PM, Rhys Perry wrote:

So that the signature is correct and consistent, the inputs to a export
intrinsic should always be 32-bit floats.

This and the previous commit fixes a large amount crashes from
dEQP-VK.spirv_assembly.instruction.graphics.16bit_storage.input_output_int_*
tests



They don't crash for me? Please explain how to reproduce.


Fixes: b722b29f10d ('radv: add support for 16bit input/output')
Signed-off-by: Rhys Perry 
---
   src/amd/vulkan/radv_nir_to_llvm.c | 6 +-
   1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/src/amd/vulkan/radv_nir_to_llvm.c 
b/src/amd/vulkan/radv_nir_to_llvm.c
index 0c91118e5a..90bcc8dbfe 100644
--- a/src/amd/vulkan/radv_nir_to_llvm.c
+++ b/src/amd/vulkan/radv_nir_to_llvm.c
@@ -2464,12 +2464,8 @@ si_llvm_init_export_args(struct radv_shader_context *ctx,
   } else
   memcpy(>out[0], values, sizeof(values[0]) * 4);

- for (unsigned i = 0; i < 4; ++i) {
- if (!(args->enabled_channels & (1 << i)))
- continue;
-
+ for (unsigned i = 0; i < 4; ++i)
   args->out[i] = ac_to_float(>ac, args->out[i]);
- }
   }

   static void


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


Re: [Mesa-dev] [PATCH 2/2] radv: ensure export arguments are always float

2018-12-06 Thread Rhys Perry
./deqp-vk 
--deqp-case=dEQP-VK.spirv_assembly.instruction.graphics.16bit_storage.input_output_int_32_to_16.scalar_uint0_frag
should crash with something like:
deqp-vk: lib/IR/Instructions.cpp:2590: static llvm::CastInst*
llvm::CastInst::Create(llvm::Instruction::CastOps, llvm::Value*,
llvm::Type*, const llvm::Twine&, llvm::Instruction*): Assertion
`castIsValid(op, S, Ty) && "Invalid cast!"' failed.
because it's trying to zext/sext a half float to a i32.

and ./deqp-vk 
--deqp-case=dEQP-VK.spirv_assembly.instruction.graphics.16bit_storage.input_output_int_32_to_16.scalar_uint0_vert
should crash with something like:
deqp-vk: lib/IR/Instructions.cpp:348: void
llvm::CallInst::init(llvm::FunctionType*, llvm::Value*,
llvm::ArrayRef,
llvm::ArrayRef >, const
llvm::Twine&): Assertion `(i >= FTy->getNumParams() ||
FTy->getParamType(i) == Args[i]->getType()) && "Calling a function
with a bad signature!"' failed.
because it's calling the export intrinsic with incorrect argument types.

For both tests, it seems to only assert with LLVM 8 for some reason.
On Thu, 6 Dec 2018 at 13:31, Samuel Pitoiset  wrote:
>
>
>
> On 12/6/18 2:15 PM, Rhys Perry wrote:
> > So that the signature is correct and consistent, the inputs to a export
> > intrinsic should always be 32-bit floats.
> >
> > This and the previous commit fixes a large amount crashes from
> > dEQP-VK.spirv_assembly.instruction.graphics.16bit_storage.input_output_int_*
> > tests
> >
>
> They don't crash for me? Please explain how to reproduce.
>
> > Fixes: b722b29f10d ('radv: add support for 16bit input/output')
> > Signed-off-by: Rhys Perry 
> > ---
> >   src/amd/vulkan/radv_nir_to_llvm.c | 6 +-
> >   1 file changed, 1 insertion(+), 5 deletions(-)
> >
> > diff --git a/src/amd/vulkan/radv_nir_to_llvm.c 
> > b/src/amd/vulkan/radv_nir_to_llvm.c
> > index 0c91118e5a..90bcc8dbfe 100644
> > --- a/src/amd/vulkan/radv_nir_to_llvm.c
> > +++ b/src/amd/vulkan/radv_nir_to_llvm.c
> > @@ -2464,12 +2464,8 @@ si_llvm_init_export_args(struct radv_shader_context 
> > *ctx,
> >   } else
> >   memcpy(>out[0], values, sizeof(values[0]) * 4);
> >
> > - for (unsigned i = 0; i < 4; ++i) {
> > - if (!(args->enabled_channels & (1 << i)))
> > - continue;
> > -
> > + for (unsigned i = 0; i < 4; ++i)
> >   args->out[i] = ac_to_float(>ac, args->out[i]);
> > - }
> >   }
> >
> >   static void
> >
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 2/2] radv: ensure export arguments are always float

2018-12-06 Thread Samuel Pitoiset



On 12/6/18 2:15 PM, Rhys Perry wrote:

So that the signature is correct and consistent, the inputs to a export
intrinsic should always be 32-bit floats.

This and the previous commit fixes a large amount crashes from
dEQP-VK.spirv_assembly.instruction.graphics.16bit_storage.input_output_int_*
tests



They don't crash for me? Please explain how to reproduce.


Fixes: b722b29f10d ('radv: add support for 16bit input/output')
Signed-off-by: Rhys Perry 
---
  src/amd/vulkan/radv_nir_to_llvm.c | 6 +-
  1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/src/amd/vulkan/radv_nir_to_llvm.c 
b/src/amd/vulkan/radv_nir_to_llvm.c
index 0c91118e5a..90bcc8dbfe 100644
--- a/src/amd/vulkan/radv_nir_to_llvm.c
+++ b/src/amd/vulkan/radv_nir_to_llvm.c
@@ -2464,12 +2464,8 @@ si_llvm_init_export_args(struct radv_shader_context *ctx,
} else
memcpy(>out[0], values, sizeof(values[0]) * 4);
  
-	for (unsigned i = 0; i < 4; ++i) {

-   if (!(args->enabled_channels & (1 << i)))
-   continue;
-
+   for (unsigned i = 0; i < 4; ++i)
args->out[i] = ac_to_float(>ac, args->out[i]);
-   }
  }
  
  static void



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


[Mesa-dev] [PATCH 2/2] radv: ensure export arguments are always float

2018-12-06 Thread Rhys Perry
So that the signature is correct and consistent, the inputs to a export
intrinsic should always be 32-bit floats.

This and the previous commit fixes a large amount crashes from
dEQP-VK.spirv_assembly.instruction.graphics.16bit_storage.input_output_int_*
tests

Fixes: b722b29f10d ('radv: add support for 16bit input/output')
Signed-off-by: Rhys Perry 
---
 src/amd/vulkan/radv_nir_to_llvm.c | 6 +-
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/src/amd/vulkan/radv_nir_to_llvm.c 
b/src/amd/vulkan/radv_nir_to_llvm.c
index 0c91118e5a..90bcc8dbfe 100644
--- a/src/amd/vulkan/radv_nir_to_llvm.c
+++ b/src/amd/vulkan/radv_nir_to_llvm.c
@@ -2464,12 +2464,8 @@ si_llvm_init_export_args(struct radv_shader_context *ctx,
} else
memcpy(>out[0], values, sizeof(values[0]) * 4);
 
-   for (unsigned i = 0; i < 4; ++i) {
-   if (!(args->enabled_channels & (1 << i)))
-   continue;
-
+   for (unsigned i = 0; i < 4; ++i)
args->out[i] = ac_to_float(>ac, args->out[i]);
-   }
 }
 
 static void
-- 
2.19.2

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