- User Since
- May 22 2014, 1:24 PM (248 w, 1 d)
Thu, Feb 21
- Added TODO comment about handling non-zero extract index.
- Reformatted switch.
Wed, Feb 20
Tue, Feb 19
Should we change the generic model too then since most CPUs prefer inc/dec?
I'm guessing that's a lot more test diffs, so no problem to make it another patch, but just want to know if that's the way to think about the generic model.
Abandoning. Added a sentence to the LangRef here:
Thanks, Peter. 'HasFastVariableShuffle' is a heuristic, so we're never going to get it right all the time, but it sounds like we should leave this particular bit of logic as-is and try to chisel out smaller patterns/transforms for sure wins.
Mon, Feb 18
Sun, Feb 17
The change looks ok, but can you describe here or point to a discussion about how this fits in the overall plan for overflow op codegen? Ie, what exactly is enabled by moving this code?
Sat, Feb 16
Independent of this patch, but just so everyone's aware - there's currently no consistency in the way we transform to the overflow intrinsics. We may transform to sadd.with.overflow as an IR canonicalization (no target checks):
Fri, Feb 15
Restrict the change to targets without fast-variable-shuffle.
Better to stage this in 2 parts?
I can add a check for fast-variable-shuffle, so we get the clear improvements. Then, a follow-up can remove that check and see if that results in any real-world fallout.
Add llvm_unreachable so we don't accidentally return without a value.
Thu, Feb 14
@jonpa - IIRC, some targets infinite loop if we always allow the generic trunc fold in DAGCombiner after legalization, but you might be able to constrain it using the existing TLI hooks:
Can you describe what's happening within SimplifySetCC for the affected test?
Adding more potential reviewers for legalization changes. I'm not sure how much combining/optimization we want to do in here.
LGTM - have you requested commit access?
Wed, Feb 13
Some IR tests added here:
I think that's better than IRBuilder tests. Feel free to add other predicates if you think they are needed, but I think those 4 give us good coverage.
Tue, Feb 12
This took some work, but I have an IR test case that should show the bug: