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

[Wang, Rander]

Ok, I will refine my patch according to discussing

Thank you!

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



> -----Original Message-----
> From: Wang, Rander
> Sent: Tuesday, June 13, 2017 1:24 PM
> To: Song, Ruiling <ruiling.song at intel.com>; beignet at freedesktop.org
> Subject: RE: [Beignet] [PATCH V2] backend: add global immediate 
> optimization
> 
> 
> 
> -----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.
Yes, I understand you are checking against the "retype case". But I think even retype occurs, your optimization still works.
For example:

mov.s32 %1, 0xffff0000
and.u32   %3, %2, %1

Your optimization should still work for such case.
> 
> > +    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
Then how about noMask? Do you need to check noMask == 1?
Both predication and pcip-mask could introduce partial write to a register.
> 
> Thanks!
> Ruiling