<div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Tue, Dec 1, 2015 at 11:52 AM Gao, Yunzhong <<a href="mailto:yunzhong_gao@playstation.sony.com">yunzhong_gao@playstation.sony.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Actually, I think the smart signbit here is not correct. For the fneg case, you need to<br></blockquote><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
toggle the sign bits of both components of a ppc_fp128: negate(d1 + d2) should<br>
produce (-d1 - d2) instead of (-d1 + d2). It probably indicates that this pass is wrong<br>
for both big-endian and little-endian ppc64 targets, but existing test cases were not strong<br>
enough to catch such cases. For the fabs case, you can test positivity using only one<br>
signbit, but to negate the number, you still need to toggle both sign bits.<br></blockquote><div><br></div><div>Sure, I didn't pay much attention on crafting the signbit.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
I have a feeling that you will run into more cases in the DAG combiner where you want<br>
to add special handling codes for ppc_fp128, and I also feel that this type is peculiar<br>
enough that other backend owners upon trying to add more DAG combiner passes are<br>
likely to miss adding the special handling codes as they probably should.</blockquote><div><br></div><div>This is what I was worrying about.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> In that case it<br>
would be a better idea to handle the ppc_fp128 DAG combiner cases somewhere under<br>
llvm::PPCTargetLowering::PerformDAGCombine() instead of in the target-independent<br>
DAGCombiner.cpp,<br></blockquote><div><br></div><div><div>It will work, but it seems doubling the maintenance for floating point combiners. I wonder if there is any better solution to make both combiners and endianness less painful - the problem sounds not as hard as what we have now.</div><div><br></div><div>As a simple solution, when see a LLVM IR bitcast, instead of generating (ISD::BITCAST x), can we generate (exchange_hi_lo (ISD::BITCAST x)) instead? ISD::BITCAST itself doesn't swap the registers anymore. In such case:</div></div><div>(exchange_hi_lo (ISD::BITCAST (fabs x)) is transformed to:</div><div>(exchange_hi_lo (and (ISD::BITCAST x) (not signbit)), which is correct. I'm not sure if exchange_hi_lo exists or not.</div></div></div>