- User Since
- May 6 2015, 11:54 AM (214 w, 3 d)
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.
May 27 2016
Incorporate code review suggestions by Teresa.
Thanks. Fixed the issues you cited, will post a revised patch.
May 26 2016
I think I agree with probinson about the need for removing the markers in the opt-none case. I think the most expedient thing to do would be to make the skipFunction() check at the same point in the code where "DisableColoring" is checked (line 1008), this way the "removeAllMarkers()" helper will be called on the way out. This has the minor compile-time drawback that there is some analysis done to get to this point, but that seems like a minor concern for bisections purposes.
May 24 2016
Submitted in r270559.
May 22 2016
Another friendly ping... I'd like to get this submitted at some point.
May 18 2016
May 16 2016
Incorporate fixes suggested by Wei.
Will post a revised patch shortly.
Improvements to comments.
Wei, you suggested adding an assert that triggers if an instruction is found otuside the the start/end lifetime that accesses memory.