On Don, 2013-02-21 at 10:38 +0100, Christian König wrote:
> Am 21.02.2013 09:39, schrieb Michel Dänzer:
> > On Mit, 2013-02-20 at 18:46 +0100, Christian König wrote:
> >> diff --git a/lib/Target/R600/SIInstructions.td
> >> b/lib/Target/R600/SIInstructions.td
> >> index 700b8f8..866c7cb 100644
> >> --- a/lib/Target/R600/SIInstructions.td
> >> +++ b/lib/Target/R600/SIInstructions.td
> >> @@ -620,7 +620,7 @@ def V_INTERP_P1_F32 : VINTRP <
> >> 0x00000000,
> >> (outs VReg_32:$dst),
> >> (ins VReg_32:$i, i32imm:$attr_chan, i32imm:$attr, M0Reg:$m0),
> >> - "V_INTERP_P1_F32",
> >> + "V_INTERP_P1_F32 $dst, $i, $attr_chan, $attr, $m0",
> > I didn't put $m0 in the V_INTERP_MOV_F32 asm string because it's not an
> > explicit operand but used implicitly. I can see the value in printing it
> > anyway, but it should probably be distinguished from the explicit
> > operands somehow, e.g. "[$m0]" or something like that.
>
> Any suggestion for the full asm string?
E.g.
"V_INTERP_P1_F32 $dst, $i, $attr_chan, $attr, [$m0]"
> >> @@ -722,16 +722,19 @@ def S_WAITCNT : SOPP <0x0000000c, (ins
> >> i32imm:$simm16), "S_WAITCNT $simm16",
> >> //def S_TTRACEDATA : SOPP_ <0x00000016, "S_TTRACEDATA", []>;
> >>
> >> def V_CNDMASK_B32_e32 : VOP2 <0x00000000, (outs VReg_32:$dst),
> >> - (ins VSrc_32:$src0, VReg_32:$src1, VCCReg:$vcc), "V_CNDMASK_B32_e32",
> >> + (ins VSrc_32:$src0, VReg_32:$src1, VCCReg:$vcc),
> >> + "V_CNDMASK_B32_e32 $dst, $src0, $src1, $vcc",
> >> []
> >> >{
> >> let DisableEncoding = "$vcc";
> > These asm strings should be made consistent with whichever way we decide
> > to handle implicit operands as well.
>
> I left out the implicit regs for the S_CBRANCH_* instruction because
> they are part of the instruction name anyway, but for everything else I
> don't see any reason why we shouldn't print them.
>
> For the example of V_CNDMASK_B32_e32 I would say that we should
> definitely print it, cause that bring us in line with the _e64 encoding
> which can have the condition in any SReg_64.
Right. Along similar lines, printing them for S_CBRANCH_* might help
catch bugs which somehow cause the register allocator to try and use a
different register.
> > Other than these nits, the series looks good to me. Be sure to run make
> > check, in case the lit tests need to be updated for the asm string
> > changes.
>
> I'm usually only running "llvm-lit test/CodeGen/R600/", cause I don't
> build anything else than the R600 target and so running all the other
> tests shouldn't make much sense.
That's probably fine, though make check takes less than a minute on my
quad core Llano.
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Debian, X and DRI developer
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
