Re: [Mesa-dev] [PATCH] R600/SI: Use MULADD_IEEE/V_MAD_F32 instruction for mad pattern
On Mon, 2013-02-11 at 16:12 +0100, Tom Stellard wrote: > On Mon, Feb 11, 2013 at 03:35:44PM +0100, Michel Dänzer wrote: > > On Son, 2013-02-10 at 19:38 +0100, Vincent Lejeune wrote: > > > > > > diff --git a/lib/Target/R600/SIInstructions.td > > > b/lib/Target/R600/SIInstructions.td > > > index a09f243..7e50e86 100644 > > > --- a/lib/Target/R600/SIInstructions.td > > > +++ b/lib/Target/R600/SIInstructions.td > > > @@ -1423,8 +1423,8 @@ def : Pat < > > > /** VOP3 Patterns**/ > > > /** == **/ > > > > > > -def : Pat <(f32 (IL_mad AllReg_32:$src0, VReg_32:$src1, VReg_32:$src2)), > > > - (V_MAD_LEGACY_F32 AllReg_32:$src0, VReg_32:$src1, > > > VReg_32:$src2, > > > +def : Pat <(f32 (fadd (fmul AllReg_32:$src0, VReg_32:$src1), > > > VReg_32:$src2)), > > > + (V_MAD_F32 AllReg_32:$src0, VReg_32:$src1, VReg_32:$src2, > > > 0, 0, 0, 0)>; > > > > I still feel a bit uneasy about applying the SI changes without fully > > understanding why they break those piglit tests. Though I guess it might > > be okay if you're not targetting this for the Mesa stable branch... > > Tom / Christian, what's your opinion on this? > > > > I think this should go to the stable branch, because V_MAD_LEGACY is the wrong > instruction to use in the case, [...] It might not be wrong for IL_mad. -- 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
Re: [Mesa-dev] [PATCH] R600/SI: Use MULADD_IEEE/V_MAD_F32 instruction for mad pattern
On Mon, Feb 11, 2013 at 03:35:44PM +0100, Michel Dänzer wrote: > On Son, 2013-02-10 at 19:38 +0100, Vincent Lejeune wrote: > > > > diff --git a/lib/Target/R600/SIInstructions.td > > b/lib/Target/R600/SIInstructions.td > > index a09f243..7e50e86 100644 > > --- a/lib/Target/R600/SIInstructions.td > > +++ b/lib/Target/R600/SIInstructions.td > > @@ -1423,8 +1423,8 @@ def : Pat < > > /** VOP3 Patterns**/ > > /** == **/ > > > > -def : Pat <(f32 (IL_mad AllReg_32:$src0, VReg_32:$src1, VReg_32:$src2)), > > - (V_MAD_LEGACY_F32 AllReg_32:$src0, VReg_32:$src1, VReg_32:$src2, > > +def : Pat <(f32 (fadd (fmul AllReg_32:$src0, VReg_32:$src1), > > VReg_32:$src2)), > > + (V_MAD_F32 AllReg_32:$src0, VReg_32:$src1, VReg_32:$src2, > > 0, 0, 0, 0)>; > > I still feel a bit uneasy about applying the SI changes without fully > understanding why they break those piglit tests. Though I guess it might > be okay if you're not targetting this for the Mesa stable branch... > Tom / Christian, what's your opinion on this? > I think this should go to the stable branch, because V_MAD_LEGACY is the wrong instruction to use in the case, but let's hold off on committing this until we see if Christian's patches will fix the problems this patch uncovers. -Tom > > -- > 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 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] R600/SI: Use MULADD_IEEE/V_MAD_F32 instruction for mad pattern
On Mon, Feb 11, 2013 at 03:47:47PM +0100, Christian König wrote: > Am 11.02.2013 15:35, schrieb Michel Dänzer: > >On Son, 2013-02-10 at 19:38 +0100, Vincent Lejeune wrote: > >>diff --git a/lib/Target/R600/SIInstructions.td > >>b/lib/Target/R600/SIInstructions.td > >>index a09f243..7e50e86 100644 > >>--- a/lib/Target/R600/SIInstructions.td > >>+++ b/lib/Target/R600/SIInstructions.td > >>@@ -1423,8 +1423,8 @@ def : Pat < > >> /** VOP3 Patterns**/ > >> /** == **/ > >>-def : Pat <(f32 (IL_mad AllReg_32:$src0, VReg_32:$src1, VReg_32:$src2)), > >>- (V_MAD_LEGACY_F32 AllReg_32:$src0, VReg_32:$src1, VReg_32:$src2, > >>+def : Pat <(f32 (fadd (fmul AllReg_32:$src0, VReg_32:$src1), > >>VReg_32:$src2)), > >>+ (V_MAD_F32 AllReg_32:$src0, VReg_32:$src1, VReg_32:$src2, > >> 0, 0, 0, 0)>; > >I still feel a bit uneasy about applying the SI changes without fully > >understanding why they break those piglit tests. Though I guess it might > >be okay if you're not targetting this for the Mesa stable branch... > >Tom / Christian, what's your opinion on this? > > I think I'm on the way figuring out what's going wrong here. One > step is that the _e64 encoding of (for example) V_CND_* is slightly > wrong and working only by coincident, but there seems to be at least > one more thing quite wrong here. > Several of the failing shaders use indirect addressing, which on radeonsi is lowered to a lot of select statement since the driver does not support it yet. It's likely that this is the real problem. Also, do you know about the -show-mc-encoding flag for llc, it displays the encoding along side the assembly and is useful for debugging encoding issues. - Tom > Give me a day or two I've coded a whole bunch of patches while on > vacation (including a fix for this), but I doesn't have the hardware > here to test them. > > Christian. > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] R600/SI: Use MULADD_IEEE/V_MAD_F32 instruction for mad pattern
Am 11.02.2013 15:35, schrieb Michel Dänzer: On Son, 2013-02-10 at 19:38 +0100, Vincent Lejeune wrote: diff --git a/lib/Target/R600/SIInstructions.td b/lib/Target/R600/SIInstructions.td index a09f243..7e50e86 100644 --- a/lib/Target/R600/SIInstructions.td +++ b/lib/Target/R600/SIInstructions.td @@ -1423,8 +1423,8 @@ def : Pat < /** VOP3 Patterns**/ /** == **/ -def : Pat <(f32 (IL_mad AllReg_32:$src0, VReg_32:$src1, VReg_32:$src2)), - (V_MAD_LEGACY_F32 AllReg_32:$src0, VReg_32:$src1, VReg_32:$src2, +def : Pat <(f32 (fadd (fmul AllReg_32:$src0, VReg_32:$src1), VReg_32:$src2)), + (V_MAD_F32 AllReg_32:$src0, VReg_32:$src1, VReg_32:$src2, 0, 0, 0, 0)>; I still feel a bit uneasy about applying the SI changes without fully understanding why they break those piglit tests. Though I guess it might be okay if you're not targetting this for the Mesa stable branch... Tom / Christian, what's your opinion on this? I think I'm on the way figuring out what's going wrong here. One step is that the _e64 encoding of (for example) V_CND_* is slightly wrong and working only by coincident, but there seems to be at least one more thing quite wrong here. Give me a day or two I've coded a whole bunch of patches while on vacation (including a fix for this), but I doesn't have the hardware here to test them. Christian. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] R600/SI: Use MULADD_IEEE/V_MAD_F32 instruction for mad pattern
On Son, 2013-02-10 at 19:38 +0100, Vincent Lejeune wrote: > > diff --git a/lib/Target/R600/SIInstructions.td > b/lib/Target/R600/SIInstructions.td > index a09f243..7e50e86 100644 > --- a/lib/Target/R600/SIInstructions.td > +++ b/lib/Target/R600/SIInstructions.td > @@ -1423,8 +1423,8 @@ def : Pat < > /** VOP3 Patterns**/ > /** == **/ > > -def : Pat <(f32 (IL_mad AllReg_32:$src0, VReg_32:$src1, VReg_32:$src2)), > - (V_MAD_LEGACY_F32 AllReg_32:$src0, VReg_32:$src1, VReg_32:$src2, > +def : Pat <(f32 (fadd (fmul AllReg_32:$src0, VReg_32:$src1), VReg_32:$src2)), > + (V_MAD_F32 AllReg_32:$src0, VReg_32:$src1, VReg_32:$src2, > 0, 0, 0, 0)>; I still feel a bit uneasy about applying the SI changes without fully understanding why they break those piglit tests. Though I guess it might be okay if you're not targetting this for the Mesa stable branch... Tom / Christian, what's your opinion on this? -- 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
[Mesa-dev] [PATCH] R600/SI: Use MULADD_IEEE/V_MAD_F32 instruction for mad pattern
---
lib/Target/R600/AMDGPUISelLowering.cpp | 10 +++---
lib/Target/R600/AMDGPUISelLowering.h | 1 -
lib/Target/R600/AMDILISelLowering.cpp | 3 ++-
lib/Target/R600/AMDILInstrInfo.td | 1 -
lib/Target/R600/AMDILIntrinsics.td | 10 --
lib/Target/R600/R600Instructions.td| 9 -
lib/Target/R600/SIInstructions.td | 4 ++--
test/CodeGen/R600/fmad.ll | 19 +++
8 files changed, 34 insertions(+), 23 deletions(-)
create mode 100644 test/CodeGen/R600/fmad.ll
diff --git a/lib/Target/R600/AMDGPUISelLowering.cpp
b/lib/Target/R600/AMDGPUISelLowering.cpp
index d0d23d6..0a33264 100644
--- a/lib/Target/R600/AMDGPUISelLowering.cpp
+++ b/lib/Target/R600/AMDGPUISelLowering.cpp
@@ -127,9 +127,6 @@ SDValue
AMDGPUTargetLowering::LowerINTRINSIC_WO_CHAIN(SDValue Op,
return LowerIntrinsicLRP(Op, DAG);
case AMDGPUIntrinsic::AMDIL_fraction:
return DAG.getNode(AMDGPUISD::FRACT, DL, VT, Op.getOperand(1));
-case AMDGPUIntrinsic::AMDIL_mad:
- return DAG.getNode(AMDGPUISD::MAD, DL, VT, Op.getOperand(1),
- Op.getOperand(2), Op.getOperand(3));
case AMDGPUIntrinsic::AMDIL_max:
return DAG.getNode(AMDGPUISD::FMAX, DL, VT, Op.getOperand(1),
Op.getOperand(2));
@@ -176,9 +173,9 @@ SDValue AMDGPUTargetLowering::LowerIntrinsicLRP(SDValue Op,
Op.getOperand(1));
SDValue OneSubAC = DAG.getNode(ISD::FMUL, DL, VT, OneSubA,
Op.getOperand(3));
- return DAG.getNode(AMDGPUISD::MAD, DL, VT, Op.getOperand(1),
- Op.getOperand(2),
- OneSubAC);
+ return DAG.getNode(ISD::FADD, DL, VT,
+ DAG.getNode(ISD::FMUL, DL, VT, Op.getOperand(1), Op.getOperand(2)),
+ OneSubAC);
}
/// \brief Generate Min/Max node
@@ -393,7 +390,6 @@ const char*
AMDGPUTargetLowering::getTargetNodeName(unsigned Opcode) const {
switch (Opcode) {
default: return 0;
// AMDIL DAG nodes
- NODE_NAME_CASE(MAD);
NODE_NAME_CASE(CALL);
NODE_NAME_CASE(UMUL);
NODE_NAME_CASE(DIV_INF);
diff --git a/lib/Target/R600/AMDGPUISelLowering.h
b/lib/Target/R600/AMDGPUISelLowering.h
index 4b844a3..f27b5db 100644
--- a/lib/Target/R600/AMDGPUISelLowering.h
+++ b/lib/Target/R600/AMDGPUISelLowering.h
@@ -108,7 +108,6 @@ namespace AMDGPUISD {
enum {
// AMDIL ISD Opcodes
FIRST_NUMBER = ISD::BUILTIN_OP_END,
- MAD, // 32bit Fused Multiply Add instruction
CALL,// Function call based on a single integer
UMUL,// 32bit unsigned multiplication
DIV_INF, // Divide with infinity returned on zero divisor
diff --git a/lib/Target/R600/AMDILISelLowering.cpp
b/lib/Target/R600/AMDILISelLowering.cpp
index 2e60adc..3480ac8 100644
--- a/lib/Target/R600/AMDILISelLowering.cpp
+++ b/lib/Target/R600/AMDILISelLowering.cpp
@@ -451,7 +451,8 @@ AMDGPUTargetLowering::LowerSDIV24(SDValue Op, SelectionDAG
&DAG) const {
SDValue fqneg = DAG.getNode(ISD::FNEG, DL, FLTTY, fq);
// float fr = mad(fqneg, fb, fa);
- SDValue fr = DAG.getNode(AMDGPUISD::MAD, DL, FLTTY, fqneg, fb, fa);
+ SDValue fr = DAG.getNode(ISD::FADD, DL, FLTTY,
+ DAG.getNode(ISD::MUL, DL, FLTTY, fqneg, fb), fa);
// int iq = (int)fq;
SDValue iq = DAG.getNode(ISD::FP_TO_SINT, DL, INTTY, fq);
diff --git a/lib/Target/R600/AMDILInstrInfo.td
b/lib/Target/R600/AMDILInstrInfo.td
index e969bbf..110f147 100644
--- a/lib/Target/R600/AMDILInstrInfo.td
+++ b/lib/Target/R600/AMDILInstrInfo.td
@@ -116,7 +116,6 @@ def IL_retflag : SDNode<"AMDGPUISD::RET_FLAG",
SDTNone,
//======//
// Floating point math functions
def IL_div_inf : SDNode<"AMDGPUISD::DIV_INF", SDTIL_GenBinaryOp>;
-def IL_mad : SDNode<"AMDGPUISD::MAD", SDTIL_GenTernaryOp>;
//===--===//
// Integer functions
diff --git a/lib/Target/R600/AMDILIntrinsics.td
b/lib/Target/R600/AMDILIntrinsics.td
index 3f9e20f..6ec3559 100644
--- a/lib/Target/R600/AMDILIntrinsics.td
+++ b/lib/Target/R600/AMDILIntrinsics.td
@@ -92,12 +92,6 @@ let TargetPrefix = "AMDIL", isTarget = 1 in {
TernaryIntInt;
def int_AMDIL_bfm : GCCBuiltin<"__amdil_bfm">,
BinaryIntInt;
- def int_AMDIL_mad_i32 : GCCBuiltin<"__amdil_imad">,
- TernaryIntInt;
- def int_AMDIL_mad_u32 : GCCBuiltin<"__amdil_umad">,
- TernaryIntInt;
- def int_AMDIL_mad : GCCBuiltin<"__amdil_mad">,
- TernaryIntFloat;
def int_AMDIL_mulhi_i32 : GCCBuiltin<"__amdil_imul_high">,
BinaryIntInt;
def int_AMDIL_mulhi_u32 : GCCBuiltin<"__amdil_umul_high">,
@@ -110,10 +104,6 @@ let TargetPrefix = "AMDIL", isTarget = 1 in {
BinaryIntInt;
def int_AMDIL_mulhi24_u32 : GCCBuiltin<"__amdil_umul24_high">,
