-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 04/22/11 09:21, Chung-Lin Tang wrote: > This patch is the main bulk of this submission. It modifies the compare > combining part of try_combine(), adding a call of > CANONICALIZE_COMPARISON into the entire logic. > > Also, instead of testing for XEXP(SET_SRC(PATTERN(i3)),1) == const0_rtx > at the top, it now allows CONST_INT_P(XEXP(SET_SRC(PATTERN(i3)),1)), > tries to adjust it by simplify_compare_const() from the last patch, and > then tests if op1 == const0_rtx. This is a small improvement in some cases. > > (if you remove the call to simplify_compare_const(), and if > CANONICALIZE_COMPARISON is not defined for the target, then this entire > patch should be an 'idempotent patch', nothing should be changed to the > effective combine results) > > One issue that I would like to RFC, is the use of > CANONICALIZE_COMPARISON here; I'm afraid I might be abusing it. Since > added_sets_2 is set here, the value of i2src/op0 is needed afterwards. > This might not be conformant to the description of > CANONICALIZE_COMPARISON in the internals manual, which doesn't say it > can't trash op0 under arbitrary conditions while only preserving the > comparison result. > > OTOH, I don't see another suitable macro/hook (with a close enough > meaning), and the current definitions of CANONICALIZE_COMPARISON across > the targets do not seem to clash with my use here. Does this macro use > look okay, or does this justify creating a new one? You seem to have removed the block comment (which was originally within a SELECT_CC_MODE block). I can see why, but ISTM you need some comments to improve the readability of this code. The code in the if (cc_use_loc) block, it appears you're updating the cc user, a comment to that effect and why is appropriate.
Following that block, you're actually changing the current insn, again a comment indicating that would be helpful in making the code easier to read. With regards to CANONICALIZE_COMPARISON; I think you're OK. As far as I can tell, you aren't changing the value of op0, you're just changing its form. I've generally found that avoiding const0_rtx is wise. Instead I suggest using CONST0_RTX (mode) to get the constant in the proper mode. Given the tangled mess this code happens to be, could you please do a bootstrap test on target with and without SELECT_CC_MODE. x86 would be a great example of the former, powerpc the latter (use the build farm if you need to). For ppc, just bootstrapping would be fine; I don't think a full regression test is needed, just some verification that we don't break ppc build should be sufficient. jeff -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/ iQEcBAEBAgAGBQJNtbiDAAoJEBRtltQi2kC7l0AH/3Vy479EufdUs2JAWJDnEdE1 TZ1Pa6zHhbb/hx6N8Jk1xgB6xM2QhzTCzjCFKVudQVNHxGx1hA0wkCN0jOsBmr91 pifBPmqr5a+w1bH4pfVMaG7XWqSMJmQP9Y02cAIbbB42TlX+47jAzfrEVaz2jRBX 9C3FetAZPSwhBlRiCzH3EC1Psoqs6NypuZgclUzdkOGoXTELVpzK7sxj7N0Vzf0O Ubg3yStIpNFM+Kp+6g6yOjg+Q90gSCvR+EPM6jMMyFOxvykJ9RjTiIrvWklloqLO Ez9hFd+i51h3px0xEdLfifRtIYHn47uj+4EBe4b5wXNJc+hQYvEa8gX7FVD8YXw= =fQxG -----END PGP SIGNATURE-----
