[Beignet] [PATCH] GBE: fix a bool handling bug when SEL on a uniform bool variable.

[Song, Ruiling]

LGTM

> -----Original Message-----
> From: Beignet [mailto:beignet-bounces at lists.freedesktop.org] On Behalf Of
> Zhigang Gong
> Sent: Wednesday, November 05, 2014 4:04 PM
> To: beignet at lists.freedesktop.org
> Cc: Gong, Zhigang
> Subject: [Beignet] [PATCH] GBE: fix a bool handling bug when SEL on a
> uniform bool variable.
> 
> If a SEL uses a bool variable which is a uniform bool, even we can get a dag
> node within the same BB, we still need to set the externFlag bit. The reason
> is that we don't know how to generate a scalar physical flag.
> 
> Signed-off-by: Zhigang Gong <zhigang.gong at intel.com>
> ---
>  backend/src/backend/gen_insn_selection.cpp | 11 ++++++++++-
> backend/src/backend/gen_reg_allocation.cpp |  2 ++
>  2 files changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/backend/src/backend/gen_insn_selection.cpp
> b/backend/src/backend/gen_insn_selection.cpp
> index 64e9fd8..bd19702 100644
> --- a/backend/src/backend/gen_insn_selection.cpp
> +++ b/backend/src/backend/gen_insn_selection.cpp
> @@ -3804,7 +3804,16 @@ namespace gbe
>          sel.curr.physicalFlag = 0;
>          sel.curr.flagIndex = (uint16_t) pred;
>          sel.curr.predicate = GEN_PREDICATE_NORMAL;
> -        if (!dag0)
> +        // FIXME in general, if the flag is a uniform flag.
> +        // we should treat that flag as extern flag, as we
> +        // never genrate a uniform physical flag. As we can
> +        // never predicate which channel is active when this
> +        // flag is used.
> +        // We need to concentrate this logic to the modFlag bit.
> +        // If an instruction has that bit, it will generate physical
> +        // flag, otherwise it will not. But current modFlag is
> +        // just a hint. We need to fix it in the future.
> +        if (!dag0 || (sel.isScalarReg(dag0->insn.getDst(0))))
>            sel.curr.externFlag = 1;
>          if(type == ir::TYPE_S64 || type == ir::TYPE_U64)
>            sel.SEL_INT64(dst, src0, src1); diff --git
> a/backend/src/backend/gen_reg_allocation.cpp
> b/backend/src/backend/gen_reg_allocation.cpp
> index ef519d9..18f60ca 100644
> --- a/backend/src/backend/gen_reg_allocation.cpp
> +++ b/backend/src/backend/gen_reg_allocation.cpp
> @@ -585,6 +585,8 @@ namespace gbe
>                // If this is a modFlag on a scalar bool, we need to remove
> it
>                // from the allocated flags map. Then latter, the user
> could
>                // validate the flag from the scalar value correctly.
> +              // The reason is we can not predicate the active channel
> when we
> +              // need to use this flag.
>                if (IS_SCALAR_FLAG(insn)) {
>                  allocatedFlags.erase(ir::Register(insn.state.flagIndex));
>                  continue;
> --
> 1.8.3.2
> 
> _______________________________________________
> Beignet mailing list
> Beignet at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/beignet