In fact, even if user set 'FP_CONTRACT ON', we can also choose not do 'mad 
optimization.'
It depends on us whether or not do the contraction. "Contract on" is just a 
hint.
Obviously contract on will get better speed and accuracy.

I think the compiler_menger_sponge_no_shadow fail because of accuracy. Its 
reference is got with mad instruction.
I think we can use the new result to replace the reference, there should be not 
too much difference. If they differ too much, then it would be a bug.


-----Original Message-----
From: Zhigang Gong [mailto:zhigang.g...@linux.intel.com] 
Sent: Monday, November 04, 2013 3:41 PM
To: Song, Ruiling
Cc: beignet@lists.freedesktop.org
Subject: Re: [Beignet] [PATCH] GBE: disable MulAdd pattern in instruction 
selection temporarily.

This patch breaks utests compiler_menger_sponge_no_shadow.
Actually, my first feeling of this patch is that it assumes that FP_CONTRACT is 
always off and may be cause problem if this assumption is incorrect. The unit 
test case compiler_menger_sponge_no_shadow should be one of that case.

Any thoughts?

On Fri, Nov 01, 2013 at 02:16:08PM +0800, Ruiling Song wrote:
> The story starts from 'FP_CONTRACT'. In c99 spec, it describes 
> constract expression as:
> "A floating expression may be contracted, that is, evaluated as though 
> it were an atomic operation, thereby omitting rounding errors implied 
> by the source code and the expression evaluation method."
> 
> But user can use 'pragma FP_CONTRACT OFF' to disable float 
> contraction, in which condition, we should not do contraction like mad 
> optimization.
> In SPIR 1.2, named metadata 'opencl.enable.FP_CONTRACT' will be used to do 
> this.
> When Clang is ready, we need refine the backend logic.
> 
> Signed-off-by: Ruiling Song <ruiling.s...@intel.com>
> ---
>  backend/src/backend/gen_insn_selection.cpp |    3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/backend/src/backend/gen_insn_selection.cpp 
> b/backend/src/backend/gen_insn_selection.cpp
> index 7eae7ca..1f25f0e 100644
> --- a/backend/src/backend/gen_insn_selection.cpp
> +++ b/backend/src/backend/gen_insn_selection.cpp
> @@ -1856,6 +1856,9 @@ namespace gbe
>      {
>        using namespace ir;
>  
> +      // XXX TODO: we need a clean support of FP_CONTRACT to remove below 
> line 'return false'
> +      // if 'pragma FP_CONTRACT OFF' is used in cl kernel, we should not do 
> mad optimization.
> +      return false;
>        // MAD tend to increase liveness of the sources (since there are three 
> of
>        // them). TODO refine this strategy. Well, we should be able at least 
> to
>        // evaluate per basic block register pressure and selectively 
> enable
> --
> 1.7.9.5
> 
> _______________________________________________
> Beignet mailing list
> Beignet@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/beignet
_______________________________________________
Beignet mailing list
Beignet@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/beignet

Reply via email to