- User Since
- May 10 2016, 6:42 AM (97 w, 1 d)
Thu, Mar 1
Looks good to me. Thanks!
Tue, Feb 20
LGTM now. Thanks!
Feb 19 2018
Thanks a lot Marten.
Feb 16 2018
Looks good to me now. Wait a couple of days before submitting it just in case the other reviewers have more comments.
Let's see how this one goes :)
- Rebase with ToT
Feb 15 2018
Feb 14 2018
Can you coalesce them in a single regular expression, like /;\s*CHECK-(NEXT|NOT|DAG|SAME|LABEL): (not sure if this is the right syntax but I hope you got the idea.
Feb 9 2018
This LGTM. Thanks for fixing this.
Feb 7 2018
Feb 6 2018
I understand you're fixing the narrowing check from "unscoped enum/integer type" → float.
Jan 31 2018
Thank you very much @efriedma !
Jan 29 2018
Jan 24 2018
- Remove whitespace and unused variables in LowerADDSUBCARRY
- Change ConvertBooleanCarryToCarryFlag to use ARMISD::SUBC x, 1 instead of ARMISD::ADDC x, ~0, this way a single combiner in PerformAddcSubcCombine is enough.
- Do not use ISD::SUBCARRY if the third operand is a zero. While the generic combiner can lower this to ISD::USUBO it may not have a chance to run before we lower that ISD::SUBCARRY.
Jan 22 2018
Update patch based on the review comments.
Jan 18 2018
Hi @aemerson thanks a lot for the prompt fix.
Hi @efriedma gluing the nodes is too disruptive so I am exploring one of the alternatives you suggested. What do you think, does this look workable?
Jan 16 2018
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.
Jan 15 2018
Ping. Modulo the bugs we may find with ADDCARRY / SUBCARRY further thoughts on this?
Jan 12 2018
LGTM. Wait a couple of days before commiting just in case @efriedma has further comments.
Jan 10 2018
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.