[Beignet] [PATCH] improve disassembing load/store IR instructions

[Xing, Homer]

Yes, LLVM style is also clear. Good.

-----Original Message-----
From: Zhigang Gong [mailto:zhigang.gong at linux.intel.com] 
Sent: Friday, June 07, 2013 10:24 AM
To: 'Dag Lem'
Cc: Xing, Homer; beignet at lists.freedesktop.org
Subject: RE: [Beignet] [PATCH] improve disassembing load/store IR instructions

Dag,

Thanks for your opinion. And per llvm assembly annotation, the load and store is as below, which really looks very clear.

  store i32 3, i32* %1                          ; yields {void}
  %2 = load i32* %1                           ; yields {i32}:val = i32 3

Homer, what's your opinion?

> -----Original Message-----
> From: 
> beignet-bounces+zhigang.gong=linux.intel.com at lists.freedesktop.org
>
[mailto:beignet-bounces+zhigang.gong=linux.intel.com at lists.freedesktop.org]
> On Behalf Of Dag Lem
> Sent: Friday, June 07, 2013 5:16 AM
> To: Zhigang Gong
> Cc: 'Xing, Homer'; beignet at lists.freedesktop.org
> Subject: Re: [Beignet] [PATCH] improve disassembing load/store IR
instructions
> 
> IMHO the natural choice for you would be the LLVM assembly notation.
> 
> This is also adopted by SPIR, if you'd like to adapt to that standard.
> 
> The relevant documents are:
> 
> http://www.khronos.org/registry/cl/specs/spir_spec-1.0-provisional.pdf
> http://llvm.org/docs/LangRef.html
> 
> Just my two cents :-)
> 
> --
> Best regards,
> 
> Dag Lem
> 
> 
> "Zhigang Gong" <zhigang.gong at linux.intel.com> writes:
> 
> > Any other supporters? :)
> >
> >> -----Original Message-----
> >> From:
> >> beignet-bounces+zhigang.gong=linux.intel.com at lists.freedesktop.org
> >>
> > [mailto:beignet-bounces+zhigang.gong=linux.intel.com at lists.freedeskt
> > op
> > .org]
> >> On Behalf Of Xing, Homer
> >> Sent: Thursday, June 06, 2013 4:21 PM
> >> To: Zhigang Gong; beignet at lists.freedesktop.org
> >> Subject: Re: [Beignet] [PATCH] improve disassembing load/store IR
> > instructions
> >>
> >> Some developers, who don't know "using {%x} is a value, without {} 
> >> is an address" may feel confused about what "load {%1} %2" mean.
> >>
> >> In fact one of my colleagues once asked what "{}" mean in the IR. I
> > personally
> >> think the new style is clearer.
> >>
> >> -----Original Message-----
> >> From: Zhigang Gong [mailto:zhigang.gong at linux.intel.com]
> >> Sent: Thursday, June 06, 2013 4:18 PM
> >> To: Xing, Homer; beignet at lists.freedesktop.org
> >> Subject: RE: [Beignet] [PATCH] improve disassembing load/store IR
> > instructions
> >>
> >> Homer,
> >>
> >> I personally don't think the previous version is not clear. First 
> >> operator
> > is
> >> destination and the second operator is source.
> >> And use {%x} to indicate it's a value, and without {} indicates 
> >> it's an
> > address
> >> pointer. It's clear enough for me.
> >>
> >> Any strong reason why we need to change to the new style?
> >>
> >> > -----Original Message-----
> >> > From:
> >> > beignet-bounces+zhigang.gong=linux.intel.com at lists.freedesktop.or
> >> > beignet-bounces+g
> >> >
> >>
> > [mailto:beignet-bounces+zhigang.gong=linux.intel.com at lists.freedeskt
> > op
> > .org]
> >> > On Behalf Of Homer Hsing
> >> > Sent: Thursday, June 06, 2013 2:29 PM
> >> > To: beignet at lists.freedesktop.org
> >> > Cc: Homer Hsing
> >> > Subject: [Beignet] [PATCH] improve disassembing load/store IR 
> >> > instructions
> >> >
> >> > comment data operator and address operator
> >> >
> >> > before changing, disassembly code is:
> >> >   load {%1} %2
> >> >   store %1 {%2}
> >> >
> >> > after changing, disassembly code is:
> >> >   load to(%1) addr(%2)
> >> >   store addr(%1) from(%2)
> >> >
> >> > Signed-off-by: Homer Hsing <homer.xing at intel.com>
> >> > ---
> >> >  backend/src/ir/instruction.cpp | 10 +++++-----
> >> >  1 file changed, 5 insertions(+), 5 deletions(-)
> >> >
> >> > diff --git a/backend/src/ir/instruction.cpp
> >> b/backend/src/ir/instruction.cpp
> >> > index a57c204..0492a1c 100644
> >> > --- a/backend/src/ir/instruction.cpp
> >> > +++ b/backend/src/ir/instruction.cpp
> >> > @@ -894,20 +894,20 @@ namespace ir {
> >> >      INLINE void LoadInstruction::out(std::ostream &out, const 
> >> > Function
> >> &fn)
> >> > const {
> >> >        this->outOpcode(out);
> >> >        out << "." << type << "." << addrSpace << (dwAligned ? "." :
> >> > ".un")
> >> <<
> >> > "aligned";
> >> > -      out << " {";
> >> > +      out << " to(";
> >> >        for (uint32_t i = 0; i < valueNum; ++i)
> >> >          out << "%" << this->getDst(fn, i) << (i != (valueNum-1u) ? "
"
> > :
> >> "");
> >> > -      out << "}";
> >> > -      out << " %" << this->getSrc(fn, 0);
> >> > +      out << ")";
> >> > +      out << " addr(%" << this->getSrc(fn, 0) << ")";
> >> >      }
> >> >
> >> >      INLINE void StoreInstruction::out(std::ostream &out, const 
> >> > Function
> >> &fn)
> >> > const {
> >> >        this->outOpcode(out);
> >> >        out << "." << type << "." << addrSpace << (dwAligned ? "." :
> >> > ".un")
> >> <<
> >> > "aligned";
> >> > -      out << " %" << this->getSrc(fn, 0) << " {";
> >> > +      out << " addr(%" << this->getSrc(fn, 0) << ") from(";
> >> >        for (uint32_t i = 0; i < valueNum; ++i)
> >> >          out << "%" << this->getSrc(fn, i+1) << (i != (valueNum-1u) ?
"
> > "
> >> : "");
> >> > -      out << "}";
> >> > +      out << ")";
> >> >      }
> >> >
> >> >      INLINE void LabelInstruction::out(std::ostream &out, const 
> >> > Function
> >> &fn)
> >> > const {
> >> > --
> >> > 1.8.1.2
> >> >
> >> > _______________________________________________
> >> > Beignet mailing list
> >> > Beignet at lists.freedesktop.org
> >> > http://lists.freedesktop.org/mailman/listinfo/beignet
> >>
> >> _______________________________________________
> >> Beignet mailing list
> >> Beignet at lists.freedesktop.org
> >> http://lists.freedesktop.org/mailman/listinfo/beignet
> >
> > _______________________________________________
> > Beignet mailing list
> > Beignet at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/beignet
> >
> _______________________________________________
> Beignet mailing list
> Beignet at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/beignet