<html><body><div style="color:#000; background-color:#fff; font-family:HelveticaNeue, Helvetica Neue, Helvetica, Arial, Lucida Grande, sans-serif;font-size:8pt"><div style="" class=""><span style="" class="">Hi,</span></div><div class="" style="color: rgb(0, 0, 0); font-size: 10.6667px; font-family: HelveticaNeue,Helvetica Neue,Helvetica,Arial,Lucida Grande,sans-serif; background-color: transparent; font-style: normal"><br style="" class=""><span style="" class=""></span></div><div class="" style="color: rgb(0, 0, 0); font-size: 10.6667px; font-family: HelveticaNeue,Helvetica Neue,Helvetica,Arial,Lucida Grande,sans-serif; background-color: transparent; font-style: normal"><span style="" class="">sorry for the delay, I have updated the patches.</span></div><div class="" style="color: rgb(0, 0, 0); font-size: 10.6667px; font-family: HelveticaNeue,Helvetica Neue,Helvetica,Arial,Lucida Grande,sans-serif; background-color: transparent; font-style: normal"><br
style="" class="">> Did piglit run all tests up to and including OpenGL 3.3 / GLSL 3.30?</div><div class="" style="color: rgb(0, 0, 0); font-size: 10.6667px; font-family: HelveticaNeue,Helvetica Neue,Helvetica,Arial,Lucida Grande,sans-serif; background-color: transparent; font-style: normal"><br style="" class=""></div><div class="" style="color: rgb(0, 0, 0); font-size: 10.6667px; font-family: HelveticaNeue,Helvetica Neue,Helvetica,Arial,Lucida Grande,sans-serif; background-color: transparent; font-style: normal">Yes, there is no issue with GS.</div><div class="" style="color: rgb(0, 0, 0); font-size: 10.6667px; font-family: HelveticaNeue,Helvetica Neue,Helvetica,Arial,Lucida Grande,sans-serif; background-color: transparent; font-style: normal"><br style="" class=""></div><div class="" style="color: rgb(0, 0, 0); font-size: 10.6667px; font-family: HelveticaNeue,Helvetica Neue,Helvetica,Arial,Lucida Grande,sans-serif; background-color: transparent;
font-style: normal">> Would it be possible to completely remove the printing of the modifier<br style="" class="" clear="none">> arguments separately?</div><div class="" style="color: rgb(0, 0, 0); font-size: 10.6667px; font-family: HelveticaNeue,Helvetica Neue,Helvetica,Arial,Lucida Grande,sans-serif; background-color: transparent; font-style: normal"><br></div><div class="" style="color: rgb(0, 0, 0); font-size: 10.6667px; font-family: HelveticaNeue,Helvetica Neue,Helvetica,Arial,Lucida Grande,sans-serif; background-color: transparent; font-style: normal">I don't understand what you mean.</div><div class="" style="color: rgb(0, 0, 0); font-size: 10.6667px; font-family: HelveticaNeue,Helvetica Neue,Helvetica,Arial,Lucida Grande,sans-serif; background-color: transparent; font-style: normal"><br></div><div class="" style="color: rgb(0, 0, 0); font-size: 10.6667px; font-family: HelveticaNeue,Helvetica Neue,Helvetica,Arial,Lucida Grande,sans-serif;
background-color: transparent; font-style: normal">Vincent<br style="" class="" clear="none"></div><div style="display: block;" class="yahoo_quoted"> <div class="" style="font-family: HelveticaNeue, Helvetica Neue, Helvetica, Arial, Lucida Grande, sans-serif; font-size: 8pt"> <div class="" style="font-family: HelveticaNeue, Helvetica Neue, Helvetica, Arial, Lucida Grande, sans-serif; font-size: 12pt"> <div style="" class="" dir="ltr"> <font style="" class="" face="Arial" size="2"> Le Mercredi 12 février 2014 4h36, "Daenzer, Michel" <Michel.Daenzer@amd.com> a écrit :<br style="" class=""> </font> </div> <blockquote class="" style=""> <div style="" class="">On Mon, 2014-02-10 at 07:21 -0800, Vincent Lejeune wrote:<br style="" class="" clear="none">> <br style="" class="" clear="none">> I reworked an old patch that improved the dump of SI input modifiers.<br style="" class="" clear="none">> Previous patch was adding 6 input modifiers
which made instruction<br style="" class="" clear="none">> manipulation quite difficult.<br style="" class="" clear="none">> <br style="" class="" clear="none">> <br style="" class="" clear="none">> This new patch is only adding three (one modifier per inputs).<br style="" class="" clear="none"><br style="" class="" clear="none">Nice, I have some minor comments on this patch below though.<br style="" class="" clear="none"><br style="" class="" clear="none"><br style="" class="" clear="none">> Besides, I also worked on fabs folding (which may solve an issue<br style="" class="" clear="none">> raised lately because of fneg 0.0 issue).<br style="" class="" clear="none"><br style="" class="" clear="none">What issue is that?<br style="" class="" clear="none"><br style="" class="" clear="none"><br style="" class="" clear="none">> Both were tested on hd 7750 with a week old piglit and there was no<br style="" class="" clear="none">>
regression.<br style="" class="" clear="none"><br style="" class="" clear="none">Did piglit run all tests up to and including OpenGL 3.3 / GLSL 3.30?<br style="" class="" clear="none"><br style="" class="" clear="none"><br style="" class="" clear="none">> From: Vincent Lejeune <<a style="" class="" shape="rect" ymailto="mailto:vljn@ovi.com" href="mailto:vljn@ovi.com">vljn@ovi.com</a>><br style="" class="" clear="none">> Date: Wed, 5 Feb 2014 17:14:50 +0100<br style="" class="" clear="none">> Subject: [PATCH 2/2] R600/SI: Fold fabs into src input modifier<br style="" class="" clear="none">[...]<br style="" class="" clear="none">> diff --git a/test/CodeGen/R600/fabs.ll b/test/CodeGen/R600/fabs.ll<br style="" class="" clear="none">> index 2cd3a4f..15b7e22 100644<br style="" class="" clear="none">> --- a/test/CodeGen/R600/fabs.ll<br style="" class="" clear="none">> +++ b/test/CodeGen/R600/fabs.ll<br style="" class=""
clear="none">> @@ -49,6 +49,16 @@ entry:<br style="" class="" clear="none">> ret void<br style="" class="" clear="none">> }<br style="" class="" clear="none">> <br style="" class="" clear="none">> +; SI-CHECK-LABEL: @fabs_fold<br style="" class="" clear="none">> +; SI-CHECK-NOT: V_AND_B32_e32<br style="" class="" clear="none"><br style="" class="" clear="none">This should explicitly check that V_MUL_F32_e64 is used with the correct<br style="" class="" clear="none">modifier value.<br style="" class="" clear="none"><br style="" class="" clear="none">> +define void @fabs_fold(float addrspace(1)* %out, float %in0, float %<br style="" class="" clear="none">> in1) {<br style="" class="" clear="none">> +entry:<br style="" class="" clear="none">> + %0 = call float @fabs(float %in0)<br style="" class="" clear="none">> + %1 = fmul float %0, %in1<br style="" class="" clear="none">> +
store float %1, float addrspace(1)* %out<br style="" class="" clear="none">> + ret void<br style="" class="" clear="none">> +}<br style="" class="" clear="none">> +<br style="" class="" clear="none"><br style="" class="" clear="none">Other than that, this patch looks good to me.<br style="" class="" clear="none"><br style="" class="" clear="none"><br style="" class="" clear="none">> From: Vincent Lejeune <<a style="" class="" shape="rect" ymailto="mailto:vljn@ovi.com" href="mailto:vljn@ovi.com">vljn@ovi.com</a>><br style="" class="" clear="none">> Date: Sun, 2 Feb 2014 23:26:49 +0100<br style="" class="" clear="none">> Subject: [PATCH 1/2] R600/SI: Prettier display of input modifiers<br style="" class="" clear="none">[...]<br style="" class="" clear="none">> diff --git a/lib/Target/R600/InstPrinter/AMDGPUInstPrinter.cpp<br style="" class="" clear="none">> b/lib/Target/R600/InstPrinter/AMDGPUInstPrinter.cpp<br style=""
class="" clear="none">> index 7105879..e1da2ad 100644<br style="" class="" clear="none">> --- a/lib/Target/R600/InstPrinter/AMDGPUInstPrinter.cpp<br style="" class="" clear="none">> +++ b/lib/Target/R600/InstPrinter/AMDGPUInstPrinter.cpp<br style="" class="" clear="none">> @@ -106,6 +106,18 @@ void AMDGPUInstPrinter::printOperand(const MCInst<br style="" class="" clear="none">> *MI, unsigned OpNo,<br style="" class="" clear="none">> }<br style="" class="" clear="none">> }<br style="" class="" clear="none">> <br style="" class="" clear="none">> +void AMDGPUInstPrinter::printOperandAndMods(const MCInst *MI,<br style="" class="" clear="none">> unsigned OpNo,<br style="" class="" clear="none">> + raw_ostream &O) {<br style="" class=""
clear="none">> + unsigned InputModifiers = MI->getOperand(OpNo).getImm();<br style="" class="" clear="none">> + if (InputModifiers & 0x1)<br style="" class="" clear="none">> + O << "- ";<br style="" class="" clear="none"><br style="" class="" clear="none">Why a space after the minus sign?<br style="" class="" clear="none"><br style="" class="" clear="none"><br style="" class="" clear="none">> diff --git a/lib/Target/R600/SIInstructions.td<br style="" class="" clear="none">> b/lib/Target/R600/SIInstructions.td<br style="" class="" clear="none">> index 25fd7d5..fb49e9a 100644<br style="" class="" clear="none">> --- a/lib/Target/R600/SIInstructions.td<br style="" class="" clear="none">> +++ b/lib/Target/R600/SIInstructions.td<br style="" class="" clear="none">[...]<br style="" class="" clear="none">> @@ -1684,8 +1688,7 @@ def : BitConvert <v16f32, v16i32, VReg_512>;<br style="" class=""
clear="none">> <br style="" class="" clear="none">> def : Pat <<br style="" class="" clear="none">> (int_AMDIL_clamp f32:$src, (f32 FP_ZERO), (f32 FP_ONE)),<br style="" class="" clear="none">> - (V_ADD_F32_e64 $src, (i32 0 /* SRC1 */),<br style="" class="" clear="none">> - 0 /* ABS */, 1 /* CLAMP */, 0 /* OMOD */, 0 /* NEG */)<br style="" class="" clear="none">> + (V_ADD_F32_e64 $src, (i32 0 /* SRC1 */), 1 /* CLAMP */, 0 /* OMOD<br style="" class="" clear="none">> */)<br style="" class="" clear="none">> >;<br style="" class="" clear="none"><br style="" class="" clear="none">The /* OMOD */ comment needs to be adapted or simply removed.<br style="" class="" clear="none"><br style="" class="" clear="none"><br style="" class="" clear="none">> diff --git a/test/CodeGen/R600/fneg.ll b/test/CodeGen/R600/fneg.ll<br style="" class="" clear="none">> index f4e6be6..7ad760c
100644<br style="" class="" clear="none">> --- a/test/CodeGen/R600/fneg.ll<br style="" class="" clear="none">> +++ b/test/CodeGen/R600/fneg.ll<br style="" class="" clear="none">> @@ -51,7 +51,7 @@ entry:<br style="" class="" clear="none">> ; R600-CHECK: -KC0[2].Z<br style="" class="" clear="none">> ; SI-CHECK-LABEL: @fneg_free<br style="" class="" clear="none">> ; XXX: We could use V_ADD_F32_e64 with the negate bit here instead.<br style="" class="" clear="none">> -; SI-CHECK: V_SUB_F32_e64 v{{[0-9]}}, 0.000000e+00, s{{[0-9]}}, 0, 0,<div style="" class="" id="yqtfd50978"><br style="" class="" clear="none">> 0, 0</div><br style="" class="" clear="none">> +; SI-CHECK: V_SUB_F32_e64 v{{[0-9]}}, 0.000000e+00, s{{[0-9]}}, 0, 0<br style="" class="" clear="none"><br style="" class="" clear="none">Would it be possible to completely remove the printing of the modifier<br style="" class="" clear="none">arguments
separately?<br style="" class="" clear="none"><br style="" class="" clear="none"><br style="" class="" clear="none">Other than that, this patch looks good to me as well.<br style="" class="" clear="none"><br style="" class="" clear="none"><br style="" class="" clear="none">-- <br style="" class="" clear="none">Earthling Michel Dänzer | <a style="" class="" shape="rect" href="http://www.amd.com/" target="_blank">http://www.amd.com</a><br style="" class="" clear="none">Libre software enthusiast | Mesa and X developer<div style="" class="" id="yqtfd00215"><br style="" class="" clear="none"></div><br style="" class=""><br style="" class=""></div> </blockquote> </div> </div> </div> </div></body></html>