[Beignet] [PATCH V2] backend: add global immediate optimization

[Wang, Rander]

-----Original Message-----
From: Song, Ruiling 
Sent: Tuesday, June 13, 2017 10:24 AM
To: Wang, Rander <rander.wang at intel.com>; beignet at freedesktop.org
Cc: Wang, Rander <rander.wang at intel.com>
Subject: RE: [Beignet] [PATCH V2] backend: add global immediate optimization

> +          else if (src0.type == GEN_TYPE_D || src0.type == GEN_TYPE_UD)
> +          {
> +            int s0 = src0.value.d;
> +            if (src0.absolute)
> +              s0 = fabs(s0);
Here I think it should be abs(s0), right? ===== Yes, typo

> +            if (src0.negation)
> +              s0 = -s0;
> +
> +            int s1 = src1.value.d;
> +            if (src1.absolute)
> +              s1 = fabs(s1);
Same as above. ====== Yes, typo

> +
> +    if(info->intermedia.type != var.type)
> +    {
> +        bool isOk = false;
> +        assert(info->replacement.file == GEN_IMMEDIATE_VALUE);
> +        if(var.type == GEN_TYPE_D && info->intermedia.type == GEN_TYPE_UD)
> +        {
> +          if(info->replacement.value.ud >= 0x80000000)
> +            return false;
> +          isOk = true;
> +        }
> +
> +        if(var.type == GEN_TYPE_UD && info->intermedia.type == GEN_TYPE_D)
> +        {
> +          if(info->replacement.value.d < 0)
> +            return false;
> +          isOk = true;
> +        }
Could you explain a little bit about the above checks for negative number?
Did you have concern over "var with negative modifier"? or any other reason?

=====  There are some global imm are UD or D. but it is retyped to D or UD when used later.

> +    SelectionBlock &block = *mblockList->begin();
> +    for(SelectionInstruction &insn : block.insnList)
> +    {
> +        GenRegister src0 = insn.src(0);
> +        if(insn.opcode == SEL_OP_MOV &&
> +            src0.file == GEN_IMMEDIATE_VALUE &&
> +            (src0.type == GEN_TYPE_UD || src0.type == GEN_TYPE_UD || src0.type

Typo? "(src0.type == GEN_TYPE_UD || src0.type == GEN_TYPE_UD"  =========== Yes, typo

> == GEN_TYPE_F) &&
> +              insn.state.predicate == GEN_PREDICATE_NONE)
What's your point here to check GEN_PREDICATE_NONE?
====== it is a reserved optimization. It is easy to make mistake with predicate

Thanks!
Ruiling