- User Since
- May 6 2015, 11:54 AM (224 w, 1 d)
Thu, Aug 15
FYI, I filed an LLVM bug to track this issue (has a testcase and dumps showing incorrect unwind, etc). Bug is
Mon, Aug 12
LGTM. I did some hand testing with the patch and the results seem fine.
Jul 19 2019
Jul 18 2019
Add include of <string> as well, also needed.
Jul 17 2019
Jul 16 2019
Good cleanup -- LGTM.
Mar 14 2019
Thanks for reviewing.
Mar 11 2019
Additional changes suggested by Wei.
Mar 8 2019
Mar 6 2019
Formatting and variable/function naming.
Mar 1 2019
Nov 30 2018
Nov 26 2018
Nov 16 2018
Nov 7 2018
Comment change only.
Thanks for the review. I've fixed up the header comment.
Nov 5 2018
Oct 25 2018
LGTM. I ran the new recipe in the debugger and it is indeed exercising the PR37130 fix code correctly (lifetime marker set is different, but that doesn't matter in this case).
Jul 11 2018
Thanks for looking into this-- I've definitely run into the "Unable to schedule pass" issue but hadn't been able to sort out the cause.
Jun 26 2018
Jun 25 2018
Update commit message to reflect that this is for both ARM and x86.
If there are no other comments or concerns on this patch, I plan to submit it tomorrow. Thanks -NM.
Jun 21 2018
May 29 2018
Clean up unused meta data in testcase. Fix typo in commit msg.
Thanks for reviewing. I'll remove the cruft from the testcase and check it in.
May 25 2018
I agree, given the nature of the change. I'll prepare another patch.
May 22 2018
Sorry I'm not sure what to do in that case. Locally it runs fine on my machine, and I've tested it on another system and it's ok there. You're probably safe enough to re-land the change without the test case.
May 18 2018
Failing buildbot run is:
May 16 2018
Thanks! Review comments and suggestions much appreciated. I will try to submit this later in the week.
Incorporate changes suggested by Simon:
- remove option to disable statically unreachable block elim
- modify MIR test to include "-start-before isel"
May 14 2018
May 11 2018
Revised test case. New hidden option to turn off statically
unreachable block elimination.
May 10 2018
Friendly ping... let me know if I need to recruit additional reviews.
May 3 2018
Ready for review at this point.
May 1 2018
@thanm You can take the reproducer in the bug report, produce LLVM-IR from it (-S -emit-llvm), then run it through llc with "-stop-before stack-slot-coloring" to get MIR output. The test file would then be invoked with the same options as before except you'd add '-start-before stack-slot-coloring'.
Thank you! I will look into doing that.
Apr 30 2018
I would welcome suggestions on how to write a regression test / lit test for this issue. The tricky part seems to be insuring that a statically unreachable BB survives all the way through the llc pipeline to make it to stack coloring.
Mar 1 2018
Sep 28 2017
Remark: there are several outstanding LLVM bugs for this problem:
Sep 27 2017
Jun 12 2017
Jun 8 2017
FWIW I tried a self-build of clang with and without the change and looked at the -stats output to see what sort of improvement there is. Looks pretty modest (<1%) but it is positive overall.
Jun 6 2017
I pulled your patch and did a bootstrap build with it, no issues. Overall this seems like an improvement over the initial patch. Please see inline comments.
May 15 2017
Apr 4 2017
Jun 1 2016
Incorporate code review suggestions from Wei.