<html><head><meta http-equiv="Content-Type" content="text/html charset=windows-1252"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><br><div><div>On Aug 6, 2014, at 12:25 AM, Michel Dänzer <<a href="mailto:michel@daenzer.net">michel@daenzer.net</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div style="font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">On 06.08.2014 15:34, Matt Arsenault wrote:<br><blockquote type="cite"><br>On Aug 4, 2014, at 1:09 AM, Michel Dänzer <<a href="mailto:michel@daenzer.net">michel@daenzer.net</a>> wrote:<br><br><blockquote type="cite">On 02.08.2014 11:33, Matt Arsenault wrote:<br><blockquote type="cite">f32 has a comment and patterns about the behavior with 0 and negate<br>modifier which I'm unsure about. I think the source modifier should<br>work if the source is 0, since the negate modifier is supposed to be<br>applied before abs. Does this pattern need to be skipped for f64<br>too?<br></blockquote><br>The original problem was that fneg was implemented as V_ADD_F32_e64 of 0<br>and the operand with the negate source modifier set. This resulted in<br>+0.0 instead of -0.0 (i.e. the sign bit was not set) for an argument of<br>+0.0. There's a (graphics) piglit test which failed because of that.<br><br>Given that FNEG_SI and FABS_SI directly manipulate the sign bit, I<br>wouldn't expect that to require any special treatment anymore. As you<br>say, the source modifiers should work fine when folding these into other<br>operations.<br><br></blockquote><br><br><blockquote type="cite">So, I suspect the purpose of the (fneg (fabs)) pattern isn't to avoid<br>that problem but to avoid expanding both FABS_SI and FABS_SI.<br></blockquote><br><br>Yes, that seems to be what it accomplishes.<br></blockquote><br>A solution preserving that but allowing the (fneg (fabs)) to be folded<br>into other instructions might be to add FNEG_FABS_SI?<br></div></blockquote><div><br></div><div>This already works correctly. The complex patterns check for both the ISD::FNEG and ISD::FABS and fold them.</div><div><br></div><br><blockquote type="cite"><div style="font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><br><br><blockquote type="cite">f64 fneg is still implemented that way because there is no v_sub_f64.<br></blockquote><br>I suspect v_sub_f* would have the same issue as v_add_f32 did anyway.<br><br><br><blockquote type="cite">Here is an updated set that implements that for f64. These also<br>switch to preferring using the literal constant as the first operand,<br>rather than loading the immediate into a register.<br></blockquote><br>Sounds good, but I have to defer to Tom or others for review.<br><br><br>--<span class="Apple-converted-space"> </span><br>Earthling Michel Dänzer            |                  <a href="http://www.amd.com/">http://www.amd.com</a><br>Libre software enthusiast          |                Mesa and X developer</div></blockquote></div><br></body></html>