- User Since
- May 10 2016, 6:42 AM (88 w, 1 d)
Tue, Jan 16
Hi @efriedma thanks a lot for the suggestions and the testcase. I already considered gluing though it impacts scheduling and the change will be noisy for tests but I'll look into your other suggestions too.
Mon, Jan 15
Ping. Modulo the bugs we may find with ADDCARRY / SUBCARRY further thoughts on this?
Fri, Jan 12
LGTM. Wait a couple of days before commiting just in case @efriedma has further comments.
Wed, Jan 10
LGTM. Please wait a couple of days before submitting just in case @efriedma has further comments.
Dec 15 2017
Dec 14 2017
- New tests in D41122 are now updated in this change to show the change in codegen
- New tests before D34515 is applied showing current codegen (and not after like I did the last time)
Dec 13 2017
Oh, that makes a lot of sense. I'll do. Sorry for the misunderstanding.
Dec 12 2017
- I forgot to update the Thumb counterpart of long-setcc.ll in the last update
- Split tests in D41122
- Improve test long-setcc.ll
- Sign extend operands as we will want to use the higher bits (they could be rubbish in the previous diff). Add a comment.
Dec 11 2017
We want to do here is to calculate the ADDCARRY / SUBCARRY with a wider type and then use that result to compute the carry/borrow. I think that by sign extension of the operands, the carry/borrow of wider operation should exactly be the same as in a narrower type:
Hi @miyuki I can commit it.
Dec 8 2017
Dec 7 2017
The generic combiner DAGCombiner::visitTRUNCATE does this
- Fix combiner with addcarry that causes PR35103
- Add regression test
Dec 6 2017
We believe there is a problem here as the generic combiner presumes we're going to use the first result (0) but actually the code is using the second (1)
Nov 29 2017
Nov 15 2017
A combiner kicks in for t33 because it is of the form (add, X, (addcarry Y, 0, C) and then it becomes (addcarry X, Y, C) which looks correct but I think it interacts badly with the nodes t24, t44 and t42 (which also have similar forms).
Hmm, scheduling is correct. I was all wrong, the legalized DAG is already wrong.
Nov 14 2017
Thanks a lot for the pointer Eli! :-)
UploadedDAG after isel.
I'm trying to understand what went wrong in the following input
Nov 3 2017
Nov 2 2017
Thank you @mclow.lists !
Nov 1 2017
This is causing PR35103, so reopening after the revert, so I can investigate what is going wrong there.
Oct 16 2017
Oct 12 2017
- Reinstate the transformation for ARM as well.
Oct 10 2017
@efriedma Ah I see. If I get you right, the initial change was more sensible. Also Sam's concerns were on a file that is explicitly marked as -O0.
Oct 9 2017
- Remove another instance of this issue
Oct 6 2017
- I wanted to mark two tests but forgot to add one in the previous change
Oct 5 2017
- Reordered combiner and added early exit for non-integers.
- Constrained the CLZ combiner to T32 because it is not clear whether it is profitable in A32 (where conditional moves are more accessible)
- Added negative tests to make sure no branches are emitted where we don't want them.
Oct 3 2017
Oct 2 2017
Sep 29 2017
Sep 28 2017
Friendly ping :)
Sep 21 2017
- Updated the patch to use ADDCARRY / SUBCARRY nodes.
Sep 19 2017
Sep 14 2017
Ok, Chrome for Linux Arm took a bit to build with a debug compiler but finished successfully.
Sep 13 2017
In the second instance of PR34045 what happens is that ADDC is exactly HiAdd, not just a predecessor of the latter.
- fix for the second instance of PR34045
- also integrates the changes to fix PR34564, fall-out of the original change, that was reverted as well
Sep 12 2017
We're still hitting PR34045. I also plan integrate the fix in D37690 to ease reverting if needed.