- User Since
- May 22 2014, 1:24 PM (220 w, 6 d)
The change should be visible in tests because there's a DAGCombine that uses isKnownNeverNaN()?
Make the vector type check an assert instead of an actual condition for the transform (no test diffs).
I've fixed this by doing a ptrtoint convert on the assumption that ptrtoints are free.
Tue, Aug 14
Mon, Aug 13
Forgot to click 'Accept' here in Phab.
About the autogen script problem - we could hack that to use something less likely than 'TMP' as its regex string (eg, 'AUTOGEN'), but that will result in significant test file churn if someone updates an existing file.
Tests LGTM. It might help me or other reviewers see how these patterns escape the current set of transforms in foldAndOfICmps(), so please commit.
Sun, Aug 12
Fri, Aug 10
I suppose the extra check doesn't hurt anything, and this is all disorganized anyway. The simplifications of calls to constants should really be in InstSimplify.
The vector changes look correct, so LGTM.
You should be able to adjust 2 lines of code to allow non-commutative ops after:
Thu, Aug 9
I'm not finding the mails where the extraction of target-specific code came up, but my vague memory says @rnk had some ideas about how to improve things.
The title of this patch includes "Constant folding" - shouldn't the code be in ConstantFolding.cpp or some descendant of that?
Wed, Aug 8
I'm still confused. I don't see why we need to check isNegatibleForFree().
Marking as accepted here on Phab to avoid unknown state when committing.
About the bit hacking: I don't think clang should be in the optimization business. We should be able to take the most obvious/simple representation for this builtin and reduce it as needed (either in instcombine or the backend). So it would be better to use the version with the subtract rather than shift/or. That version corresponds more directly to the code in APInt::getMinSignedBits()?
Tue, Aug 7
As we discussed in D49306, I agree that this is probably a good perf transform, but I don't think we've shown any compelling reason to do this in IR vs. DAGCombiner.
I may be starting a substantial yak shaving with the inline question, but I wonder if it wouldn't be better to canonicalize with the 'not' after the 'xor' (ie, the inverse of this patch). I don't have a motivating example yet, but that just seems more natural rather than semi-arbitralily choosing an operand to invert.
Let's handle this one step at a time. Please remove the FP changes. Let's make this patch only about handling the non-commutative integer binops.